Commit 9cf1f69cfa4e2e953f842e010573e76ec7486552
Exists in
master
and in
4 other branches
Merge branch 'feature/note_milestone_events'
Showing
13 changed files
with
124 additions
and
41 deletions
Show diff stats
app/assets/stylesheets/sections/events.scss
| @@ -31,7 +31,6 @@ | @@ -31,7 +31,6 @@ | ||
| 31 | * | 31 | * |
| 32 | */ | 32 | */ |
| 33 | .event-item { | 33 | .event-item { |
| 34 | - min-height: 40px; | ||
| 35 | border-bottom: 1px solid #eee; | 34 | border-bottom: 1px solid #eee; |
| 36 | .event-title { | 35 | .event-title { |
| 37 | color: #333; | 36 | color: #333; |
| @@ -50,14 +49,18 @@ | @@ -50,14 +49,18 @@ | ||
| 50 | } | 49 | } |
| 51 | } | 50 | } |
| 52 | .avatar { | 51 | .avatar { |
| 53 | - width: 32px; | 52 | + position: relative; |
| 53 | + top: -3px; | ||
| 54 | } | 54 | } |
| 55 | .event_icon { | 55 | .event_icon { |
| 56 | + position: relative; | ||
| 56 | float: right; | 57 | float: right; |
| 57 | border: 1px solid #EEE; | 58 | border: 1px solid #EEE; |
| 58 | padding: 5px; | 59 | padding: 5px; |
| 59 | @include border-radius(5px); | 60 | @include border-radius(5px); |
| 60 | background: #F9F9F9; | 61 | background: #F9F9F9; |
| 62 | + margin-left: 10px; | ||
| 63 | + top: -6px; | ||
| 61 | img { | 64 | img { |
| 62 | width: 20px; | 65 | width: 20px; |
| 63 | } | 66 | } |
| @@ -71,7 +74,7 @@ | @@ -71,7 +74,7 @@ | ||
| 71 | } | 74 | } |
| 72 | } | 75 | } |
| 73 | 76 | ||
| 74 | - padding: 15px 5px; | 77 | + padding: 16px 5px; |
| 75 | &:last-child { border:none } | 78 | &:last-child { border:none } |
| 76 | .wll:hover { background:none } | 79 | .wll:hover { background:none } |
| 77 | 80 |
app/controllers/milestones_controller.rb
| @@ -43,6 +43,7 @@ class MilestonesController < ProjectResourceController | @@ -43,6 +43,7 @@ class MilestonesController < ProjectResourceController | ||
| 43 | 43 | ||
| 44 | def create | 44 | def create |
| 45 | @milestone = @project.milestones.new(params[:milestone]) | 45 | @milestone = @project.milestones.new(params[:milestone]) |
| 46 | + @milestone.author_id_of_changes = current_user.id | ||
| 46 | 47 | ||
| 47 | if @milestone.save | 48 | if @milestone.save |
| 48 | redirect_to project_milestone_path(@project, @milestone) | 49 | redirect_to project_milestone_path(@project, @milestone) |
| @@ -52,7 +53,7 @@ class MilestonesController < ProjectResourceController | @@ -52,7 +53,7 @@ class MilestonesController < ProjectResourceController | ||
| 52 | end | 53 | end |
| 53 | 54 | ||
| 54 | def update | 55 | def update |
| 55 | - @milestone.update_attributes(params[:milestone]) | 56 | + @milestone.update_attributes(params[:milestone].merge(author_id_of_changes: current_user.id)) |
| 56 | 57 | ||
| 57 | respond_to do |format| | 58 | respond_to do |format| |
| 58 | format.js | 59 | format.js |
app/models/event.rb
| @@ -15,6 +15,7 @@ | @@ -15,6 +15,7 @@ | ||
| 15 | # | 15 | # |
| 16 | 16 | ||
| 17 | class Event < ActiveRecord::Base | 17 | class Event < ActiveRecord::Base |
| 18 | + include NoteEvent | ||
| 18 | include PushEvent | 19 | include PushEvent |
| 19 | 20 | ||
| 20 | attr_accessible :project, :action, :data, :author_id, :project_id, | 21 | attr_accessible :project, :action, :data, :author_id, :project_id, |
| @@ -58,12 +59,14 @@ class Event < ActiveRecord::Base | @@ -58,12 +59,14 @@ class Event < ActiveRecord::Base | ||
| 58 | end | 59 | end |
| 59 | end | 60 | end |
| 60 | 61 | ||
| 61 | - # Next events currently enabled for system | ||
| 62 | - # - push | ||
| 63 | - # - new issue | ||
| 64 | - # - merge request | ||
| 65 | - def allowed? | ||
| 66 | - push? || issue? || merge_request? || membership_changed? | 62 | + def proper? |
| 63 | + if push? | ||
| 64 | + true | ||
| 65 | + elsif membership_changed? | ||
| 66 | + true | ||
| 67 | + else | ||
| 68 | + (issue? || merge_request? || note? || milestone?) && target | ||
| 69 | + end | ||
| 67 | end | 70 | end |
| 68 | 71 | ||
| 69 | def project_name | 72 | def project_name |
| @@ -94,6 +97,14 @@ class Event < ActiveRecord::Base | @@ -94,6 +97,14 @@ class Event < ActiveRecord::Base | ||
| 94 | action == self.class::Reopened | 97 | action == self.class::Reopened |
| 95 | end | 98 | end |
| 96 | 99 | ||
| 100 | + def milestone? | ||
| 101 | + target_type == "Milestone" | ||
| 102 | + end | ||
| 103 | + | ||
| 104 | + def note? | ||
| 105 | + target_type == "Note" | ||
| 106 | + end | ||
| 107 | + | ||
| 97 | def issue? | 108 | def issue? |
| 98 | target_type == "Issue" | 109 | target_type == "Issue" |
| 99 | end | 110 | end |
app/models/milestone.rb
| @@ -13,7 +13,8 @@ | @@ -13,7 +13,8 @@ | ||
| 13 | # | 13 | # |
| 14 | 14 | ||
| 15 | class Milestone < ActiveRecord::Base | 15 | class Milestone < ActiveRecord::Base |
| 16 | - attr_accessible :title, :description, :due_date, :closed | 16 | + attr_accessible :title, :description, :due_date, :closed, :author_id_of_changes |
| 17 | + attr_accessor :author_id_of_changes | ||
| 17 | 18 | ||
| 18 | belongs_to :project | 19 | belongs_to :project |
| 19 | has_many :issues | 20 | has_many :issues |
| @@ -67,4 +68,8 @@ class Milestone < ActiveRecord::Base | @@ -67,4 +68,8 @@ class Milestone < ActiveRecord::Base | ||
| 67 | def open? | 68 | def open? |
| 68 | !closed | 69 | !closed |
| 69 | end | 70 | end |
| 71 | + | ||
| 72 | + def author_id | ||
| 73 | + author_id_of_changes | ||
| 74 | + end | ||
| 70 | end | 75 | end |
app/models/note.rb
| @@ -121,4 +121,12 @@ class Note < ActiveRecord::Base | @@ -121,4 +121,12 @@ class Note < ActiveRecord::Base | ||
| 121 | def downvote? | 121 | def downvote? |
| 122 | note.start_with?('-1') || note.start_with?(':-1:') | 122 | note.start_with?('-1') || note.start_with?(':-1:') |
| 123 | end | 123 | end |
| 124 | + | ||
| 125 | + def noteable_type_name | ||
| 126 | + if noteable_type.present? | ||
| 127 | + noteable_type.downcase | ||
| 128 | + else | ||
| 129 | + "wall" | ||
| 130 | + end | ||
| 131 | + end | ||
| 124 | end | 132 | end |
app/observers/activity_observer.rb
| 1 | class ActivityObserver < ActiveRecord::Observer | 1 | class ActivityObserver < ActiveRecord::Observer |
| 2 | - observe :issue, :merge_request | 2 | + observe :issue, :merge_request, :note, :milestone |
| 3 | 3 | ||
| 4 | def after_create(record) | 4 | def after_create(record) |
| 5 | - Event.create( | ||
| 6 | - project: record.project, | ||
| 7 | - target_id: record.id, | ||
| 8 | - target_type: record.class.name, | ||
| 9 | - action: Event.determine_action(record), | ||
| 10 | - author_id: record.author_id | ||
| 11 | - ) | 5 | + event_author_id = record.author_id |
| 6 | + | ||
| 7 | + # Skip status notes | ||
| 8 | + if record.kind_of?(Note) && record.note.include?("_Status changed to ") | ||
| 9 | + return true | ||
| 10 | + end | ||
| 11 | + | ||
| 12 | + if event_author_id | ||
| 13 | + Event.create( | ||
| 14 | + project: record.project, | ||
| 15 | + target_id: record.id, | ||
| 16 | + target_type: record.class.name, | ||
| 17 | + action: Event.determine_action(record), | ||
| 18 | + author_id: event_author_id | ||
| 19 | + ) | ||
| 20 | + end | ||
| 12 | end | 21 | end |
| 13 | 22 | ||
| 14 | def after_save(record) | 23 | def after_save(record) |
| 15 | - if record.changed.include?("closed") | 24 | + if record.changed.include?("closed") && record.author_id_of_changes |
| 16 | Event.create( | 25 | Event.create( |
| 17 | project: record.project, | 26 | project: record.project, |
| 18 | target_id: record.id, | 27 | target_id: record.id, |
| @@ -0,0 +1,21 @@ | @@ -0,0 +1,21 @@ | ||
| 1 | +module NoteEvent | ||
| 2 | + def note_commit_id | ||
| 3 | + target.noteable_id | ||
| 4 | + end | ||
| 5 | + | ||
| 6 | + def note_short_commit_id | ||
| 7 | + note_commit_id[0..8] | ||
| 8 | + end | ||
| 9 | + | ||
| 10 | + def note_commit? | ||
| 11 | + target.noteable_type == "Commit" | ||
| 12 | + end | ||
| 13 | + | ||
| 14 | + def note_target | ||
| 15 | + target.noteable | ||
| 16 | + end | ||
| 17 | + | ||
| 18 | + def note_target_id | ||
| 19 | + target.noteable_id | ||
| 20 | + end | ||
| 21 | +end |
app/views/dashboard/index.atom.builder
| @@ -7,7 +7,7 @@ xml.feed "xmlns" => "http://www.w3.org/2005/Atom", "xmlns:media" => "http://sear | @@ -7,7 +7,7 @@ xml.feed "xmlns" => "http://www.w3.org/2005/Atom", "xmlns:media" => "http://sear | ||
| 7 | xml.updated @events.maximum(:updated_at).strftime("%Y-%m-%dT%H:%M:%SZ") if @events.any? | 7 | xml.updated @events.maximum(:updated_at).strftime("%Y-%m-%dT%H:%M:%SZ") if @events.any? |
| 8 | 8 | ||
| 9 | @events.each do |event| | 9 | @events.each do |event| |
| 10 | - if event.allowed? | 10 | + if event.proper? |
| 11 | event = EventDecorator.decorate(event) | 11 | event = EventDecorator.decorate(event) |
| 12 | xml.entry do | 12 | xml.entry do |
| 13 | event_link = event.feed_url | 13 | event_link = event.feed_url |
app/views/events/_event.html.haml
| 1 | -- if event.allowed? | 1 | +- if event.proper? |
| 2 | %div.event-item | 2 | %div.event-item |
| 3 | - = event_image(event) | 3 | + %span.cgray.right |
| 4 | + #{time_ago_in_words(event.created_at)} ago. | ||
| 5 | + | ||
| 4 | = image_tag gravatar_icon(event.author_email), class: "avatar s24" | 6 | = image_tag gravatar_icon(event.author_email), class: "avatar s24" |
| 5 | 7 | ||
| 6 | - if event.push? | 8 | - if event.push? |
| 7 | = render "events/event/push", event: event | 9 | = render "events/event/push", event: event |
| 10 | + .clearfix | ||
| 11 | + - elsif event.note? | ||
| 12 | + = render "events/event/note", event: event | ||
| 8 | - else | 13 | - else |
| 9 | = render "events/event/common", event: event | 14 | = render "events/event/common", event: event |
| 10 | 15 | ||
| 11 | - .clearfix | ||
| 12 | - %span.cgray.right | ||
| 13 | - = time_ago_in_words(event.created_at) | ||
| 14 | - ago. | ||
| 15 | - .clearfix |
| @@ -0,0 +1,23 @@ | @@ -0,0 +1,23 @@ | ||
| 1 | +.event-title | ||
| 2 | + %span.author_name= link_to_author event | ||
| 3 | + %span.event_label commented on #{event.target.noteable_type_name} | ||
| 4 | + - if event.target and event.note_target | ||
| 5 | + - if event.note_commit? | ||
| 6 | + = link_to event.note_short_commit_id, project_commit_path(event.project, event.note_commit_id), class: "commit_short_id" | ||
| 7 | + - else | ||
| 8 | + = link_to [event.project, event.note_target] do | ||
| 9 | + %strong= truncate event.note_target_id | ||
| 10 | + | ||
| 11 | + - else | ||
| 12 | + %strong (deleted) | ||
| 13 | + at | ||
| 14 | + - if event.project | ||
| 15 | + = link_to_project event.project | ||
| 16 | + - else | ||
| 17 | + = event.project_name | ||
| 18 | + | ||
| 19 | +.event-body | ||
| 20 | + %span.hint | ||
| 21 | + | ||
| 22 | + %i.icon-comment | ||
| 23 | + = truncate event.target.note, length: 70 |
app/views/groups/show.atom.builder
| @@ -7,7 +7,7 @@ xml.feed "xmlns" => "http://www.w3.org/2005/Atom", "xmlns:media" => "http://sear | @@ -7,7 +7,7 @@ xml.feed "xmlns" => "http://www.w3.org/2005/Atom", "xmlns:media" => "http://sear | ||
| 7 | xml.updated @events.maximum(:updated_at).strftime("%Y-%m-%dT%H:%M:%SZ") if @events.any? | 7 | xml.updated @events.maximum(:updated_at).strftime("%Y-%m-%dT%H:%M:%SZ") if @events.any? |
| 8 | 8 | ||
| 9 | @events.each do |event| | 9 | @events.each do |event| |
| 10 | - if event.allowed? | 10 | + if event.proper? |
| 11 | event = EventDecorator.decorate(event) | 11 | event = EventDecorator.decorate(event) |
| 12 | xml.entry do | 12 | xml.entry do |
| 13 | event_link = event.feed_url | 13 | event_link = event.feed_url |
spec/models/event_spec.rb
| @@ -59,7 +59,7 @@ describe Event do | @@ -59,7 +59,7 @@ describe Event do | ||
| 59 | end | 59 | end |
| 60 | 60 | ||
| 61 | it { @event.push?.should be_true } | 61 | it { @event.push?.should be_true } |
| 62 | - it { @event.allowed?.should be_true } | 62 | + it { @event.proper?.should be_true } |
| 63 | it { @event.new_branch?.should be_true } | 63 | it { @event.new_branch?.should be_true } |
| 64 | it { @event.tag?.should be_false } | 64 | it { @event.tag?.should be_false } |
| 65 | it { @event.branch_name.should == "master" } | 65 | it { @event.branch_name.should == "master" } |
spec/observers/activity_observer_spec.rb
| @@ -34,15 +34,17 @@ describe ActivityObserver do | @@ -34,15 +34,17 @@ describe ActivityObserver do | ||
| 34 | it { @event.target.should == @issue } | 34 | it { @event.target.should == @issue } |
| 35 | end | 35 | end |
| 36 | 36 | ||
| 37 | - #describe "Issue commented" do | ||
| 38 | - #before do | ||
| 39 | - #@issue = create(:issue, project: project) | ||
| 40 | - #@note = create(:note, noteable: @issue, project: project) | ||
| 41 | - #@event = Event.last | ||
| 42 | - #end | ||
| 43 | - | ||
| 44 | - #it_should_be_valid_event | ||
| 45 | - #it { @event.action.should == Event::Commented } | ||
| 46 | - #it { @event.target.should == @note } | ||
| 47 | - #end | 37 | + describe "Issue commented" do |
| 38 | + before do | ||
| 39 | + Note.observers.enable :activity_observer do | ||
| 40 | + @issue = create(:issue, project: project) | ||
| 41 | + @note = create(:note, noteable: @issue, project: project, author: @issue.author) | ||
| 42 | + @event = Event.last | ||
| 43 | + end | ||
| 44 | + end | ||
| 45 | + | ||
| 46 | + it_should_be_valid_event | ||
| 47 | + it { @event.action.should == Event::Commented } | ||
| 48 | + it { @event.target.should == @note } | ||
| 49 | + end | ||
| 48 | end | 50 | end |