-
Notifications
You must be signed in to change notification settings - Fork 39
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
[MDS-6006] Allow ability to edit and remove the orgbook associations #3140
Conversation
|
||
useEffect(() => { | ||
if (selectedParty !== current_party) { | ||
setSelectedParty(placeHolder); |
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.
Any particular reason for setting the selected party state to be the constant placeholder value "Start typing to search OrgBook..." ?
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.
In the flow,
- User sees the place holder (initial step, when no party is associated)
- Then user types and select one party and click on
Associate
button. This is will disabled the search box - Then (if) user want to change the selected party, user clicks on
Disassociate
button (screen shots in the PR) - In step (3), if assigned selected party as
- setSelectedParty(NONE) -> it will show None in the search box
- setSelectedParty("") -> it will show the empty in the search box.
So I thought assigning the place holder to the party, makes things easier for the user.
If there's better options to handle this, please let me know.
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.
@henryoforeh-dev actually, setSelectedParty(undefined) resolve the issue. I'll commit this
raise NotFound('OrgBook entity not found.') | ||
|
||
party_orgbook_entity.delete() | ||
return None, 204 |
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.
-
The @api.marshal_with decorator specifies a status code of 201, which is used for "Created" responses. However, for a successful deletion, the status code should be 204 (No Content).
-
The @api.expect(parser) decorator suggests that some input validation or parsing is expected, but it is not being utilized within the function.
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.
This is addressed.
Quality Gate passed for 'bcgov-sonarcloud_mds_minespace-web'Issues Measures |
Quality Gate failed for 'bcgov-sonarcloud_mds_common'Failed conditions |
Quality Gate failed for 'bcgov-sonarcloud_mds_core-web'Failed conditions |
Quality Gate passed for 'bcgov-sonarcloud_mds_core-api'Issues Measures |
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.
Delete and remake makes sense to me in this case.
Only admins can see button and disassociate, good call! assuming Rebecca agrees.
I wonder if we should start showing the Party NAME and Orgbook names together (the original feature overrode the party name with the orgbook name). But that is future work.
…3140) * Allow ability to edit and remove the orgbook associations * Fixing the issue of missing the party after the update * Fixing the issue of changing the selected party before associating * Fixing the label when nothing is selected. * Updating roles for delete resource * Addressing PR comments * Addressing PR comments
Objective
MDS-6006
party_orgbook_entity
table.Why are you making this change? Provide a short explanation and/or screenshots