Feat update curriculummembership api #1043
Open
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
jayanta2018
force-pushed
the
feat-update-curriculummembership-api
branch
from
e068655 to
2ed80c1
Compare
hihilary
requested changes
Jan 16, 2025
| * @param curriculumMembershipDto the dto to patch | ||
| * @return the ResponseEntity with status 200 (OK) and with body the patched dto | ||
| */ | ||
| @PatchMapping("/curriculum-membership") |
There was a problem hiding this comment.
- Pepe suggested using the plural format: https://hee-nhs-tis.slack.com/archives/C03GBMYGZD4/p1736356584130419?thread_ts=1736352700.784459&cid=C03GBMYGZD4
- Just want to check with you.
patchrequests are not going to be logged in the AuditingAspect class, are you okay with it?
There was a problem hiding this comment.
- Ok thanks. I will go for plural format.
- Yeah, looks like it is, patch requests are not logged in because we don't have patch enum here in the GenericAuditEventType class. We also need to write a pointcut and advice (that might be a new Jira ticket, what do you think?) Patch was used earlier in ProgrammeMembership update. So I went for patch because of this AC "Given I have left some of the column empty on the template
When the template is uploaded
Then existing TIS values should not be overwritten with blanks."
| @Transactional | ||
| public CurriculumMembershipDTO patch(CurriculumMembershipDTO cmDto) { | ||
| log.debug("Request to patch CurriculumMembership : {}", cmDto); | ||
| CurriculumMembershipDTO curriculumMembershipDtoFromDb = findOne(cmDto.getId()); |
There was a problem hiding this comment.
Please check the explanation on Sonar. Any method marked with @Transactional seems to be executed asynchronously, so it suggets to use a injected instance to invoke it rather than this.
| public CurriculumMembershipDTO patch(CurriculumMembershipDTO cmDto) { | ||
| log.debug("Request to patch CurriculumMembership : {}", cmDto); | ||
| CurriculumMembershipDTO curriculumMembershipDtoFromDb = findOne(cmDto.getId()); | ||
| if (curriculumMembershipDtoFromDb == null) { |
There was a problem hiding this comment.
This is the only place curriculumMembershipDtoFromDb is used, but I thought you would update it with the dto got from the client.
...n/java/com/transformuk/hee/tis/tcs/service/service/impl/CurriculumMembershipServiceImpl.java
Show resolved
Hide resolved
tcs-client/src/main/java/com/transformuk/hee/tis/tcs/client/service/impl/TcsServiceImpl.java
Outdated
Show resolved
Hide resolved
jayanta2018
force-pushed
the
feat-update-curriculummembership-api
branch
from
6d7f575 to
69450f9
Compare
jayanta2018
force-pushed
the
feat-update-curriculummembership-api
branch
from
69450f9 to
53c6c5b
Compare
Checked with James and Ade. It is for validation only. The confluence page also says so. |
jayanta2018
force-pushed
the
feat-update-curriculummembership-api
branch
from
ba8cc3e to
9b09123
Compare
jayanta2018
force-pushed
the
feat-update-curriculummembership-api
branch
from
9b09123 to
0d5d7a5
Compare
|
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.
This end point is required by the Bulk Upload service for updating curriculum memberships.
TIS21-3819