Skip to content

Commit

Permalink
Add ability to mark multiple notifications as read/unread
Browse files Browse the repository at this point in the history
When dealing with a big amount of notifications, it is very
time consuming to mark them one by one.
Therefore a way to update multiple notifications at once
is required.

Co-authored-by: Eduardo Navarro <enavarro@suse.com>
  • Loading branch information
krauselukas and eduardoj committed Oct 1, 2020
1 parent d1f2cdd commit 0004b34
Show file tree
Hide file tree
Showing 9 changed files with 139 additions and 57 deletions.
1 change: 1 addition & 0 deletions src/api/app/assets/javascripts/webui/application.js
Original file line number Diff line number Diff line change
Expand Up @@ -52,3 +52,4 @@
//= require webui/kiwi_editor.js
//= require webui/monitor.js
//= require webui/responsive_ux
//= require webui/notification.js
32 changes: 32 additions & 0 deletions src/api/app/assets/javascripts/webui/notification.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
function setSelectAllCheckbox() {
$('#select-all-notifications').change(function() {
var checkboxes = $(this).closest('form').find('input[type=checkbox]');
checkboxes.prop('checked', $(this).is(':checked'));
});
}

function setCheckboxCounterAndSubmitButton() {
var amountBoxesChecked = 0;
$("input[id^='notification_ids_']").each(function() {
if($(this).is(':checked')){
amountBoxesChecked += 1;
}
});

if(amountBoxesChecked <= 0) {
$('#done-button').addClass('disabled');
$('#select-all-label').text('Select all');
} else {
$('#done-button').removeClass('disabled');
$('#select-all-label').text(amountBoxesChecked + ' selected');
}
}

function handleNotificationCheckboxSelection() { // jshint ignore:line
setCheckboxCounterAndSubmitButton();
setSelectAllCheckbox();
$('input[type="checkbox"]').change(function() {
setCheckboxCounterAndSubmitButton();
});
}

Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
// TODO: As soon as notifications_redesign feature is 100% rolled out,
// all the 'body.notifications-redesign' wrappers can be removed.
$notifications-filter-box-height: 3.5rem;

body.notifications-redesign {
.avatars-counter, .simulated-avatar {
Expand All @@ -26,6 +27,9 @@ body.notifications-redesign {
&.show { border-top: 1px solid $gray-300;}
}
}
#notification-action-bar {
&.sticky-top { top: $top-navigation-height; }
}
}

