ReviewWanted Notifications Reasons

Problem Statement & Solution Document for "ReviewWanted Notifications Reasons"

Reality?

WebUI

• Notifications are for all users.
• Users can configure where they want to receive their Notifications (Web UI, RSS Feeds, email, ...).
• Every time a ReviewWanted Event is raised, a new Notification (with event_type = Event::ReviewWanted) is created.
• Users can receive Notifications for ReviewWanted Events if they are somehow involved.
• Involved users are those who are required to do a review (directly or as member of a group/project/package).
• The new notification replaces all the previous notifications regarding the same BsRequest. So refreshing the view, you'll see a BsRequest Notification with the information updated (date, state, users involved including the new reviewers).
• The Request Notification created by a ReviewWanted events displays avatars of all the users/groups/projects/packages that are required as reviewers and also the BsRequest creator.
• A maximum of 6 avatars are displayed, the rest are "hidden" behind a (+N).

RSS Feeds

• RSS Feeds are for all users.
• RSS Feeds' items come from notifications.
• Notifications are built from events.
• Users can configure where they want to receive their notifications (Web UI, RSS Feeds, email, ...).
• A RSS Feeds item related to a ReviewWanted event, contains information about:
  • who requires the review (review creator/who)
  • his/her reason (review reason/comment)
  • why that user received the notification: by user, by group, by project, by package.

The information about the reason is retrieved from a field (event_payload) that is going to disappear.

Consequences?

WebUI

Even if it can be deducted from the avatars, there isn't a clear way to distinguish why the user received the ReviewWanted notification.

What's even worst, the avatar that corresponds to the user (user/group/project/package avatar) can be hidden since the view displays a maximum of 6 avatars.

RSS Feeds

Apparently, there is no issue on the user side. However, there will be an issue as soon as we remove the event_payload field from Notifications. Once this happens, we won't be able to match every single BsRequest Notification (coming from ReviewWanted) with its corresponding Review. So we'll miss information like: by user/group/project/package, review creator/who, review reason/comment.

Future?

WebUI

The WebUI displays clearly why the user received the notification. Either because he/she was required as a reviewer, or because he/she is a member of a group/project/package that has been required as a reviewer.

RSS Feeds

We can get rid of Notification's event_payload field and still be able to display information about the review (by user/group/project/package, review creator/who, review reason/comment) on every RSS Feeds item.

Proposal!

• Add attribute review to model Notification. This will require:

  • to create a migration to add a field review_id to table notifications,
  • in the WebUI, when displaying the BsRequest notification, we have to check if the event_type is Event::ReviewWanted. In this case, we have to add the review details (by user/group/project/package).

• In RSS Feeds, we'll use the review_id value to retrieve the review details (by user/group/project/package, review creator/who, review reason/comment).

• What to do with the existent data?

  • no data migration, just display review details from now on, if the BsRequest Notification has a review_id.

Declined proposals

• Make Review a notifiable, back. This will require:
  • a migration of data, to change some BsRequest notifications to Review notifications.
  • in the WebUI, to group by BsRequest notifications and its Reviews notifications, when showing a row related to a request.

Regarding the proposal, we had two options about migrations, the following is discarded:

• create data migration to fill the review_id field in already-existent notifications. However, getting the review_id corresponding to each BsRequest notification is not straightforward. We should take all the reviews associated with a BsRequest that match the fields in the event_payload, what can not be very accurate.