Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

project-schema.json: add benefits array #438

Merged
merged 11 commits into from
Nov 28, 2023
Merged

project-schema.json: add benefits array #438

merged 11 commits into from
Nov 28, 2023

Conversation

odscjen
Copy link
Contributor

@odscjen odscjen commented Nov 21, 2023

Related issues
closes #335

Description

added additional field, benefit.title that was in the example in #335 (comment) but not in the table of new elements which was just an oversight as this field had already been discussed as part of the mapping for one of the relevant new sustainability data points in IDS

Merge checklist

If there are changes to project-schema.json or project-package-schema.json:

  • Update the examples:
    • docs/examples/example.json
    • docs/examples/blank.json
  • Run ./manage.py pre-commit to update docs/_static/i8n.csv

If you added a new definition to the schema, run ./manage.py pre-commit.

If you added a new codelist:

  • Add an entry to docs/reference/codelists.md

Copy link
Contributor

@duncandewhurst duncandewhurst left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For compatibility with Flatten Tool, I think that Benefit still needs an .id field because .beneficiaries is an array of objects. Although, I've just remembered that flattening is disabled in the OC4IDS Data Review Tool so I'll defer to @jpmckinney on whether .id is needed.

schema/project-level/project-schema.json Outdated Show resolved Hide resolved
mapping/sustainability.yaml Outdated Show resolved Hide resolved
mapping/sustainability.yaml Outdated Show resolved Hide resolved
mapping/sustainability.yaml Outdated Show resolved Hide resolved
mapping/sustainability.yaml Outdated Show resolved Hide resolved
mapping/sustainability.yaml Outdated Show resolved Hide resolved
schema/project-level/project-schema.json Outdated Show resolved Hide resolved
schema/project-level/project-schema.json Outdated Show resolved Hide resolved
removing `wholeListMerge` properties

Co-authored-by: Duncan Dewhurst <[email protected]>
@odscjen
Copy link
Contributor Author

odscjen commented Nov 24, 2023

tests failing due to no id in array objects, tests to be changed as per #438 (comment)

@odscjen odscjen requested a review from jpmckinney November 24, 2023 09:40
@jpmckinney jpmckinney merged commit a34077f into 0.9-dev Nov 28, 2023
9 of 10 checks passed
@jpmckinney jpmckinney deleted the 335_benefits branch November 28, 2023 21:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add numberOfBeneficiaries in planning
3 participants