fix: use importer for update object property [DHIS2-11944] #9079
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
jbee
commented
Oct 19, 2021
| new MergeParams<>( userCredentials, persistedUserCredentials ).setMergeMode( bundle.getMergeMode() ) ); | ||
| preheatService.connectReferences( persistedUserCredentials, bundle.getPreheat(), | ||
| bundle.getPreheatIdentifier() ); | ||
| if ( userCredentials != persistedUserCredentials ) |
There was a problem hiding this comment.
Not sure when or why but apparently these can be the very same instance and merging them messes up proxy collections I guess. Anyhow, only merging dissimilar instances should not be less correct.
jbee
commented
Oct 19, 2021
Comment on lines
+104
to
+105
| && (PropertyType.REFERENCE == p.getPropertyType() && schema.getKlass() != p.getKlass() | ||
| || PropertyType.REFERENCE == p.getItemPropertyType() && schema.getKlass() != p.getItemKlass()) ) |
There was a problem hiding this comment.
This is so we don't process self-referencing fields like User in a User like it does occur for createdBy and lastUpdatedBy.
jbee
commented
Oct 19, 2021
| @@ -269,7 +267,7 @@ private Patch diff( HttpServletRequest request ) | |||
| return patchService.diff( new PatchParams( mapper.readTree( request.getInputStream() ) ) ); | |||
| } | |||
|
|
|||
| @RequestMapping( value = "/{uid}/{property}", method = { RequestMethod.PUT, RequestMethod.PATCH } ) | |||
| @PatchMapping( "/{uid}/{property}" ) | |||
There was a problem hiding this comment.
We talked about PUT being mapped twice so it got removed here. Only PATCH ever worked and as I understood it was documented.
|
Kudos, SonarCloud Quality Gate passed! |
mortenoh
approved these changes
Oct 19, 2021
larshelge
approved these changes
Oct 21, 2021
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
Goal of this PR was to use importer to apply single object property updates so that additional validation performed in the object bundle hooks would execute for these updates as well.
Using the importer has revealed a few issues that have been addressed. I pointed these out in comments on the line in question.
Automatic Tests
This adds a test scenario that might look overly complicated at first sight. This is because this more complex scenario would show an issue with the update so the test make sure this issue is not present. A similar scenario was added to full object update to check and make sure a similar problem does not exist there.