Handle Redmine groups in ProtectedBranches

Author: root

Diff for: app/helpers/repository_protected_branches_helper.rb

@@ -1,7 +1,11 @@
 module RepositoryProtectedBranchesHelper
 
   def protected_branch_available_users
-    @project.member_principals.map(&:user).uniq.sort
+    @project.member_principals.map(&:principal).select { |m| m.class.name == 'User' }.uniq.sort
+  end
+
+  def protected_branch_available_groups
+    @project.member_principals.map(&:principal).select { |m| m.class.name == 'Group' }.uniq.sort
   end
 
 end

Diff for: app/models/protected_branches_member.rb

@@ -0,0 +1,23 @@
+class ProtectedBranchesMember < ActiveRecord::Base
+  unloadable
+
+  ## Relations
+  belongs_to :protected_branch, class_name: 'RepositoryProtectedBranche'
+  belongs_to :principal
+
+  ## Callbacks
+  after_destroy :remove_dependent_objects
+
+
+  private
+
+
+    def remove_dependent_objects
+      return unless principal.class.name == 'Group'
+      principal.users.each do |user|
+        member = self.class.find_by_principal_id_and_inherited_by(user.id, principal.id)
+        member.destroy! unless member.nil?
+      end
+    end
+
+end

Diff for: app/models/protected_branches_user.rb

@@ -7,12 +7,12 @@ class RepositoryProtectedBranche < ActiveRecord::Base
   acts_as_list
 
   ## Attributes
-  attr_accessible :path, :permissions, :position, :user_ids
+  attr_accessible :path, :permissions, :position, :user_ids, :group_ids
 
   ## Relations
   belongs_to :repository
-  has_many   :protected_branches_users, foreign_key: :protected_branch_id, dependent: :destroy
-  has_many   :users, through: :protected_branches_users
+  has_many   :protected_branches_members, foreign_key: :protected_branch_id, dependent: :destroy
+  has_many   :members, through: :protected_branches_members, source: :principal
 
   ## Validations
   validates :repository_id, presence: true
@@ -36,8 +36,80 @@ def clone_from(parent)
   end
 
 
+  # Accessors
+  #
+  def users
+    members.select { |m| m.class.name == 'User' }.uniq
+  end
+
+
+  def groups
+    members.select { |m| m.class.name == 'Group' }
+  end
+
+
   def allowed_users
     users.map { |u| u.gitolite_identifier }.sort
   end
 
+
+  # Mass assignment
+  #
+  def user_ids=(ids)
+    current_ids = users.map(&:id)
+    create_member(ids, current_ids, 'User')
+  end
+
+
+  def group_ids=(ids)
+    current_ids = groups.map(&:id)
+    create_member(ids, current_ids, 'Group') do |group|
+      ids = group.users.map(&:id)
+      current_ids = users_by_group_id(group.id).map(&:id)
+      create_member(ids, current_ids, 'User', inherited_by: group.id, destroy: false)
+    end
+  end
+
+
+  # Triggered by Group callbacks
+  #
+  def add_user_member(user, group)
+    ids = users_by_group_id(group.id).push(user).map(&:id)
+    current_ids = users_by_group_id(group.id).map(&:id)
+    create_member(ids, current_ids, 'User', inherited_by: group.id, destroy: false)
+  end
+
+
+  def remove_user_member(user, group)
+    return unless users_by_group_id(group.id).include?(user)
+    member = protected_branches_members.find_by_protected_branch_id_and_principal_id_and_inherited_by(id, user.id, group.id)
+    member.destroy! unless member.nil?
+  end
+
+
+  private
+
+
+    def users_by_group_id(id)
+      protected_branches_members.select { |pbm| pbm.principal.class.name == 'User' && pbm.inherited_by == id }.map(&:principal)
+    end
+
+
+    def create_member(ids, current_ids, klass, destroy: true, inherited_by: nil, &block)
+      ids = (ids || []).collect(&:to_i) - [0]
+      new_ids = ids - current_ids
+
+      new_ids.each do |id|
+        object = klass.constantize.find_by_id(id)
+        next if object.nil?
+        protected_branches_members.create(principal_id: object.id, inherited_by: inherited_by)
+        yield object if block_given?
+      end
+
+      if destroy
+        member_to_destroy = protected_branches_members.select { |m| m.principal.class.name == klass && !ids.include?(m.principal.id) }
+        member_to_destroy.each(&:destroy) if member_to_destroy.any?
+      end
+    end
+
 end