@include media-breakpoint-up(md) {
Expand All @@ -41,13 +45,20 @@ body.notifications-redesign {
#notifications-filter-desktop {
&.show { border-top: 1px solid $gray-300; }
&.sticky-top { top: $top-navigation-height; }
height: $notifications-filter-box-height;
// To not overlap with the notification action bar
z-index: calc(#{$zindex-sticky} + 1);

#filters {
max-height: 100vw; overflow: auto;
-webkit-box-shadow: 2px 3px 5px rgba(0,0,0,.2);
-moz-box-shadow: 2px 3px 5px rgba(0,0,0,.2);
box-shadow: 2px 3px 5px rgba(0,0,0,.2);
}
}
#notification-action-bar {
&.sticky-top { top: calc(#{$top-navigation-height} + #{$notifications-filter-box-height}); }
}
}

@media (orientation: landscape) {
Expand Down
31 changes: 17 additions & 14 deletions src/api/app/controllers/webui/users/notifications_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -7,26 +7,25 @@ class Webui::Users::NotificationsController < Webui::WebuiController
before_action :check_feature_toggle

def index
@notifications = fetch_notifications
@notifications = paginated_notifications
@filtered_project = Project.find_by(name: params[:project])
@notifications_filter = notifications_filter
end

def update
notification = User.session.notifications.find(params[:id])
authorize notification, policy_class: NotificationPolicy
notifications = fetch_notifications.where(id: params[:notification_ids])

if notification.toggle(:delivered).save
flash.now[:success] = "Successfully marked the notification as #{notification.unread? ? 'unread' : 'read'}"
else
flash.now[:error] = "Couldn't mark the notification as #{notification.unread? ? 'read' : 'unread'}"
# rubocop:disable Rails/SkipsModelValidations
unless notifications.update_all('delivered = !delivered')
flash.now[:error] = "Couldn't mark the notifications as #{notifications.first.unread? ? 'read' : 'unread'}"
end
# rubocop:enable Rails/SkipsModelValidations

respond_to do |format|
format.html { redirect_to my_notifications_path }
format.js do
render partial: 'update', locals: {
notifications: fetch_notifications,
notifications: paginated_notifications,
notifications_filter: notifications_filter
}
end
Expand Down Expand Up @@ -74,12 +73,16 @@ def notifications_count
end

def fetch_notifications
notifications_for_subscribed_user = User.session.notifications.for_web
notifications = if params[:project]
NotificationsFinder.new(notifications_for_subscribed_user).for_project_name(params[:project])
else
NotificationsFinder.new(notifications_for_subscribed_user).for_notifiable_type(params[:type])
end
notifications_for_subscribed_user = NotificationsFinder.new(policy_scope(Notification))
if params[:project]
notifications_for_subscribed_user.for_project_name(params[:project])
else
notifications_for_subscribed_user.for_notifiable_type(params[:type])
end
end

def paginated_notifications
notifications = fetch_notifications
params[:page] = notifications.page(params[:page]).total_pages if notifications.page(params[:page]).out_of_range?
params[:show_all] ? show_all(notifications) : notifications.page(params[:page])
end
Expand Down
7 changes: 4 additions & 3 deletions src/api/app/policies/notification_policy.rb
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
class NotificationPolicy < ApplicationPolicy
def update?
return true if record.subscriber_type == 'User' && record.subscriber_id == user.id
return true if record.subscriber_type == 'Group' && record.subscriber_id.include?(user.groups.ids)
class Scope < Scope
def resolve
NotificationsFinder.new(scope).for_subscribed_user(user)
end
end
end
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
.card.sticky-top#notification-action-bar
.card-body.py-3
.row.px-0.px-md-1
.col
.custom-control.custom-checkbox
= check_box_tag('select-all-notifications', 1, false, class: 'custom-control-input')
%label.custom-control-label.align-middle.mr-4#select-all-label{ for: 'select-all-notifications' } Select All
- button_text = type == 'read' ? "Mark as 'Unread'" : "Mark as 'Read'"
= button_tag(type: 'submit', class: 'btn btn-sm btn-outline-success px-3 disabled', id: 'done-button') do
= button_text
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
.card
.card-body
- if notifications.empty?
- if notifications.empty?
.card
.card-body
%p
- case selected_filter[:type]
- when 'reviews', 'comments', 'requests'
Expand All @@ -9,40 +9,56 @@
Mark notifications as "Read" and they'll appear here
- else
There are no notifications, but there's a world of opportunities!
- else
.text-center
%span.ml-3= page_entries_info notifications, entry_name: 'notification'
= link_to_all unless notifications.total_pages == 1 && params[:show_all].nil?
- else
- update_path = my_notifications_path(type: selected_filter[:type], project: selected_filter[:project],
page: params[:page], show_all: params[:show_all])
= form_tag(update_path, method: :put, remote: true) do
= render partial: 'notification_action_bar', locals: { type: selected_filter[:type] }
.card
.card-body
.text-center
%span.ml-3= page_entries_info notifications, entry_name: 'notification'
= link_to_all unless notifications.total_pages == 1 && params['show_all'].nil?
.list-group.list-group-flush.mt-3
- notifications.each do |n|
- notification = NotificationPresenter.new(n)
.list-group-item.px-0.px-md-1.py-3
.row
.col-auto.pr-0
.custom-control.custom-checkbox
= check_box_tag('notification_ids[]', notification.id, false,
id: "notification_ids_#{notification.id}", class: 'custom-control-input')
= label_tag("notification_ids_#{notification.id}", '', class: 'custom-control-label')
.col
.row
.col
- if notification.notifiable_type == 'BsRequest'
= image_tag('icons/request-icon.svg', height: 18, title: 'Request notification')
= link_to(notification.notifiable_link[:text], notification.notifiable_link[:path], class: 'mx-1')
%small.text-nowrap #{time_ago_in_words(notification.created_at)} ago
%span.badge.ml-1{ class: "badge badge-#{request_badge_color(notification.notifiable.state)}" }
= notification.notifiable.state
- else
%i.fas.fa-comments{ title: 'Comment notification' }
= link_to(notification.notifiable_link[:text], notification.notifiable_link[:path], class: 'mx-1')
%small.text-nowrap #{time_ago_in_words(notification.created_at)} ago
.col-auto.actions.ml-auto.align-self-end.align-self-md-start
- title, icon = notification.unread? ? ['Mark as "Read"', 'fa-check'] : ['Mark as "Unread"', 'fa-undo']
- update_path = my_notifications_path(notification_ids: [notification.id], type: selected_filter[:type],
project: selected_filter[:project], page: params[:page],
show_all: params[:show_all])
= link_to(update_path, id: format('update-notification-%d', notification.id),
method: :put, class: 'btn btn-sm btn-outline-success px-3', title: title, remote: true) do
%i.fas{ class: "#{icon}" }
.row.mt-1.pl-sm-4
.col-auto.pr-0
= render partial: 'notification_avatars', locals: { avatar_objects: notification.avatar_objects }
.col-auto.pl-xs-2
%span.text-word-break-all= action_description(notification)
.row.d-none.d-md-block.pl-4
.col
%p.mt-3.mb-0= notification.excerpt
= paginate notifications, views_prefix: 'webui', window: 2, params: { action: 'index', id: nil }

.list-group.list-group-flush.mt-3
- notifications.each do |n|
- notification = NotificationPresenter.new(n)
.list-group-item.px-0.px-md-1.py-3.container
.row
.col
- if notification.notifiable_type == 'BsRequest'
= image_tag('icons/request-icon.svg', height: 18, title: 'Request notification')
= link_to(notification.notifiable_link[:text], notification.notifiable_link[:path], class: 'mx-1')
%small.text-nowrap #{time_ago_in_words(notification.created_at)} ago
%span.badge.ml-1{ class: "badge badge-#{request_badge_color(notification.notifiable.state)}" }
= notification.notifiable.state
- else
%i.fas.fa-comments{ title: 'Comment notification' }
= link_to(notification.notifiable_link[:text], notification.notifiable_link[:path], class: 'mx-1')
%small.text-nowrap #{time_ago_in_words(notification.created_at)} ago
.col-auto.actions.ml-auto.align-self-end.align-self-md-start
- title, icon = notification.unread? ? ['Mark as "Read"', 'fa-check'] : ['Mark as "Unread"', 'fa-undo']
- update_path = my_notification_path(id: notification, type: selected_filter[:type], project: selected_filter[:project],
page: params[:page], show_all: params[:show_all])
= link_to(update_path, id: format('update-notification-%d', notification.id),
method: :put, class: 'btn btn-sm btn-outline-success px-3', title: title, remote: true) do
%i.fas{ class: "#{icon}" }
.row.mt-1.pl-sm-4
.col-auto.pr-0
= render partial: 'notification_avatars', locals: { avatar_objects: notification.avatar_objects }
.col-auto.pl-xs-2
%span.text-word-break-all= action_description(notification)
.row.d-none.d-md-block.pl-4
.col
%p.mt-3.mb-0= notification.excerpt
= paginate notifications, views_prefix: 'webui', window: 2, params: { action: 'index', id: nil }
- content_for :ready_function do
handleNotificationCheckboxSelection();
1 change: 1 addition & 0 deletions src/api/app/views/webui/users/notifications/_update.js.erb
Original file line number Diff line number Diff line change
Expand Up @@ -2,3 +2,4 @@ $('#filters').html("<%= j(render partial: 'notifications_filter', locals: { filt
$('#notifications-list').html("<%= j(render partial: 'notifications_list', locals: { notifications: notifications, selected_filter: notifications_filter.selected_filter }) %>");
$('#flash').html("<%= escape_javascript(render(layout: false, partial: 'layouts/webui/flash', object: flash)) %>");
$('#notifications-counter').html("<%= escape_javascript(render partial: 'layouts/webui/responsive_ux/unread_notifications_counter') %>");
$(document).ready(function() { handleNotificationCheckboxSelection(); });
9 changes: 8 additions & 1 deletion src/api/config/routes/webui_routes.rb
Original file line number Diff line number Diff line change
Expand Up @@ -306,7 +306,14 @@
scope :my do
resources :tasks, only: [:index], controller: 'webui/users/tasks', as: :my_tasks

resources :notifications, only: [:index, :update], controller: 'webui/users/notifications', as: :my_notifications
resources :notifications, only: [:index], controller: 'webui/users/notifications', as: :my_notifications do
collection do
# We allow updating multiple notifications in a single HTTP request
put :update
end
end

resource :notification, only: [:update], controller: 'webui/users/notifications', as: :my_notification

resources :subscriptions, only: [:index], controller: 'webui/users/subscriptions', as: :my_subscriptions do
collection do
Expand Down

0 comments on commit 0004b34

Please sign in to comment.