From 92e183542fe0e13930220ba3bbf67b9197cfc026 Mon Sep 17 00:00:00 2001 From: Felipe Artur Date: Tue, 21 Jun 2016 16:50:13 -0300 Subject: Insert notification settings dropdown into groups --- CHANGELOG | 3 + app/assets/javascripts/dispatcher.js.coffee | 1 + app/controllers/groups_controller.rb | 31 ++++++---- .../notification_settings_controller.rb | 22 ++++++- app/views/groups/show.html.haml | 3 + .../notifications/buttons/_notifications.html.haml | 2 +- .../notification_settings_controller_spec.rb | 69 +++++++++++++++++----- 7 files changed, 102 insertions(+), 29 deletions(-) diff --git a/CHANGELOG b/CHANGELOG index 3dfb88f212a..65f9043e91b 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -1,4 +1,6 @@ Please view this file on the master branch, on stable branches it's out of date. +v 8.10(unreleased) + - Add notifications dropdown for groups v 8.9.0 (unreleased) - Fix Error 500 when using closes_issues API with an external issue tracker @@ -29,6 +31,7 @@ v 8.9.0 (unreleased) - Use gitlab-shell v3.0.0 - Upgrade to jQuery 2 - Use Knapsack to evenly distribute tests across multiple nodes + - Add notifications dropdown for groups - Add `sha` parameter to MR merge API, to ensure only reviewed changes are merged - Don't allow MRs to be merged when commits were added since the last review / page load - Add DB index on users.state diff --git a/app/assets/javascripts/dispatcher.js.coffee b/app/assets/javascripts/dispatcher.js.coffee index 404101710b9..a4a309810c5 100644 --- a/app/assets/javascripts/dispatcher.js.coffee +++ b/app/assets/javascripts/dispatcher.js.coffee @@ -81,6 +81,7 @@ class Dispatcher new Activities() when 'groups:show' shortcut_handler = new ShortcutsNavigation() + new NotificationsForm() when 'groups:group_members:index' new GroupMembers() new UsersSelect() diff --git a/app/controllers/groups_controller.rb b/app/controllers/groups_controller.rb index ee4fcc4e360..f65f9da3f9e 100644 --- a/app/controllers/groups_controller.rb +++ b/app/controllers/groups_controller.rb @@ -37,15 +37,12 @@ class GroupsController < Groups::ApplicationController end def show - @last_push = current_user.recent_push if current_user + if current_user + @last_push = current_user.recent_push + @notification_setting = current_user.notification_settings_for(group) + end - @projects = @projects.includes(:namespace) - @projects = @projects.sorted_by_activity - @projects = filter_projects(@projects) - @projects = @projects.sort(@sort = params[:sort]) - @projects = @projects.page(params[:page]) if params[:filter_projects].blank? - - @shared_projects = GroupProjectsFinder.new(group, only_shared: true).execute(current_user) + setup_projects respond_to do |format| format.html @@ -77,10 +74,6 @@ class GroupsController < Groups::ApplicationController def edit end - def projects - @projects = @group.projects.page(params[:page]) - end - def update if Groups::UpdateService.new(@group, current_user, group_params).execute redirect_to edit_group_path(@group), notice: "Group '#{@group.name}' was successfully updated." @@ -97,6 +90,20 @@ class GroupsController < Groups::ApplicationController protected + def setup_projects + @projects = @projects.includes(:namespace) + @projects = @projects.sorted_by_activity + @projects = filter_projects(@projects) + @projects = @projects.sort(@sort = params[:sort]) + @projects = @projects.page(params[:page]) if params[:filter_projects].blank? + + @shared_projects = GroupProjectsFinder.new(group, only_shared: true).execute(current_user) + end + + def projects + @projects = @group.projects.page(params[:page]) + end + def authorize_create_group! unless can?(current_user, :create_group, nil) return render_404 diff --git a/app/controllers/notification_settings_controller.rb b/app/controllers/notification_settings_controller.rb index 5d425ad8420..735e562a497 100644 --- a/app/controllers/notification_settings_controller.rb +++ b/app/controllers/notification_settings_controller.rb @@ -2,9 +2,11 @@ class NotificationSettingsController < ApplicationController before_action :authenticate_user! def create - project = current_user.projects.find(params[:project][:id]) + resource = find_resource - @notification_setting = current_user.notification_settings_for(project) + return render_404 unless can_read?(resource) + + @notification_setting = current_user.notification_settings_for(resource) @saved = @notification_setting.update_attributes(notification_setting_params) render_response @@ -19,6 +21,22 @@ class NotificationSettingsController < ApplicationController private + def find_resource + resource = + if params[:project].present? + Project.find(params[:project][:id]) + elsif params[:namespace].present? + Group.find(params[:namespace][:id]) + end + end + + def can_read?(resource) + ability_name = resource.class.name.downcase + ability_name = "read_#{ability_name}".to_sym + + can?(current_user, ability_name, resource) + end + def render_response render json: { html: view_to_html_string("notifications/buttons/_notifications", notification_setting: @notification_setting), diff --git a/app/views/groups/show.html.haml b/app/views/groups/show.html.haml index 85635bc4616..ffcf8ca5dc0 100644 --- a/app/views/groups/show.html.haml +++ b/app/views/groups/show.html.haml @@ -15,10 +15,13 @@ %span.visibility-icon.has-tooltip{ data: { container: 'body' }, title: visibility_icon_description(@group) } = visibility_level_icon(@group.visibility_level, fw: false) + = render 'notifications/buttons/notifications', notification_setting: @notification_setting + - if @group.description.present? .cover-desc.description = markdown(@group.description, pipeline: :description) + %div{ class: container_class } .top-area %ul.nav-links diff --git a/app/views/notifications/buttons/_notifications.html.haml b/app/views/notifications/buttons/_notifications.html.haml index 5e81e6c96b9..9aaa2890bdd 100644 --- a/app/views/notifications/buttons/_notifications.html.haml +++ b/app/views/notifications/buttons/_notifications.html.haml @@ -16,7 +16,7 @@ %button.dropdown-new.btn.btn-default.notifications-btn#notifications-button{ type: "button", data: { toggle: "modal", target: "#" + notifications_menu_identifier("modal", notification_setting) } } = icon("bell", class: "js-notification-loading") = notification_title(notification_setting.level) - %button.btn.btn-danger.dropdown-toggle{ data: { toggle: "dropdown", target: notifications_menu_identifier("dropdown", notification_setting) } } + %button.btn.dropdown-toggle{ data: { toggle: "dropdown", target: notifications_menu_identifier("dropdown", notification_setting) } } %span.caret .sr-only Toggle dropdown = render "shared/notifications/notification_dropdown", notification_setting: notification_setting diff --git a/spec/controllers/notification_settings_controller_spec.rb b/spec/controllers/notification_settings_controller_spec.rb index 15d155833b4..ad48b9e9087 100644 --- a/spec/controllers/notification_settings_controller_spec.rb +++ b/spec/controllers/notification_settings_controller_spec.rb @@ -2,6 +2,7 @@ require 'spec_helper' describe NotificationSettingsController do let(:project) { create(:empty_project) } + let(:group) { create(:group, :internal) } let(:user) { create(:user) } before do @@ -20,33 +21,73 @@ describe NotificationSettingsController do end context 'when authorized' do + let(:custom_events) do + events = {} + + NotificationSetting::EMAIL_EVENTS.each do |event| + events[event.to_s] = true + end + + events + end + before do sign_in(user) end - it 'returns success' do - post :create, - project: { id: project.id }, - notification_setting: { level: :participating } + context 'for projects' do + let(:notification_setting) { user.notification_settings_for(project) } - expect(response.status).to eq 200 - end + it 'creates notification setting' do + post :create, + project: { id: project.id }, + notification_setting: { level: :participating } - context 'and setting custom notification setting' do - let(:custom_events) do - events = {} + expect(response.status).to eq 200 + expect(notification_setting.level).to eq("participating") + expect(notification_setting.user_id).to eq(user.id) + expect(notification_setting.source_id).to eq(project.id) + expect(notification_setting.source_type).to eq("Project") + end - NotificationSetting::EMAIL_EVENTS.each do |event| - events[event] = "true" + context 'with custom settings' do + it 'creates notification setting' do + post :create, + project: { id: project.id }, + notification_setting: { level: :custom }.merge(custom_events) + + expect(response.status).to eq 200 + expect(notification_setting.level).to eq("custom") + expect(notification_setting.events).to eq(custom_events) end end + end - it 'returns success' do + context 'for groups' do + let(:notification_setting) { user.notification_settings_for(group) } + + it 'creates notification setting' do post :create, - project: { id: project.id }, - notification_setting: { level: :participating, events: custom_events } + namespace: { id: group.id }, + notification_setting: { level: :watch } expect(response.status).to eq 200 + expect(notification_setting.level).to eq("watch") + expect(notification_setting.user_id).to eq(user.id) + expect(notification_setting.source_id).to eq(group.id) + expect(notification_setting.source_type).to eq("Namespace") + end + + context 'with custom settings' do + it 'creates notification setting' do + post :create, + namespace: { id: group.id }, + notification_setting: { level: :custom }.merge(custom_events) + + expect(response.status).to eq 200 + expect(notification_setting.level).to eq("custom") + expect(notification_setting.events).to eq(custom_events) + end end end end -- cgit v1.2.1