Diff for: app/models/repository_protected_branche.rb

@@ -5,13 +5,24 @@
   %p= f.select :permissions, options_for_select(RepositoryProtectedBranche::VALID_PERMS, @protected_branch.permissions), required: true, label: :label_permissions
 
   = hidden_field_tag 'repository_protected_branche[user_ids][]', ''
+  = hidden_field_tag 'repository_protected_branche[group_ids][]', ''
 
-  %p
-    %label{ for: 'repository_protected_branche[user_ids]' }= l(:label_user_list) + ' :'
+  .container
+    .row
+      .col-md-6
+        %h4= label_with_icon(l('label_user_plural'), 'fa-user')
+        - protected_branch_available_users.each do |user|
+          %p{ style: 'padding: 0px;' }
+            %label{ style: 'margin-left: 0; width: auto; font-weight: lighter;' }
+              = check_box_tag 'repository_protected_branche[user_ids][]', user.id, @protected_branch.users.include?(user)
+              = user
+            %br
 
-  - protected_branch_available_users.each do |user|
-    %p
-      %label{ style: 'margin-left: 0; width: auto; font-weight: lighter;' }
-        = check_box_tag 'repository_protected_branche[user_ids][]', user.id, @protected_branch.users.include?(user)
-        = user
-      %br
+      .col-md-6
+        %h4= label_with_icon(l('label_group_plural'), 'fa-users')
+        - protected_branch_available_groups.each do |group|
+          %p{ style: 'padding: 0px;' }
+            %label{ style: 'margin-left: 0; width: auto; font-weight: lighter;' }
+              = check_box_tag 'repository_protected_branche[group_ids][]', group.id, @protected_branch.groups.include?(group)
+              = group
+            %br

Diff for: app/views/repository_protected_branches/_form.html.haml

@@ -34,9 +34,7 @@
 
             %td{ style: 'font-family: Consolas;' }= protected_branch.path
 
-            %td
-              - protected_branch.users.each do |user|
-                %span{ class: 'label label-info' }= user
+            %td= protected_branch.users.map { |u| link_to(u) }.join(', ').html_safe
 
             %td{ class: 'buttons' }
               - if User.current.git_allowed_to?(:edit_repository_protected_branches, @repository)

Diff for: app/views/repository_protected_branches/index.html.haml

@@ -0,0 +1,17 @@
+class CreateProtectedBranchesMembers < ActiveRecord::Migration
+
+  def self.up
+    create_table :protected_branches_members do |t|
+      t.column :protected_branch_id, :integer
+      t.column :principal_id,        :integer
+      t.column :inherited_by,        :integer
+    end
+
+    add_index :protected_branches_members, [:protected_branch_id, :principal_id, :inherited_by], unique: true, name: 'unique_protected_branch_member'
+  end
+
+  def self.down
+    drop_table :protected_branches_members
+  end
+
+end

Diff for: db/migrate/20150823030000_create_protected_branches_members.rb

@@ -7,7 +7,7 @@ class MigrateProtectedBranchesUsers < ActiveRecord::Migration
   def self.up
     RepositoryProtectedBrancheWrapped.all.each do |protected_branch|
       users = protected_branch.user_list.map { |u| User.find_by_login(u) }.compact.uniq
-      protected_branch.users << users
+      protected_branch.user_ids = users.map(&:id)
     end
     remove_column :repository_protected_branches, :user_list
   end

Diff for: db/migrate/20150823030000_create_protected_branches_users.rb

