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

feature/json-validation #157

Merged
merged 4 commits into from
Dec 31, 2021

Conversation

BrianL3
Copy link
Collaborator

@BrianL3 BrianL3 commented Dec 31, 2021

Link to Relevant Issue

This pull request resolves #150

Description of Changes

As part of ModelService's sanity checks (did anything get returned from NetworkService? was there an error? etc) before it creates a Model, it now checks to see if the required properties are on the json. This does not cascade down, but we have very flat database objects so it works out.

This enables us to have required properties on our Model objects.

Link to Forked Storybook Site

Storybook (no changes to visual components)

Brian Ledbetter added 2 commits December 30, 2021 16:25
…while testing. Probably due to lack of pagination on PersonPage
Copy link
Collaborator

@tohuynh tohuynh left a comment

Choose a reason for hiding this comment

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

Have some fixes for reqruied fields and TransriptJson.

And a general question regarding the creation of new Model out of a model_ref pattern:

if (typeof jsonData["body_ref"] === "object") {
      this.body = new Body(jsonData["body_ref"]);

To pass the two test cases, add the required fields for the example Event at
https://github.com/BrianL3/cdp-frontend/blob/feature/json-validation/src/networking/test/ModelService.test.ts#L10
https://github.com/BrianL3/cdp-frontend/blob/feature/json-validation/src/networking/test/ModelService.test.ts#L18

Promise.resolve(new NetworkResponse({ agenda_uri: "test", <add required fields> }))

src/models/Body.ts Outdated Show resolved Hide resolved
src/models/Event.ts Outdated Show resolved Hide resolved
src/models/TranscriptJson.ts Show resolved Hide resolved
src/models/Vote.ts Outdated Show resolved Hide resolved
src/models/util/validateResponseData.ts Show resolved Hide resolved
@tohuynh
Copy link
Collaborator

tohuynh commented Dec 31, 2021

getRequiredProperties also need to be updated after changing the model files to have missing required fields as well.

@evamaxfield evamaxfield changed the title Feature/json validation feature/json-validation Dec 31, 2021
Copy link
Member

@evamaxfield evamaxfield left a comment

Choose a reason for hiding this comment

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

This all looks good to me. Once again, I cannot wait until we get a backend person / a dev ops person to write some fancy code gen for this type of stuff.

@BrianL3 BrianL3 merged commit 641fcd4 into CouncilDataProject:main Dec 31, 2021
@tohuynh tohuynh mentioned this pull request Jan 2, 2022
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.

JSON Validation
3 participants