Commit 05c9ab94c2377e0e0b7933c15fb91d11f8295441
Exists in
spb-stable
and in
3 other branches
Merge branch 'refactor-author-id-of-changes' into 'master'
Remove author_id_of_changes To prevent confusion because we already have `current_user`
Showing
12 changed files
with
12 additions
and
28 deletions
Show diff stats
app/controllers/projects/issues_controller.rb
| @@ -76,7 +76,7 @@ class Projects::IssuesController < Projects::ApplicationController | @@ -76,7 +76,7 @@ class Projects::IssuesController < Projects::ApplicationController | ||
| 76 | end | 76 | end |
| 77 | 77 | ||
| 78 | def update | 78 | def update |
| 79 | - @issue.update_attributes(params[:issue].merge(author_id_of_changes: current_user.id)) | 79 | + @issue.update_attributes(params[:issue]) |
| 80 | @issue.reset_events_cache | 80 | @issue.reset_events_cache |
| 81 | 81 | ||
| 82 | respond_to do |format| | 82 | respond_to do |format| |
app/controllers/projects/merge_requests_controller.rb
| @@ -109,7 +109,7 @@ class Projects::MergeRequestsController < Projects::ApplicationController | @@ -109,7 +109,7 @@ class Projects::MergeRequestsController < Projects::ApplicationController | ||
| 109 | params[:merge_request].delete(:source_project_id) | 109 | params[:merge_request].delete(:source_project_id) |
| 110 | params[:merge_request].delete(:target_project_id) | 110 | params[:merge_request].delete(:target_project_id) |
| 111 | 111 | ||
| 112 | - if @merge_request.update_attributes(params[:merge_request].merge(author_id_of_changes: current_user.id)) | 112 | + if @merge_request.update_attributes(params[:merge_request]) |
| 113 | @merge_request.reset_events_cache | 113 | @merge_request.reset_events_cache |
| 114 | 114 | ||
| 115 | respond_to do |format| | 115 | respond_to do |format| |
app/controllers/projects/milestones_controller.rb
| @@ -38,7 +38,6 @@ class Projects::MilestonesController < Projects::ApplicationController | @@ -38,7 +38,6 @@ class Projects::MilestonesController < Projects::ApplicationController | ||
| 38 | 38 | ||
| 39 | def create | 39 | def create |
| 40 | @milestone = @project.milestones.new(params[:milestone]) | 40 | @milestone = @project.milestones.new(params[:milestone]) |
| 41 | - @milestone.author_id_of_changes = current_user.id | ||
| 42 | 41 | ||
| 43 | if @milestone.save | 42 | if @milestone.save |
| 44 | redirect_to project_milestone_path(@project, @milestone) | 43 | redirect_to project_milestone_path(@project, @milestone) |
| @@ -48,7 +47,7 @@ class Projects::MilestonesController < Projects::ApplicationController | @@ -48,7 +47,7 @@ class Projects::MilestonesController < Projects::ApplicationController | ||
| 48 | end | 47 | end |
| 49 | 48 | ||
| 50 | def update | 49 | def update |
| 51 | - @milestone.update_attributes(params[:milestone].merge(author_id_of_changes: current_user.id)) | 50 | + @milestone.update_attributes(params[:milestone]) |
| 52 | 51 | ||
| 53 | respond_to do |format| | 52 | respond_to do |format| |
| 54 | format.js | 53 | format.js |
app/models/concerns/issuable.rb
app/models/issue.rb
| @@ -30,8 +30,7 @@ class Issue < ActiveRecord::Base | @@ -30,8 +30,7 @@ class Issue < ActiveRecord::Base | ||
| 30 | scope :of_user_team, ->(team) { where(project_id: team.project_ids, assignee_id: team.member_ids) } | 30 | scope :of_user_team, ->(team) { where(project_id: team.project_ids, assignee_id: team.member_ids) } |
| 31 | 31 | ||
| 32 | attr_accessible :title, :assignee_id, :position, :description, | 32 | attr_accessible :title, :assignee_id, :position, :description, |
| 33 | - :milestone_id, :label_list, :author_id_of_changes, | ||
| 34 | - :state_event | 33 | + :milestone_id, :label_list, :state_event |
| 35 | 34 | ||
| 36 | acts_as_taggable_on :labels | 35 | acts_as_taggable_on :labels |
| 37 | 36 |
app/models/merge_request.rb
| @@ -38,7 +38,7 @@ class MergeRequest < ActiveRecord::Base | @@ -38,7 +38,7 @@ class MergeRequest < ActiveRecord::Base | ||
| 38 | 38 | ||
| 39 | delegate :commits, :diffs, :last_commit, :last_commit_short_sha, to: :merge_request_diff, prefix: nil | 39 | delegate :commits, :diffs, :last_commit, :last_commit_short_sha, to: :merge_request_diff, prefix: nil |
| 40 | 40 | ||
| 41 | - attr_accessible :title, :assignee_id, :source_project_id, :source_branch, :target_project_id, :target_branch, :milestone_id, :author_id_of_changes, :state_event, :description | 41 | + attr_accessible :title, :assignee_id, :source_project_id, :source_branch, :target_project_id, :target_branch, :milestone_id, :state_event, :description |
| 42 | 42 | ||
| 43 | attr_accessor :should_remove_source_branch | 43 | attr_accessor :should_remove_source_branch |
| 44 | 44 |
app/models/milestone.rb
| @@ -16,8 +16,7 @@ | @@ -16,8 +16,7 @@ | ||
| 16 | class Milestone < ActiveRecord::Base | 16 | class Milestone < ActiveRecord::Base |
| 17 | include InternalId | 17 | include InternalId |
| 18 | 18 | ||
| 19 | - attr_accessible :title, :description, :due_date, :state_event, :author_id_of_changes | ||
| 20 | - attr_accessor :author_id_of_changes | 19 | + attr_accessible :title, :description, :due_date, :state_event |
| 21 | 20 | ||
| 22 | belongs_to :project | 21 | belongs_to :project |
| 23 | has_many :issues | 22 | has_many :issues |
| @@ -89,6 +88,6 @@ class Milestone < ActiveRecord::Base | @@ -89,6 +88,6 @@ class Milestone < ActiveRecord::Base | ||
| 89 | end | 88 | end |
| 90 | 89 | ||
| 91 | def author_id | 90 | def author_id |
| 92 | - author_id_of_changes | 91 | + nil |
| 93 | end | 92 | end |
| 94 | end | 93 | end |
app/observers/activity_observer.rb
| @@ -2,8 +2,6 @@ class ActivityObserver < BaseObserver | @@ -2,8 +2,6 @@ class ActivityObserver < BaseObserver | ||
| 2 | observe :issue, :note, :milestone | 2 | observe :issue, :note, :milestone |
| 3 | 3 | ||
| 4 | def after_create(record) | 4 | def after_create(record) |
| 5 | - event_author_id = record.author_id | ||
| 6 | - | ||
| 7 | if record.kind_of?(Note) | 5 | if record.kind_of?(Note) |
| 8 | # Skip system notes, like status changes and cross-references. | 6 | # Skip system notes, like status changes and cross-references. |
| 9 | return true if record.system? | 7 | return true if record.system? |
| @@ -12,9 +10,7 @@ class ActivityObserver < BaseObserver | @@ -12,9 +10,7 @@ class ActivityObserver < BaseObserver | ||
| 12 | return true if record.noteable_type.blank? | 10 | return true if record.noteable_type.blank? |
| 13 | end | 11 | end |
| 14 | 12 | ||
| 15 | - if event_author_id | ||
| 16 | - create_event(record, Event.determine_action(record)) | ||
| 17 | - end | 13 | + create_event(record, Event.determine_action(record)) if current_user |
| 18 | end | 14 | end |
| 19 | 15 | ||
| 20 | def after_close(record, transition) | 16 | def after_close(record, transition) |
app/services/merge_requests/auto_merge_service.rb
| @@ -9,11 +9,10 @@ module MergeRequests | @@ -9,11 +9,10 @@ module MergeRequests | ||
| 9 | merge_request.lock | 9 | merge_request.lock |
| 10 | 10 | ||
| 11 | if Gitlab::Satellite::MergeAction.new(current_user, merge_request).merge!(commit_message) | 11 | if Gitlab::Satellite::MergeAction.new(current_user, merge_request).merge!(commit_message) |
| 12 | - merge_request.author_id_of_changes = current_user.id | ||
| 13 | merge_request.merge | 12 | merge_request.merge |
| 14 | 13 | ||
| 15 | notification.merge_mr(merge_request, current_user) | 14 | notification.merge_mr(merge_request, current_user) |
| 16 | - create_merge_event(merge_request) | 15 | + create_merge_event(merge_request, current_user) |
| 17 | execute_project_hooks(merge_request) | 16 | execute_project_hooks(merge_request) |
| 18 | 17 | ||
| 19 | true | 18 | true |
app/services/merge_requests/base_merge_service.rb
| @@ -7,13 +7,13 @@ module MergeRequests | @@ -7,13 +7,13 @@ module MergeRequests | ||
| 7 | NotificationService.new | 7 | NotificationService.new |
| 8 | end | 8 | end |
| 9 | 9 | ||
| 10 | - def create_merge_event(merge_request) | 10 | + def create_merge_event(merge_request, current_user) |
| 11 | Event.create( | 11 | Event.create( |
| 12 | project: merge_request.target_project, | 12 | project: merge_request.target_project, |
| 13 | target_id: merge_request.id, | 13 | target_id: merge_request.id, |
| 14 | target_type: merge_request.class.name, | 14 | target_type: merge_request.class.name, |
| 15 | action: Event::MERGED, | 15 | action: Event::MERGED, |
| 16 | - author_id: merge_request.author_id_of_changes | 16 | + author_id: current_user.id |
| 17 | ) | 17 | ) |
| 18 | end | 18 | end |
| 19 | 19 |
app/services/merge_requests/merge_service.rb
| @@ -7,11 +7,10 @@ module MergeRequests | @@ -7,11 +7,10 @@ module MergeRequests | ||
| 7 | # to target branch | 7 | # to target branch |
| 8 | class MergeService < BaseMergeService | 8 | class MergeService < BaseMergeService |
| 9 | def execute(merge_request, current_user, commit_message) | 9 | def execute(merge_request, current_user, commit_message) |
| 10 | - merge_request.author_id_of_changes = current_user.id | ||
| 11 | merge_request.merge | 10 | merge_request.merge |
| 12 | 11 | ||
| 13 | notification.merge_mr(merge_request, current_user) | 12 | notification.merge_mr(merge_request, current_user) |
| 14 | - create_merge_event(merge_request) | 13 | + create_merge_event(merge_request, current_user) |
| 15 | execute_project_hooks(merge_request) | 14 | execute_project_hooks(merge_request) |
| 16 | 15 | ||
| 17 | true | 16 | true |
spec/models/concerns/issuable_spec.rb
| @@ -25,11 +25,6 @@ describe Issue, "Issuable" do | @@ -25,11 +25,6 @@ describe Issue, "Issuable" do | ||
| 25 | it { described_class.should respond_to(:assigned) } | 25 | it { described_class.should respond_to(:assigned) } |
| 26 | end | 26 | end |
| 27 | 27 | ||
| 28 | - it "has an :author_id_of_changes accessor" do | ||
| 29 | - issue.should respond_to(:author_id_of_changes) | ||
| 30 | - issue.should respond_to(:author_id_of_changes=) | ||
| 31 | - end | ||
| 32 | - | ||
| 33 | describe ".search" do | 28 | describe ".search" do |
| 34 | let!(:searchable_issue) { create(:issue, title: "Searchable issue") } | 29 | let!(:searchable_issue) { create(:issue, title: "Searchable issue") } |
| 35 | 30 |