Skip to content
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

SAK-50903 Assignments a new assignment using groups should emit an asn.new.assignment event #13234

Merged
merged 1 commit into from
Jan 29, 2025

Conversation

susowrc
Copy link
Contributor

@susowrc susowrc commented Jan 24, 2025

SAK-50903 Creating a new assignment and putting it to site users creates an asn.new.assignment event, but this does not happen when putting it to selected groups.

https://sakaiproject.atlassian.net/browse/SAK-50903

@susowrc
Copy link
Contributor Author

susowrc commented Jan 24, 2025

@jkjanetschek We note that you were the last person who was playing this part. Could you review the changes uploaded?

@jkjanetschek
Copy link
Contributor

@susowrc Yes, I was working on this code area when I worked on SAK-45992. Back then, I did not switch EVENT_UPDATE_ASSIGNMENT_ACCESS to EVENT_ADD_ASSIGNMENT to change as little as possible while fixing SAK-45992.

As I see it, everything related to UserNotifications works fine with your changes.

@ern ern changed the title SAK-50903 Creating a new assignment and putting it to site users creates an asn.new.assignment event, but this does not happen when putting it to selected groups. SAK-50903 Assignments a new assignment using groups should emit an asn.new.assignment event Jan 27, 2025
@@ -9105,7 +9105,7 @@ private void post_save_assignment(RunData data, String postOrSave) {
if (aGroups.size() != 0) {
// If already open
if (openTime.isBefore(Instant.now())) {
eventTrackingService.post(eventTrackingService.newEvent(AssignmentConstants.EVENT_UPDATE_ASSIGNMENT_ACCESS, assignmentReference, true));
eventTrackingService.post(eventTrackingService.newEvent(AssignmentConstants.EVENT_ADD_ASSIGNMENT, assignmentReference, true));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this appears to be the same logic in the else so the if can be simplified

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, we noticed that too. We didn't want to manipulate any more code than necessary. I have made the changes and uploaded them. Thank you.

…tes an asn.new.assignment event, but this does not happen when putting it to selected groups.
@jesusmmp jesusmmp merged commit 840b3a4 into sakaiproject:master Jan 29, 2025
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants