From 94befdd502e619313534664393f4c72a24c9ebec Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Wed, 7 Mar 2012 00:13:43 -0800 Subject: [PATCH] Events improved. Open/close issue, merge request events displayed --- app/controllers/issues_controller.rb | 2 +- app/controllers/merge_requests_controller.rb | 2 +- app/models/activity_observer.rb | 17 +++++++++-- app/models/event.rb | 30 ++++++++++++++----- app/models/issue.rb | 1 + app/models/merge_request.rb | 1 + app/models/project.rb | 3 +- app/views/events/_event.html.haml | 6 +++- .../events/_event_changed_issue.html.haml | 14 +++++++++ .../_event_changed_merge_request.html.haml | 19 ++++++++++++ app/views/events/_event_push.html.haml | 14 +++++---- .../20120307095918_add_author_id_to_event.rb | 5 ++++ db/schema.rb | 3 +- 13 files changed, 98 insertions(+), 19 deletions(-) create mode 100644 app/views/events/_event_changed_issue.html.haml create mode 100644 app/views/events/_event_changed_merge_request.html.haml create mode 100644 db/migrate/20120307095918_add_author_id_to_event.rb diff --git a/app/controllers/issues_controller.rb b/app/controllers/issues_controller.rb index 4e3be259..53d8b74d 100644 --- a/app/controllers/issues_controller.rb +++ b/app/controllers/issues_controller.rb @@ -76,7 +76,7 @@ class IssuesController < ApplicationController end def update - @issue.update_attributes(params[:issue]) + @issue.update_attributes(params[:issue].merge(:author_id_of_changes => current_user.id)) respond_to do |format| format.js diff --git a/app/controllers/merge_requests_controller.rb b/app/controllers/merge_requests_controller.rb index 1cf75876..9d5a401d 100644 --- a/app/controllers/merge_requests_controller.rb +++ b/app/controllers/merge_requests_controller.rb @@ -87,7 +87,7 @@ class MergeRequestsController < ApplicationController def update respond_to do |format| - if @merge_request.update_attributes(params[:merge_request]) + if @merge_request.update_attributes(params[:merge_request].merge(:author_id_of_changes => current_user.id)) format.html { redirect_to [@project, @merge_request], notice: 'Merge request was successfully updated.' } format.json { head :ok } else diff --git a/app/models/activity_observer.rb b/app/models/activity_observer.rb index 46564161..324d8207 100644 --- a/app/models/activity_observer.rb +++ b/app/models/activity_observer.rb @@ -1,12 +1,25 @@ class ActivityObserver < ActiveRecord::Observer - observe :issue, :merge_request, :note + observe :issue, :merge_request def after_create(record) Event.create( :project => record.project, :target_id => record.id, :target_type => record.class.name, - :action => Event.determine_action(record) + :action => Event.determine_action(record), + :author_id => record.author_id ) end + + def after_save(record) + if record.changed.include?("closed") + Event.create( + :project => record.project, + :target_id => record.id, + :target_type => record.class.name, + :action => (record.closed ? Event::Closed : Event::Reopened), + :author_id => record.author_id_of_changes + ) + end + end end diff --git a/app/models/event.rb b/app/models/event.rb index c5c08df6..8ab127c0 100644 --- a/app/models/event.rb +++ b/app/models/event.rb @@ -1,4 +1,6 @@ class Event < ActiveRecord::Base + default_scope where("author_id IS NOT NULL") + Created = 1 Updated = 2 Closed = 3 @@ -26,13 +28,22 @@ class Event < ActiveRecord::Base # - new issue # - merge request def allowed? - push? || new_issue? || new_merge_request? + push? || new_issue? || new_merge_request? || + changed_merge_request? || changed_issue? end def push? action == self.class::Pushed end + def closed? + action == self.class::Closed + end + + def reopened? + action == self.class::Reopened + end + def new_tag? data[:ref]["refs/tags"] end @@ -57,10 +68,6 @@ class Event < ActiveRecord::Base @tag_name ||= data[:ref].gsub("refs/tags/", "") end - def pusher - User.find_by_id(data[:user_id]) - end - def new_issue? target_type == "Issue" && action == Created @@ -71,6 +78,16 @@ class Event < ActiveRecord::Base action == Created end + def changed_merge_request? + target_type == "MergeRequest" && + [Closed, Reopened].include?(action) + end + + def changed_issue? + target_type == "Issue" && + [Closed, Reopened].include?(action) + end + def issue target if target_type == "Issue" end @@ -80,7 +97,7 @@ class Event < ActiveRecord::Base end def author - target.author + @author ||= User.find(author_id) end def commits @@ -89,7 +106,6 @@ class Event < ActiveRecord::Base end end - delegate :id, :name, :email, :to => :pusher, :prefix => true, :allow_nil => true delegate :name, :email, :to => :author, :prefix => true, :allow_nil => true delegate :title, :to => :issue, :prefix => true, :allow_nil => true delegate :title, :to => :merge_request, :prefix => true, :allow_nil => true diff --git a/app/models/issue.rb b/app/models/issue.rb index aa0a2acc..4210297c 100644 --- a/app/models/issue.rb +++ b/app/models/issue.rb @@ -5,6 +5,7 @@ class Issue < ActiveRecord::Base has_many :notes, :as => :noteable, :dependent => :destroy attr_protected :author, :author_id, :project, :project_id + attr_accessor :author_id_of_changes validates_presence_of :project_id validates_presence_of :assignee_id diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb index 8af46242..97872724 100644 --- a/app/models/merge_request.rb +++ b/app/models/merge_request.rb @@ -5,6 +5,7 @@ class MergeRequest < ActiveRecord::Base has_many :notes, :as => :noteable, :dependent => :destroy attr_protected :author, :author_id, :project, :project_id + attr_accessor :author_id_of_changes validates_presence_of :project_id validates_presence_of :assignee_id diff --git a/app/models/project.rb b/app/models/project.rb index d9a9284e..5c1d7946 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -68,7 +68,8 @@ class Project < ActiveRecord::Base Event.create( :project => self, :action => Event::Pushed, - :data => data + :data => data, + :author_id => Key.find_by_identifier(author_key_id).user.id ) end diff --git a/app/views/events/_event.html.haml b/app/views/events/_event.html.haml index 23a8601b..f091fff5 100644 --- a/app/views/events/_event.html.haml +++ b/app/views/events/_event.html.haml @@ -2,7 +2,11 @@ .event_feed - if event.new_issue? = render "events/event_new_issue", :event => event - - if event.new_merge_request? + - elsif event.new_merge_request? = render "events/event_new_merge_request", :event => event + - elsif event.changed_merge_request? + = render "events/event_changed_merge_request", :event => event + - elsif event.changed_issue? + = render "events/event_changed_issue", :event => event - elsif event.push? = render "events/event_push", :event => event diff --git a/app/views/events/_event_changed_issue.html.haml b/app/views/events/_event_changed_issue.html.haml new file mode 100644 index 00000000..65cfebbd --- /dev/null +++ b/app/views/events/_event_changed_issue.html.haml @@ -0,0 +1,14 @@ += image_tag gravatar_icon(event.author_email), :class => "avatar" +%strong #{event.author_name} +- if event.closed? + closed +- else + reopened +issue += link_to project_issue_path(event.project, event.issue) do + %strong= truncate event.issue_title +at +%strong= link_to event.project.name, event.project +%span.cgray + = time_ago_in_words(event.created_at) + ago. diff --git a/app/views/events/_event_changed_merge_request.html.haml b/app/views/events/_event_changed_merge_request.html.haml new file mode 100644 index 00000000..a55e609c --- /dev/null +++ b/app/views/events/_event_changed_merge_request.html.haml @@ -0,0 +1,19 @@ += image_tag gravatar_icon(event.author_email), :class => "avatar" +%strong #{event.author_name} +- if event.closed? + closed +- else + reopened +merge request += link_to project_merge_request_path(event.project, event.merge_request) do + %strong= truncate event.merge_request_title +at +%strong= link_to event.project.name, event.project +%span.cgray + = time_ago_in_words(event.created_at) + ago. +%br +%span.label= event.merge_request.source_branch +→ +%span.label= event.merge_request.target_branch + diff --git a/app/views/events/_event_push.html.haml b/app/views/events/_event_push.html.haml index b48429f4..c59b482f 100644 --- a/app/views/events/_event_push.html.haml +++ b/app/views/events/_event_push.html.haml @@ -1,6 +1,6 @@ - if event.new_branch? || event.new_tag? - = image_tag gravatar_icon(event.pusher_email), :class => "avatar" - %strong #{event.pusher_name} + = image_tag gravatar_icon(event.author_email), :class => "avatar" + %strong #{event.author_name} pushed new - if event.new_tag? tag @@ -16,8 +16,8 @@ = time_ago_in_words(event.created_at) ago. - else - = image_tag gravatar_icon(event.pusher_email), :class => "avatar" - %strong #{event.pusher_name} + = image_tag gravatar_icon(event.author_email), :class => "avatar" + %strong #{event.author_name} pushed to = link_to project_commits_path(event.project, :ref => event.branch_name) do %strong= event.branch_name @@ -31,6 +31,10 @@ Compare #{event.commits.first.commit.id[0..8]}...#{event.commits.last.id[0..8]} - @project = event.project %ul.unstyled - = render event.commits + - if event.commits.size > 4 + = render event.commits[0..2] + %li ... and #{event.commits.size - 3} more commits + - else + = render event.commits diff --git a/db/migrate/20120307095918_add_author_id_to_event.rb b/db/migrate/20120307095918_add_author_id_to_event.rb new file mode 100644 index 00000000..1d24d41e --- /dev/null +++ b/db/migrate/20120307095918_add_author_id_to_event.rb @@ -0,0 +1,5 @@ +class AddAuthorIdToEvent < ActiveRecord::Migration + def change + add_column :events, :author_id, :integer, :null => true + end +end diff --git a/db/schema.rb b/db/schema.rb index 8838b219..45e18a68 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -11,7 +11,7 @@ # # It's strongly recommended to check this file into your version control system. -ActiveRecord::Schema.define(:version => 20120301185805) do +ActiveRecord::Schema.define(:version => 20120307095918) do create_table "events", :force => true do |t| t.string "target_type" @@ -22,6 +22,7 @@ ActiveRecord::Schema.define(:version => 20120301185805) do t.datetime "created_at", :null => false t.datetime "updated_at", :null => false t.integer "action" + t.integer "author_id" end create_table "issues", :force => true do |t|