feat(#10038): add PlaceQualifier and related methods
#10057
Conversation
Signed-off-by: Apoorva Pendse <[email protected]>
| if (!parent.parent) { | ||
| return Object.keys(parent).length > 1; | ||
| } |
There was a problem hiding this comment.
This was a case I missed addressing in #10043
There was a problem hiding this comment.
Basically for the final parent.
There was a problem hiding this comment.
nice. can you add a comment above this as well for future selves.
apoorvapendse
force-pushed
the
place-qualifier
branch
2 times, most recently
from
fd06592 to
4e197c4
Compare
| if (!parent.parent) { | ||
| return Object.keys(parent).length > 1; | ||
| } |
There was a problem hiding this comment.
nice. can you add a comment above this as well for future selves.
| */ | ||
| export type PlaceQualifier = ContactQualifier & Readonly<{ | ||
| parent?: NormalizedParent; | ||
| contact?: NormalizedParent; |
There was a problem hiding this comment.
question: Isn't contact supposed to be either a string containing a UUID string or a object that contains the key _id which has the value of a UUID string. Using Identifiable interface would be more appropriate here.
There was a problem hiding this comment.
Yes, I was a bit unclear about this.
But I ended up seeing and following:
There was a problem hiding this comment.
I re-read and re-ran the code for this. I have an explanation for this. It's true that the contact will have a value of string or an object with _id with value string. However, when requesting a place along with it's lineage, the contact object is expected to be a person typed object with parent as a field and so on. In which NormalizedParent is a nice fit-in. Example.
This can be left as it is.
| if ('parent' in qualifier) { | ||
| if (!isNormalizedParent(qualifier.parent)) { | ||
| throw new InvalidArgumentError(`Missing required fields in the parent hierarchy [${JSON.stringify(qualifier)}].`); | ||
| } | ||
| if (hasBloatedLineage(qualifier)) { | ||
| throw new InvalidArgumentError(`Additional fields found in the parent lineage [${JSON.stringify(qualifier)}].`); | ||
| } | ||
| } | ||
|
|
||
| if (hasField(qualifier, {name: 'contact', type: 'object'})) { | ||
| if (!isNormalizedParent(qualifier.contact)) { | ||
| // eslint-disable-next-line max-len | ||
| throw new InvalidArgumentError(`Missing required fields in the contact hierarchy [${JSON.stringify(qualifier)}].`); | ||
| } | ||
| if (hasBloatedLineage(qualifier.contact)){ | ||
| throw new InvalidArgumentError(`Additional fields found in the contact lineage [${JSON.stringify(qualifier)}].`); | ||
| } | ||
| } |
There was a problem hiding this comment.
suggestion: there's probably a way to refactor this
Signed-off-by: Apoorva Pendse <[email protected]>
There was a problem hiding this comment.
I've added missing coverage for qualifer.ts in the other PR(#10056), so skipping that here.
Signed-off-by: Apoorva Pendse <[email protected]>
Signed-off-by: Apoorva Pendse <[email protected]>
apoorvapendse
force-pushed
the
place-qualifier
branch
from
15858d1 to
b41451d
Compare
…-contacts-and-reports' into place-qualifier
Description
WIP Pull request to add
PlaceQualifierand related validations.Related: #10038
Code review checklist
can_view_old_navigationpermission to see the old design. Test it has appropriate design for RTL languages.License
The software is provided under AGPL-3.0. Contributions to this project are accepted under the same license.