Commit f36f0dac9d2a009f29d2253dcd7c66d5a46ffd56
1 parent
7a4c9588
Exists in
master
and in
4 other branches
Consolidate functionality shared between Issue and MergeRequest
Any associations, validations, delegates, scopes and methods that were exactly the same in both Issue and MergeRequest models have been moved to a new IssueCommonality module (role) that gets included by each class. There was actually quite a bit of duplication, because MergeRequests are basically just specialized Issues.
Showing
3 changed files
with
52 additions
and
86 deletions
Show diff stats
app/models/issue.rb
| ... | ... | @@ -4,58 +4,17 @@ class Issue < ActiveRecord::Base |
| 4 | 4 | |
| 5 | 5 | acts_as_taggable_on :labels |
| 6 | 6 | |
| 7 | - belongs_to :project | |
| 8 | 7 | belongs_to :milestone |
| 9 | - belongs_to :author, :class_name => "User" | |
| 10 | - belongs_to :assignee, :class_name => "User" | |
| 11 | - has_many :notes, :as => :noteable, :dependent => :destroy | |
| 12 | - | |
| 13 | - attr_protected :author, :author_id, :project, :project_id | |
| 14 | - attr_accessor :author_id_of_changes | |
| 15 | - | |
| 16 | - validates_presence_of :project_id | |
| 17 | - validates_presence_of :author_id | |
| 18 | - | |
| 19 | - delegate :name, | |
| 20 | - :email, | |
| 21 | - :to => :author, | |
| 22 | - :prefix => true | |
| 23 | - | |
| 24 | - delegate :name, | |
| 25 | - :email, | |
| 26 | - :to => :assignee, | |
| 27 | - :allow_nil => true, | |
| 28 | - :prefix => true | |
| 29 | - | |
| 30 | - validates :title, | |
| 31 | - :presence => true, | |
| 32 | - :length => { :within => 0..255 } | |
| 33 | 8 | |
| 34 | 9 | validates :description, |
| 35 | 10 | :length => { :within => 0..2000 } |
| 36 | 11 | |
| 37 | - scope :opened, where(:closed => false) | |
| 38 | - scope :closed, where(:closed => true) | |
| 39 | - scope :assigned, lambda { |u| where(:assignee_id => u.id)} | |
| 40 | - | |
| 41 | 12 | acts_as_list |
| 42 | 13 | |
| 43 | 14 | def self.open_for(user) |
| 44 | 15 | opened.assigned(user) |
| 45 | 16 | end |
| 46 | 17 | |
| 47 | - def self.search query | |
| 48 | - where("title like :query", :query => "%#{query}%") | |
| 49 | - end | |
| 50 | - | |
| 51 | - def today? | |
| 52 | - Date.today == created_at.to_date | |
| 53 | - end | |
| 54 | - | |
| 55 | - def new? | |
| 56 | - today? && created_at == updated_at | |
| 57 | - end | |
| 58 | - | |
| 59 | 18 | def is_assigned? |
| 60 | 19 | !!assignee_id |
| 61 | 20 | end | ... | ... |
app/models/merge_request.rb
| ... | ... | @@ -10,47 +10,15 @@ class MergeRequest < ActiveRecord::Base |
| 10 | 10 | CAN_BE_MERGED = 2 |
| 11 | 11 | CANNOT_BE_MERGED = 3 |
| 12 | 12 | |
| 13 | - belongs_to :project | |
| 14 | - belongs_to :author, :class_name => "User" | |
| 15 | - belongs_to :assignee, :class_name => "User" | |
| 16 | - has_many :notes, :as => :noteable, :dependent => :destroy | |
| 17 | - | |
| 18 | 13 | serialize :st_commits |
| 19 | 14 | serialize :st_diffs |
| 20 | 15 | |
| 21 | - attr_protected :author, :author_id, :project, :project_id | |
| 22 | - attr_accessor :author_id_of_changes, | |
| 23 | - :should_remove_source_branch | |
| 16 | + attr_accessor :should_remove_source_branch | |
| 24 | 17 | |
| 25 | - validates_presence_of :project_id | |
| 26 | - validates_presence_of :author_id | |
| 27 | 18 | validates_presence_of :source_branch |
| 28 | 19 | validates_presence_of :target_branch |
| 29 | 20 | validate :validate_branches |
| 30 | 21 | |
| 31 | - delegate :name, | |
| 32 | - :email, | |
| 33 | - :to => :author, | |
| 34 | - :prefix => true | |
| 35 | - | |
| 36 | - delegate :name, | |
| 37 | - :email, | |
| 38 | - :to => :assignee, | |
| 39 | - :allow_nil => true, | |
| 40 | - :prefix => true | |
| 41 | - | |
| 42 | - validates :title, | |
| 43 | - :presence => true, | |
| 44 | - :length => { :within => 0..255 } | |
| 45 | - | |
| 46 | - scope :opened, where(:closed => false) | |
| 47 | - scope :closed, where(:closed => true) | |
| 48 | - scope :assigned, lambda { |u| where(:assignee_id => u.id)} | |
| 49 | - | |
| 50 | - def self.search query | |
| 51 | - where("title like :query", :query => "%#{query}%") | |
| 52 | - end | |
| 53 | - | |
| 54 | 22 | def self.find_all_by_branch(branch_name) |
| 55 | 23 | where("source_branch like :branch or target_branch like :branch", :branch => branch_name) |
| 56 | 24 | end |
| ... | ... | @@ -96,14 +64,6 @@ class MergeRequest < ActiveRecord::Base |
| 96 | 64 | self.save |
| 97 | 65 | end |
| 98 | 66 | |
| 99 | - def today? | |
| 100 | - Date.today == created_at.to_date | |
| 101 | - end | |
| 102 | - | |
| 103 | - def new? | |
| 104 | - today? && created_at == updated_at | |
| 105 | - end | |
| 106 | - | |
| 107 | 67 | def diffs |
| 108 | 68 | st_diffs || [] |
| 109 | 69 | end |
| ... | ... | @@ -136,7 +96,7 @@ class MergeRequest < ActiveRecord::Base |
| 136 | 96 | commits.first |
| 137 | 97 | end |
| 138 | 98 | |
| 139 | - def merged? | |
| 99 | + def merged? | |
| 140 | 100 | merged && merge_event |
| 141 | 101 | end |
| 142 | 102 | |
| ... | ... | @@ -153,7 +113,7 @@ class MergeRequest < ActiveRecord::Base |
| 153 | 113 | end |
| 154 | 114 | |
| 155 | 115 | def probably_merged? |
| 156 | - unmerged_commits.empty? && | |
| 116 | + unmerged_commits.empty? && | |
| 157 | 117 | commits.any? && open? |
| 158 | 118 | end |
| 159 | 119 | |
| ... | ... | @@ -171,8 +131,8 @@ class MergeRequest < ActiveRecord::Base |
| 171 | 131 | self.update_attributes :state => CANNOT_BE_MERGED |
| 172 | 132 | end |
| 173 | 133 | |
| 174 | - def reloaded_commits | |
| 175 | - if open? && unmerged_commits.any? | |
| 134 | + def reloaded_commits | |
| 135 | + if open? && unmerged_commits.any? | |
| 176 | 136 | self.st_commits = unmerged_commits |
| 177 | 137 | save |
| 178 | 138 | end | ... | ... |
app/roles/issue_commonality.rb
| 1 | 1 | # Contains common functionality shared between Issues and MergeRequests |
| 2 | 2 | module IssueCommonality |
| 3 | + extend ActiveSupport::Concern | |
| 4 | + | |
| 5 | + included do | |
| 6 | + attr_protected :author, :author_id, :project, :project_id | |
| 7 | + | |
| 8 | + belongs_to :project | |
| 9 | + belongs_to :author, :class_name => "User" | |
| 10 | + belongs_to :assignee, :class_name => "User" | |
| 11 | + has_many :notes, :as => :noteable, :dependent => :destroy | |
| 12 | + | |
| 13 | + validates_presence_of :project_id | |
| 14 | + validates_presence_of :author_id | |
| 15 | + | |
| 16 | + validates :title, | |
| 17 | + :presence => true, | |
| 18 | + :length => { :within => 0..255 } | |
| 19 | + | |
| 20 | + | |
| 21 | + scope :opened, where(:closed => false) | |
| 22 | + scope :closed, where(:closed => true) | |
| 23 | + scope :assigned, lambda { |u| where(:assignee_id => u.id)} | |
| 24 | + | |
| 25 | + delegate :name, | |
| 26 | + :email, | |
| 27 | + :to => :author, | |
| 28 | + :prefix => true | |
| 29 | + | |
| 30 | + delegate :name, | |
| 31 | + :email, | |
| 32 | + :to => :assignee, | |
| 33 | + :allow_nil => true, | |
| 34 | + :prefix => true | |
| 35 | + | |
| 36 | + attr_accessor :author_id_of_changes | |
| 37 | + end | |
| 38 | + | |
| 39 | + def self.search query | |
| 40 | + where("title like :query", :query => "%#{query}%") | |
| 41 | + end | |
| 42 | + | |
| 43 | + def today? | |
| 44 | + Date.today == created_at.to_date | |
| 45 | + end | |
| 46 | + | |
| 47 | + def new? | |
| 48 | + today? && created_at == updated_at | |
| 49 | + end | |
| 3 | 50 | end | ... | ... |