Resolve "Notification asking students for feedback"
Closes #242 (closed), #245 (closed)
This merge request sightly refactors the way notifications are created. The task of creating notifications has been moved to notification service instead of random classes. Also has a new notification been added, the ask for feedback notification for students. If they click on the notification they will be redirected towards the feedback page for that specific request.
Furthemore, it also lets students be notified if a TA is not able to find them.
Merge request reports
Activity
added typefeature-request label
added 1 commit
- d7697394 - Move te changes to the database to the correct file
mentioned in issue #245 (closed)
added 2 commits
assigned to @otto, @LiamClark, and @Lemaire
unassigned @Lemaire
added 817 commits
-
dc692555...c1281d52 - 803 commits from branch
development - b3e3a814 - Do not send notifications to TA's if a new request is created
- 3f3c54e3 - Remove unused code
- ee854deb - Revert "Remove unused code"
- f99bcb57 - Refactor code for sending table updates to TA
- 6df68f91 - Move createNotification method to notification service
- 6ec6e4bd - Create notification method for feedback
- b794fc26 - Send push notification to the student
- 8a7e0267 - Add urls to notification such that clicking on them brings you to a usefull page
- b69c675d - Create test for feedback notifications
- 3f49ccbb - Move te changes to the database to the correct file
- a4c6b886 - Split Notifications into notifications and StudentFeedbackNotifications"
- 3c81b0ff - Rename notificationcontroller test class to proper naming
- bf91c4c9 - Notify the student when the TA is not able to find him/her
- 78e37ba5 - Running spotless
Toggle commit list-
dc692555...c1281d52 - 803 commits from branch
added 25 commits
-
78e37ba5...dcc79553 - 22 commits from branch
development - bf0f2b42 - Merge branch 'development' into 242-notification-asking-students-for-feedback
- c032f250 - Remove duplicate test file
- 7c9bbac2 - Add comments to created tests for notifications
Toggle commit list-
78e37ba5...dcc79553 - 22 commits from branch
added 12 commits
- 3cc4364f - Do not send notifications to TA's if a new request is created
- 9add3790 - Refactor code for sending table updates to TA
- 72423469 - Send push notification to the student
- 8247782e - Add urls to notification such that clicking on them brings you to a usefull page
- 9445c460 - Create test for feedback notifications
- 7b0c7848 - Split Notifications into notifications and StudentFeedbackNotifications"
- 9c087c83 - Notify the student when the TA is not able to find him/her
- 8edd3c53 - Add comments to created tests for notifications
- f0cea6ea - Allow notifications to also be send to all group members.
- 601d2d28 - Let request service use the correct method for creating the not found notifications
- 675886f2 - Create a test class for notificationsservice.
- ef260985 - Stop tests from being flakey!
Toggle commit list115 String url = "/request/" + request.getId(); 116 Map<User, Notification> notifications = new HashMap<>(); 117 for (User user : entity.getUsers()) { 118 notifications.put(user, new StudentFeedbackNotification(user, request.getLab().getCourse(), 119 "Please provide feedback", 120 "Please provide feedback for your TA for request: queue.tudelft.nl/", 1000, url)); 121 } 122 return notifications; 123 } 124 125 public Map<User, Notification> createNotFoundNotification(RequestEntity requestEntity, Course course) { 126 Map<User, Notification> notifications = new HashMap<>(); 127 for (User user : requestEntity.getUsers()) { 128 notifications.put(user, new Notification(user, 129 course, "Not Found", 130 "The TA was not able to find you. Please check if you filled in the correct room!", 100)); If it is unclear where to go because Queue doesn't say where to go and the TA only presses get next for his/her own room, then a suggestion about checking your room would make sense. I think the bigger problem here is that Queue actually doesn't tell you where the TA is and we should let the TA enter this information for every lab the first time get-next is pressed probably. Perhaps combine that with the idea of clickable rooms? #196 (closed)
- Resolved by Otto Visser
Am I correct if I say that at the moment there is no code yet that actually actively generates a notification to provide feedback? If so: make an issue that this code still needs to be added, if not: where is it and how did I miss it
- Last reply by Cédric Willekens
88 88 - column: 89 89 name: question 90 90 type: text 91 92 - changeSet: 93 - id: 3 94 - author: "Cedric Willekens" 95 - changes: 96 - addColumn: 97 tableName: NOTIFICATION 98 columns: 99 - column: 100 name: url 150 151 notificationService.sendRequestTableUpdateForAllAssistants(request); 151 152 } 152 153 153 private void notifyNewRequest(Request request, User assistant) { 154 @Transactional 155 void notifyNewRequest(Request request, User assistant) { -
@LiamClark What's your opinion on this?
-
The changes to the notification here won't be written to the database, at least at first glance. The notification is created in createNotification. At that point it's a "detached" object. It only exists in memory and not in the database yet.
Calling save on those objects is what safe is for, to introduce a newly created object into a persistence context. Objects that are already associated to a persistence context, those created by a repository query, have their changes flushed by when the transaction commits.
That's however not the case here so we should keep save. Although we need to change our usage of save in other cases, I suggest not doing so yet until we have decided with the entire team on how to so.
added 263 commits
-
268cc04d...9ee7848c - 249 commits from branch
development - 68108004 - Do not send notifications to TA's if a new request is created
- 44ced5e4 - Refactor code for sending table updates to TA
- e00e6f55 - Send push notification to the student
- 120a21bf - Add urls to notification such that clicking on them brings you to a usefull page
- fe253d8a - Create test for feedback notifications
- 6807ea01 - Split Notifications into notifications and StudentFeedbackNotifications"
- 1c40b75b - Notify the student when the TA is not able to find him/her
- f86b475f - Add comments to created tests for notifications
- 2344302c - Allow notifications to also be send to all group members.
- 52ae9a9c - Let request service use the correct method for creating the not found notifications
- bf4106d7 - Create a test class for notificationsservice.
- 8cf4898d - Stop tests from being flakey!
- c1973f0d - Add javadoc
- 0b316134 - Remove StudentFeedBackNotification class.
Toggle commit list-
268cc04d...9ee7848c - 249 commits from branch
added 1 commit
- 79206320 - Make feedback asking notification java 11 ready
You seem to have a license violation in
NotificationServiceTestandNotificationControllerTestwhich is currently failing the build: https://gitlab.ewi.tudelft.nl/eip/labrador/queue/-/jobs/898476Edited by Chris Lemaireadded 38 commits
-
24477e6f...ebb6b0bc - 36 commits from branch
development - f83ec913 - Licence formatting
- ea69f911 - Merge branch 'development' into 242-notification-asking-students-for-feedback
-
24477e6f...ebb6b0bc - 36 commits from branch
mentioned in merge request !289 (merged)
unassigned @LiamClark
