-
Notifications
You must be signed in to change notification settings - Fork 4
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
EVA-3529 Notify submission update to user through email #10
Conversation
nitin-ebi
commented
Mar 14, 2024
@@ -85,9 +95,17 @@ public boolean checkUserHasAccessToSubmission(SubmissionAccount account, String | |||
Optional<Submission> optSubmission = submissionRepository.findById(submissionId); | |||
if (optSubmission.isPresent()) { | |||
SubmissionAccount submissionAccount = optSubmission.get().getSubmissionAccount(); | |||
return submissionAccount.getId() == account.getId(); | |||
return submissionAccount.getId().equals(account.getId()); |
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.
I assume this fixes EVA-3536 as well 👍
.addGap(1) | ||
.addText("Here is the update for your submission: ") | ||
.addGap(1) | ||
.addText("submission id: " + submissionId) |
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.
.addText("submission id: " + submissionId) | |
.addText("Submission ID: " + submissionId) |
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.
done
@@ -87,6 +88,7 @@ public ResponseEntity<?> markSubmissionUploaded(@RequestHeader("Authorization") | |||
} | |||
|
|||
Submission submission = this.submissionService.markSubmissionUploaded(submissionId); | |||
submissionService.sendMailNotificationForStatusUpdate(submissionAccount, submissionId, SubmissionStatus.UPLOADED, Boolean.TRUE); |
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.
Just curious, is there a reason to use a Boolean
object here instead of a primitive?
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.
Not really and frankly not needed. Have changed to primitive to be consistent