Merge branch 'issue_3359_3' into 'master'

CHANGELOG

@@ -13,6 +13,7 @@ v 8.10.0 (unreleased)
   - Fix pagination when sorting by columns with lots of ties (like priority)
   - Exclude email check from the standard health check
   - Fix changing issue state columns in milestone view
+  - Add notification settings dropdown for groups
   - Fix user creation with stronger minimum password requirements !4054 (nathan-pmt)
   - PipelinesFinder uses git cache data
   - Check for conflicts with existing Project's wiki path when creating a new project.

app/assets/javascripts/dispatcher.js.coffee

@@ -84,6 +84,8 @@ class Dispatcher
         new Activities()
       when 'groups:show'
         shortcut_handler = new ShortcutsNavigation()
+        new NotificationsForm()
+        new NotificationsDropdown()
       when 'groups:group_members:index'
         new GroupMembers()
         new UsersSelect()

app/assets/stylesheets/framework/blocks.scss

@@ -137,7 +137,7 @@
     margin: 0;
     font-size: 24px;
     font-weight: normal;
-    margin-bottom: 5px;
+    margin-bottom: 10px;
     color: #4c4e54;
     font-size: 23px;
     line-height: 1.1;

app/assets/stylesheets/pages/groups.scss

@@ -48,11 +48,7 @@
 
   .access-request-button {
     @include btn-gray;
-    position: absolute;
-    right: 16px;
-    bottom: 32px;
-    padding: 3px 10px;
+    margin-right: 10px;
     text-transform: none;
-    background-color: $background-color;
   }
 }

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
-
-    @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?
+    if current_user
+      @last_push            = current_user.recent_push
+      @notification_setting = current_user.notification_settings_for(group)
+    end
 
-    @shared_projects = GroupProjectsFinder.new(group, only_shared: true).execute(current_user)
+    setup_projects
 
     respond_to do |format|
       format.html
@@ -97,6 +94,16 @@ 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 authorize_create_group!
     unless can?(current_user, :create_group, nil)
       return render_404

app/controllers/notification_settings_controller.rb

@@ -2,11 +2,9 @@ class NotificationSettingsController < ApplicationController
   before_action :authenticate_user!
 
   def create
-    project = Project.find(params[:project][:id])
+    return render_404 unless can_read?(resource)
 
-    return render_404 unless can?(current_user, :read_project, project)
-
-    @notification_setting = current_user.notification_settings_for(project)
+    @notification_setting = current_user.notification_settings_for(resource)
     @saved = @notification_setting.update_attributes(notification_setting_params)
 
     render_response
@@ -21,6 +19,22 @@ class NotificationSettingsController < ApplicationController
 
   private
 
+  def resource
+    @resource ||=
+      if params[:project_id].present?
+        Project.find(params[:project_id])
+      elsif params[:namespace_id].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("shared/notifications/_button", notification_setting: @notification_setting),

app/helpers/notifications_helper.rb

@@ -72,6 +72,6 @@ module NotificationsHelper
   # Create hidden field to send notification setting source to controller
   def hidden_setting_source_input(notification_setting)
     return unless notification_setting.source_type
-    hidden_field_tag "#{notification_setting.source_type.downcase}[id]", notification_setting.source_id
+    hidden_field_tag "#{notification_setting.source_type.downcase}_id", notification_setting.source_id
   end
 end

app/views/groups/show.html.haml

@@ -15,12 +15,17 @@
           %span.visibility-icon.has-tooltip{ data: { container: 'body' }, title: visibility_icon_description(@group) }
             = visibility_level_icon(@group.visibility_level, fw: false)
 
+        %span.hidden-xs
+          = render 'shared/notifications/button', notification_setting: @notification_setting
+
+        - if current_user
+          .pull-right
+            = render 'shared/members/access_request_buttons', source: @group
+
       - if @group.description.present?
         .cover-desc.description
           = markdown(@group.description, pipeline: :description)
 
-    - if current_user
-      = render 'shared/members/access_request_buttons', source: @group
 
 %div{ class: container_class }
   .top-area

doc/workflow/notifications.md

@@ -37,12 +37,14 @@ This means that you can set a different level of notifications per group while s
 to have a finer level setting per project.
 Organization like this is suitable for users that belong to different groups but don't have the
 same need for being notified for every group they are member of.
+These settings can be configured on group page or user profile notifications dropdown.
 
 #### Project Settings
 
 Project Settings are at the top level and any setting placed at this level will take precedence of any
 other setting.
 This is suitable for users that have different needs for notifications per project basis.
+These settings can be configured on project page or user profile notifications dropdown.
 
 ## Notification events
 

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
@@ -12,7 +13,7 @@ describe NotificationSettingsController do
     context 'when not authorized' do
       it 'redirects to sign in page' do
         post :create,
-             project: { id: project.id },
+             project_id: project.id,
              notification_setting: { level: :participating }
 
         expect(response).to redirect_to(new_user_session_path)
@@ -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
@@ -57,7 +98,7 @@ describe NotificationSettingsController do
 
       it 'returns 404' do
         post :create,
-             project: { id: private_project.id },
+             project_id: private_project.id,
              notification_setting: { level: :participating }
 
         expect(response).to have_http_status(404)