-
Notifications
You must be signed in to change notification settings - Fork 2
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
Add Hedgerow schema #308
Add Hedgerow schema #308
Conversation
… example data file
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great, two minor comments !
reason: string; | ||
hedgerowLength: {metres: number}; | ||
hedgerowAgeLessThanThirty: boolean; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we actually want to fully omit description
, I think we can alternatively do something like this:
export type HedgerowRemovalNoticeProposal = Pick<ProposalBase, "boundary"> & {
reason: string;
hedgerowLength: { metres: number };
hedgerowAgeLessThanThirty: boolean;
}
Which rather than extending the whole ProposalBase will just pick the single optional boundary field and add it to the rest of our custom proposal definition !
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That makes sense to me! I would defer to you, do you want to omit description here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's omit if never actually appears in the service itself as August was clarifying !
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Made these suggested edits to Proposal.ts
}, | ||
area: { | ||
squareMetres: 2526.36, | ||
}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: I think our proposal.boundary.area
always expects both squareMetres
and hectares
properties?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah yes, just confirmed hectares are optional. Which makes sense, it passed the tests I ran locally. Would you prefer to have them included?
Several edits to add data schema for Hedgerow Removal Notice: Added HedgerowRemovalNotice Applicant, Application Data, Proposal, and Property.
Additional edits made to allow for this: