Convert serialized field to has_many relation

Author: root

Diff for: app/controllers/repository_protected_branches_controller.rb

@@ -14,8 +14,7 @@ def index
 
 
   def new
-    @protected_branch = @repository.protected_branches.new()
-    @protected_branch.user_list  = []
+    @protected_branch = @repository.protected_branches.new
   end
 
 

Diff for: app/helpers/repository_protected_branches_helper.rb

@@ -0,0 +1,7 @@
+module RepositoryProtectedBranchesHelper
+
+  def protected_branch_available_users
+    @project.member_principals.map(&:user).uniq.sort
+  end
+
+end

Diff for: app/models/protected_branches_user.rb

@@ -0,0 +1,7 @@
+class ProtectedBranchesUser < ActiveRecord::Base
+  unloadable
+
+  ## Relations
+  belongs_to :protected_branch, class_name: 'RepositoryProtectedBranche'
+  belongs_to :user
+end

Diff for: app/models/repository_protected_branche.rb

@@ -7,29 +7,21 @@ class RepositoryProtectedBranche < ActiveRecord::Base
   acts_as_list
 
   ## Attributes
-  attr_accessible :path, :permissions, :position, :user_list
+  attr_accessible :path, :permissions, :position, :user_ids
 
   ## Relations
   belongs_to :repository
+  has_many   :protected_branches_users, foreign_key: :protected_branch_id, dependent: :destroy
+  has_many   :users, through: :protected_branches_users
 
   ## Validations
   validates :repository_id, presence: true
   validates :path,          presence: true, uniqueness: { scope: [:permissions, :repository_id] }
   validates :permissions,   presence: true, inclusion: { in: VALID_PERMS }
-  validates :user_list,     presence: true
-
-  ## Serializations
-  serialize :user_list, Array
-
-  ## Callbacks
-  before_validation :remove_blank_items
 
   ## Scopes
   default_scope { order('position ASC') }
 
-  ## Delegation
-  delegate :project, to: :repository
-
 
   class << self
 
@@ -44,21 +36,8 @@ def clone_from(parent)
   end
 
 
-  def available_users
-    project.member_principals.map(&:user).compact.uniq.map{ |u| u.login }.sort
-  end
-
-
   def allowed_users
-    user_list.map{|u| User.find_by_login(u).gitolite_identifier}.sort
+    users.map { |u| u.gitolite_identifier }.sort
   end
 
-
-  private
-
-
-    def remove_blank_items
-      self.user_list = user_list.select{|u| !u.blank?} rescue []
-    end
-
 end

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

@@ -4,18 +4,14 @@
   %p= f.text_field :path, required: true, size: 65, label: l(:label_branch_path)
   %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_list][]", ""
+  = hidden_field_tag 'repository_protected_branche[user_ids][]', ''
 
   %p
-    %label{ for: "repository_protected_branche[user_list]" }= l(:label_user_list) + ' :'
+    %label{ for: 'repository_protected_branche[user_ids]' }= l(:label_user_list) + ' :'
 
-  = tag_it_list 'repository_protected_branche_user_list',
-                { name: 'repository_protected_branche[user_list][]' },
-                { source: 'user_list', placeholder: '+ add user' } do
-    - if @protected_branch.user_list.any?
-      - @protected_branch.user_list.each do |item|
-        %li= item
-
-:javascript
-  var user_list = #{raw @protected_branch.available_users.to_json};
-  $(document).ready(function(){ setTagIt(); });
+  - 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

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

@@ -35,7 +35,7 @@
             %td{ style: 'font-family: Consolas;' }= protected_branch.path
 
             %td
-              - protected_branch.user_list.each do |user|
+              - protected_branch.users.each do |user|
                 %span{ class: 'label label-info' }= user
 
             %td{ class: 'buttons' }

Diff for: db/migrate/20150823030000_create_protected_branches_users.rb

@@ -0,0 +1,16 @@
+class CreateProtectedBranchesUsers < ActiveRecord::Migration
+
+  def self.up
+    create_table :protected_branches_users do |t|
+      t.column :protected_branch_id, :integer
+      t.column :user_id,             :integer
+    end
+
+    add_index :protected_branches_users, [:protected_branch_id, :user_id], unique: true, name: 'unique__protected_branch_id_user_id'
+  end
+
+  def self.down
+    drop_table :protected_branches_users
+  end
+
+end

Diff for: db/migrate/20150823030100_migrate_protected_branches_users.rb

@@ -0,0 +1,24 @@
+class RepositoryProtectedBrancheWrapped < RepositoryProtectedBranche
+  serialize :user_list, Array
+end
+
+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
+    end
+    remove_column :repository_protected_branches, :user_list
+  end
+
+  def self.down
+    add_column :repository_protected_branches, :user_list, :text, after: :permissions
+    RepositoryProtectedBrancheWrapped.all.each do |protected_branch|
+      users = protected_branch.users.map { |u| u.login }.compact.uniq
+      protected_branch.user_list = users
+      protected_branch.save!
+    end
+  end
+
+end

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

@@ -14,6 +14,8 @@ 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
 
           # Callbacks
           after_save :check_if_status_changed

Diff for: spec/models/repository_protected_branche_spec.rb

@@ -9,7 +9,7 @@ def build_protected_branch(opts = {})
 
   describe "Valid RepositoryProtectedBranche creation" do
     before(:each) do
-      @protected_branch = build_protected_branch(:path => 'devel', :permissions => 'RW', :user_list => %w(user1 user2))
+      @protected_branch = build_protected_branch(path: 'devel', permissions: 'RW')
     end
 
     subject { @protected_branch }
@@ -18,24 +18,22 @@ def build_protected_branch(opts = {})
     it { should allow_mass_assignment_of(:path) }
     it { should allow_mass_assignment_of(:permissions) }
     it { should allow_mass_assignment_of(:position) }
-    it { should allow_mass_assignment_of(:user_list) }
+    it { should allow_mass_assignment_of(:user_ids) }
 
     ## 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) }
 
     ## Validations
     it { should be_valid }
 
     it { should validate_presence_of(:repository_id) }
     it { should validate_presence_of(:path) }
     it { should validate_presence_of(:permissions) }
-    it { should validate_presence_of(:user_list) }
 
     it { should validate_uniqueness_of(:path).scoped_to([:permissions, :repository_id]) }
 
     it { should validate_inclusion_of(:permissions).in_array(%w(RW RW+)) }
-
-    ## Serializations
-    it { should serialize(:user_list) }
   end
 end

Diff for: spec/models/user_spec.rb

@@ -3,16 +3,16 @@
 describe User do
 
   before(:each) do
-    @user = create(:user)
+    @user = build(:user)
   end
 
   subject { @user }
 
-  it { should be_valid }
-
-  it { should respond_to(:gitolite_identifier) }
+  it { should have_many(:protected_branches_users).dependent(:destroy) }
+  it { should have_many(:protected_branches).through(:protected_branches_users) }
 
   it "has a gitolite_identifier" do
-    expect(@user.gitolite_identifier).to match(/redmine_user\d+_\d+/)
+    user = create(:user)
+    expect(user.gitolite_identifier).to match(/redmine_user\d+_\d+/)
   end
 end