@@ -0,0 +1,47 @@
+require_dependency 'group'
+
+module RedmineGitHosting
+  module Patches
+    module GroupPatch
+
+      def self.included(base)
+        base.send(:include, InstanceMethods)
+        base.class_eval do
+          unloadable
+
+          # Relations
+          has_many :protected_branches_members, dependent: :destroy, foreign_key: :principal_id
+          has_many :protected_branches, through: :protected_branches_members
+
+          alias_method_chain :user_added,   :git_hosting
+          alias_method_chain :user_removed, :git_hosting
+        end
+      end
+
+
+      module InstanceMethods
+
+        def user_added_with_git_hosting(user, &block)
+          user_added_without_git_hosting(user, &block)
+          protected_branches.each do |pb|
+            pb.add_user_member(user, self)
+          end
+        end
+
+
+        def user_removed_with_git_hosting(user, &block)
+          user_removed_without_git_hosting(user, &block)
+          protected_branches.each do |pb|
+            pb.remove_user_member(user, self)
+          end
+        end
+
+      end
+
+    end
+  end
+end
+
+unless Group.included_modules.include?(RedmineGitHosting::Patches::GroupPatch)
+  Group.send(:include, RedmineGitHosting::Patches::GroupPatch)
+end

Diff for: db/migrate/20150823030100_migrate_protected_branches_users.rb

@@ -14,8 +14,9 @@ def self.included(base)
 
           # Relations
           has_many :gitolite_public_keys, dependent: :destroy
-          has_many :protected_branches_users, dependent: :destroy
-          has_many :protected_branches, through: :protected_branches_users
+
+          has_many :protected_branches_members, dependent: :destroy, foreign_key: :principal_id
+          has_many :protected_branches, through: :protected_branches_members
 
           # Callbacks
           after_save :check_if_status_changed

Diff for: lib/redmine_git_hosting/patches/group_patch.rb

@@ -0,0 +1,7 @@
+FactoryGirl.define do
+
+  factory :group do |f|
+    f.sequence(:lastname)  { |n| "GroupTest#{n}" }
+  end
+
+end

Diff for: lib/redmine_git_hosting/patches/user_patch.rb

@@ -0,0 +1,13 @@
+require File.expand_path(File.dirname(__FILE__) + '/../spec_helper')
+
+describe Group do
+
+  before(:each) do
+    @group = build(:group)
+  end
+
+  subject { @group }
+
+  it { should have_many(:protected_branches_members).dependent(:destroy) }
+  it { should have_many(:protected_branches).through(:protected_branches_members) }
+end

Diff for: spec/factories/group.rb

@@ -22,8 +22,8 @@ def build_protected_branch(opts = {})
 
     ## Relations
     it { should belong_to(:repository) }
-    it { should have_many(:protected_branches_users).with_foreign_key(:protected_branch_id).dependent(:destroy) }
-    it { should have_many(:users).through(:protected_branches_users) }
+    it { should have_many(:protected_branches_members).with_foreign_key(:protected_branch_id).dependent(:destroy) }
+    it { should have_many(:members).through(:protected_branches_members) }
 
     ## Validations
     it { should be_valid }
@@ -34,6 +34,6 @@ def build_protected_branch(opts = {})
 
     it { should validate_uniqueness_of(:path).scoped_to([:permissions, :repository_id]) }
 
-    it { should validate_inclusion_of(:permissions).in_array(%w(RW RW+)) }
+    it { should validate_inclusion_of(:permissions).in_array RepositoryProtectedBranche::VALID_PERMS }
   end
 end

Diff for: spec/models/group_spec.rb

@@ -8,8 +8,8 @@
 
   subject { @user }
 
-  it { should have_many(:protected_branches_users).dependent(:destroy) }
-  it { should have_many(:protected_branches).through(:protected_branches_users) }
+  it { should have_many(:protected_branches_members).dependent(:destroy) }
+  it { should have_many(:protected_branches).through(:protected_branches_members) }
 
   it "has a gitolite_identifier" do
     user = create(:user)