-
Notifications
You must be signed in to change notification settings - Fork 92
Edit-action #11275
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
base: develop
Are you sure you want to change the base?
Edit-action #11275
Conversation
|
Oops! Looks like you forgot to update the changelog. When updating CHANGELOG.md, please consider the following:
|
# Conflicts: # packages/client/src/v2-events/hooks/useUserDetails.ts
# Conflicts: # packages/client/src/v2-events/features/workqueues/EventOverview/EventOverview.stories.tsx
Nil20
left a comment
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.
Great clean work so far. 🙌 Looked over mostly the commons and events change, although most changes are in client. 😄 Left a few comments for you to address.
| // If there is more than one declare action, lets filter out all actions between the second last and last declare actions | ||
| // Why is this done? This is to handle 'redeclaration' cases, i.e. when a user edits a declared record and then does 'Declare with edits' | ||
| // Then we want to ignore all the actions between the second last and last declare actions. |
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.
Having two probable DECLARE actions might have impact on some places that we would not be able to identify straight away. This could be one of those places.
We let the users search with legalStatuses.DECLARED.createdAtLocation to search with a location where a record was DECLARED. Maybe you can manually test if searching with this field works okay and add a test.
Suggestion: You can test it out by adding a field like this and see if gets populated with the correct value after a re-declare with edits is done.
| const declaration = getCurrentEventState(event, eventConfig).declaration | ||
|
|
||
| if (actionType === ActionType.NOTIFY) { | ||
| if (actionType === ActionType.NOTIFY || actionType === ActionType.EDIT) { |
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.
Is it right to validate the Edit action payload with validateNotifyAction? Is it because both can have incomplete input payload?
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.
these validation functions call FieldTypeMapping to have a zod schema for validating a field value. Like here, for notify actions firstname/surname of a name field can be empty.
For Edit action, that ({ firstname: 'Cihan', surname: undefined } ) should be validated as well. So, maybe FieldTypeMappings should be updated. And add a test case regarding this.
| if (declareIndexes.length >= 2) { | ||
| const secondLast = declareIndexes[declareIndexes.length - 2] | ||
| const last = declareIndexes[declareIndexes.length - 1] | ||
| actions = sortedActions.filter((_, idx) => idx < secondLast || idx >= last) |
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.
| actions = sortedActions.filter((_, idx) => idx < secondLast || idx >= last) | |
| actions = [ | |
| ...sortedActions.slice(0, secondLast), | |
| ...sortedActions.slice(last) | |
| ] |
might be a tad more readable
Description
Resolves: #10939
cc pr: opencrvs/opencrvs-countryconfig#1182
farajaland pr: opencrvs/opencrvs-farajaland#1874
See small demo video here! https://opencrvsworkspace.slack.com/archives/C02LU432JGK/p1765505724107889
Edit-action replaces the editing as part of validation/registering functionality we had previously.
record.declared.editscope and give it to relevant test userseditAndRegister&editAndDeclareActionType.EDITuseUserDetails.ts->useCurrentUser.tsEventHistory.tsx->useUserDetails.tsso it can be used inEditPageBanner.tsxFollow-up issue for replacing current notify -> declare flow with edit: #11306. This would allow us to get rid of the 'Updated' functionality and a whole bunch of code.
Checklist