Commit 80685596d3df23f05f2bd9978c8d8f483e799028
Exists in
master
and in
4 other branches
Merge pull request #1414 from AlexDenisov/team_member_events
Team membership events
Showing
11 changed files
with
143 additions
and
3 deletions
Show diff stats
app/assets/stylesheets/common.scss
| @@ -179,6 +179,14 @@ span.update-author { | @@ -179,6 +179,14 @@ span.update-author { | ||
| 179 | &.merged { | 179 | &.merged { |
| 180 | background-color: #2A2; | 180 | background-color: #2A2; |
| 181 | } | 181 | } |
| 182 | + | ||
| 183 | + &.joined { | ||
| 184 | + background-color: #1cb9ff; | ||
| 185 | + } | ||
| 186 | + | ||
| 187 | + &.left { | ||
| 188 | + background-color: #ff5057; | ||
| 189 | + } | ||
| 182 | } | 190 | } |
| 183 | 191 | ||
| 184 | form { | 192 | form { |
app/decorators/event_decorator.rb
| @@ -8,7 +8,9 @@ class EventDecorator < ApplicationDecorator | @@ -8,7 +8,9 @@ class EventDecorator < ApplicationDecorator | ||
| 8 | "#{self.author_name} #{self.action_name} MR ##{self.target_id}:" + self.merge_request_title | 8 | "#{self.author_name} #{self.action_name} MR ##{self.target_id}:" + self.merge_request_title |
| 9 | elsif self.push? | 9 | elsif self.push? |
| 10 | "#{self.author_name} #{self.push_action_name} #{self.ref_type} " + self.ref_name | 10 | "#{self.author_name} #{self.push_action_name} #{self.ref_type} " + self.ref_name |
| 11 | - else | 11 | + elsif self.membership_changed? |
| 12 | + "#{self.author_name} #{self.action_name} #{self.project.name}" | ||
| 13 | + else | ||
| 12 | "" | 14 | "" |
| 13 | end | 15 | end |
| 14 | end | 16 | end |
app/models/event.rb
| @@ -10,6 +10,8 @@ class Event < ActiveRecord::Base | @@ -10,6 +10,8 @@ class Event < ActiveRecord::Base | ||
| 10 | Pushed = 5 | 10 | Pushed = 5 |
| 11 | Commented = 6 | 11 | Commented = 6 |
| 12 | Merged = 7 | 12 | Merged = 7 |
| 13 | + Joined = 8 # User joined project | ||
| 14 | + Left = 9 # User left project | ||
| 13 | 15 | ||
| 14 | belongs_to :project | 16 | belongs_to :project |
| 15 | belongs_to :target, polymorphic: true | 17 | belongs_to :target, polymorphic: true |
| @@ -37,7 +39,7 @@ class Event < ActiveRecord::Base | @@ -37,7 +39,7 @@ class Event < ActiveRecord::Base | ||
| 37 | # - new issue | 39 | # - new issue |
| 38 | # - merge request | 40 | # - merge request |
| 39 | def allowed? | 41 | def allowed? |
| 40 | - push? || issue? || merge_request? | 42 | + push? || issue? || merge_request? || membership_changed? |
| 41 | end | 43 | end |
| 42 | 44 | ||
| 43 | def push? | 45 | def push? |
| @@ -84,6 +86,18 @@ class Event < ActiveRecord::Base | @@ -84,6 +86,18 @@ class Event < ActiveRecord::Base | ||
| 84 | [Closed, Reopened].include?(action) | 86 | [Closed, Reopened].include?(action) |
| 85 | end | 87 | end |
| 86 | 88 | ||
| 89 | + def joined? | ||
| 90 | + action == Joined | ||
| 91 | + end | ||
| 92 | + | ||
| 93 | + def left? | ||
| 94 | + action == Left | ||
| 95 | + end | ||
| 96 | + | ||
| 97 | + def membership_changed? | ||
| 98 | + joined? || left? | ||
| 99 | + end | ||
| 100 | + | ||
| 87 | def issue | 101 | def issue |
| 88 | target if target_type == "Issue" | 102 | target if target_type == "Issue" |
| 89 | end | 103 | end |
| @@ -101,6 +115,10 @@ class Event < ActiveRecord::Base | @@ -101,6 +115,10 @@ class Event < ActiveRecord::Base | ||
| 101 | "closed" | 115 | "closed" |
| 102 | elsif merged? | 116 | elsif merged? |
| 103 | "merged" | 117 | "merged" |
| 118 | + elsif joined? | ||
| 119 | + 'joined' | ||
| 120 | + elsif left? | ||
| 121 | + 'left' | ||
| 104 | else | 122 | else |
| 105 | "opened" | 123 | "opened" |
| 106 | end | 124 | end |
app/models/users_project.rb
| @@ -23,7 +23,7 @@ class UsersProject < ActiveRecord::Base | @@ -23,7 +23,7 @@ class UsersProject < ActiveRecord::Base | ||
| 23 | def self.bulk_delete(project, user_ids) | 23 | def self.bulk_delete(project, user_ids) |
| 24 | UsersProject.transaction do | 24 | UsersProject.transaction do |
| 25 | UsersProject.where(:user_id => user_ids, :project_id => project.id).each do |users_project| | 25 | UsersProject.where(:user_id => user_ids, :project_id => project.id).each do |users_project| |
| 26 | - users_project.delete | 26 | + users_project.destroy |
| 27 | end | 27 | end |
| 28 | end | 28 | end |
| 29 | end | 29 | end |
app/observers/users_project_observer.rb
| 1 | class UsersProjectObserver < ActiveRecord::Observer | 1 | class UsersProjectObserver < ActiveRecord::Observer |
| 2 | def after_create(users_project) | 2 | def after_create(users_project) |
| 3 | Notify.project_access_granted_email(users_project.id).deliver | 3 | Notify.project_access_granted_email(users_project.id).deliver |
| 4 | + | ||
| 5 | + Event.create( | ||
| 6 | + project_id: users_project.project.id, | ||
| 7 | + action: Event::Joined, | ||
| 8 | + author_id: users_project.user.id | ||
| 9 | + ) | ||
| 4 | end | 10 | end |
| 5 | 11 | ||
| 6 | def after_update(users_project) | 12 | def after_update(users_project) |
| 7 | Notify.project_access_granted_email(users_project.id).deliver | 13 | Notify.project_access_granted_email(users_project.id).deliver |
| 8 | end | 14 | end |
| 15 | + | ||
| 16 | + def after_destroy(users_project) | ||
| 17 | + Event.create( | ||
| 18 | + project_id: users_project.project.id, | ||
| 19 | + action: Event::Left, | ||
| 20 | + author_id: users_project.user.id | ||
| 21 | + ) | ||
| 22 | + end | ||
| 23 | + | ||
| 9 | end | 24 | end |
app/views/events/_event.html.haml
| @@ -0,0 +1,9 @@ | @@ -0,0 +1,9 @@ | ||
| 1 | += image_tag gravatar_icon(event.author_email), class: "avatar" | ||
| 2 | +%strong #{event.author_name} | ||
| 3 | +%span.event_label{class: event.action_name}= event.action_name | ||
| 4 | +project | ||
| 5 | +%strong= link_to event.project.name, event.project | ||
| 6 | +%span.cgray | ||
| 7 | + = time_ago_in_words(event.created_at) | ||
| 8 | + ago. | ||
| 9 | + |
features/dashboard/dashboard.feature
| @@ -15,4 +15,14 @@ Feature: Dashboard | @@ -15,4 +15,14 @@ Feature: Dashboard | ||
| 15 | And I click "Create Merge Request" link | 15 | And I click "Create Merge Request" link |
| 16 | Then I see prefilled new Merge Request page | 16 | Then I see prefilled new Merge Request page |
| 17 | 17 | ||
| 18 | + Scenario: I should see User joined Project event | ||
| 19 | + Given user with name "John Doe" joined project "Shop" | ||
| 20 | + When I visit dashboard page | ||
| 21 | + Then I should see "John Doe joined project Shop" event | ||
| 18 | 22 | ||
| 23 | + Scenario: I should see User left Project event | ||
| 24 | + Given user with name "John Doe" joined project "Shop" | ||
| 25 | + And user with name "John Doe" left project "Shop" | ||
| 26 | + When I visit dashboard page | ||
| 27 | + Then I should see "John Doe left project Shop" event | ||
| 28 | + |
features/step_definitions/dashboard_steps.rb
| @@ -109,3 +109,28 @@ Given /^I have authored merge requests$/ do | @@ -109,3 +109,28 @@ Given /^I have authored merge requests$/ do | ||
| 109 | :author => @user, | 109 | :author => @user, |
| 110 | :project => project2 | 110 | :project => project2 |
| 111 | end | 111 | end |
| 112 | + | ||
| 113 | +Given /^user with name "(.*?)" joined project "(.*?)"$/ do |user_name, project_name| | ||
| 114 | + user = Factory.create(:user, {name: user_name}) | ||
| 115 | + project = Project.find_by_name project_name | ||
| 116 | + Event.create( | ||
| 117 | + project: project, | ||
| 118 | + author_id: user.id, | ||
| 119 | + action: Event::Joined | ||
| 120 | + ) | ||
| 121 | +end | ||
| 122 | + | ||
| 123 | +Given /^user with name "(.*?)" left project "(.*?)"$/ do |user_name, project_name| | ||
| 124 | + user = User.find_by_name user_name | ||
| 125 | + project = Project.find_by_name project_name | ||
| 126 | + Event.create( | ||
| 127 | + project: project, | ||
| 128 | + author_id: user.id, | ||
| 129 | + action: Event::Left | ||
| 130 | + ) | ||
| 131 | +end | ||
| 132 | + | ||
| 133 | +Then /^I should see "(.*?)" event$/ do |event_text| | ||
| 134 | + page.should have_content(event_text) | ||
| 135 | +end | ||
| 136 | + |
spec/models/event_spec.rb
| @@ -49,4 +49,26 @@ describe Event do | @@ -49,4 +49,26 @@ describe Event do | ||
| 49 | it { @event.branch_name.should == "master" } | 49 | it { @event.branch_name.should == "master" } |
| 50 | it { @event.author.should == @user } | 50 | it { @event.author.should == @user } |
| 51 | end | 51 | end |
| 52 | + | ||
| 53 | + describe "Joined project team" do | ||
| 54 | + let(:project) {Factory.create :project} | ||
| 55 | + let(:new_user) {Factory.create :user} | ||
| 56 | + it "should create event" do | ||
| 57 | + UsersProject.observers.enable :users_project_observer | ||
| 58 | + expect{ | ||
| 59 | + UsersProject.bulk_import(project, [new_user.id], UsersProject::DEVELOPER) | ||
| 60 | + }.to change{Event.count}.by(1) | ||
| 61 | + end | ||
| 62 | + end | ||
| 63 | + describe "Left project team" do | ||
| 64 | + let(:project) {Factory.create :project} | ||
| 65 | + let(:new_user) {Factory.create :user} | ||
| 66 | + it "should create event" do | ||
| 67 | + UsersProject.bulk_import(project, [new_user.id], UsersProject::DEVELOPER) | ||
| 68 | + UsersProject.observers.enable :users_project_observer | ||
| 69 | + expect{ | ||
| 70 | + UsersProject.bulk_delete(project, [new_user.id]) | ||
| 71 | + }.to change{Event.count}.by(1) | ||
| 72 | + end | ||
| 73 | + end | ||
| 52 | end | 74 | end |
spec/observers/users_project_observer_spec.rb
| @@ -23,6 +23,14 @@ describe UsersProjectObserver do | @@ -23,6 +23,14 @@ describe UsersProjectObserver do | ||
| 23 | Notify.should_receive(:project_access_granted_email).with(users_project.id).and_return(double(deliver: true)) | 23 | Notify.should_receive(:project_access_granted_email).with(users_project.id).and_return(double(deliver: true)) |
| 24 | subject.after_create(users_project) | 24 | subject.after_create(users_project) |
| 25 | end | 25 | end |
| 26 | + it "should create new event" do | ||
| 27 | + Event.should_receive(:create).with( | ||
| 28 | + project_id: users_project.project.id, | ||
| 29 | + action: Event::Joined, | ||
| 30 | + author_id: users_project.user.id | ||
| 31 | + ) | ||
| 32 | + subject.after_create(users_project) | ||
| 33 | + end | ||
| 26 | end | 34 | end |
| 27 | 35 | ||
| 28 | describe "#after_update" do | 36 | describe "#after_update" do |
| @@ -37,4 +45,23 @@ describe UsersProjectObserver do | @@ -37,4 +45,23 @@ describe UsersProjectObserver do | ||
| 37 | subject.after_update(users_project) | 45 | subject.after_update(users_project) |
| 38 | end | 46 | end |
| 39 | end | 47 | end |
| 48 | + describe "#after_destroy" do | ||
| 49 | + it "should called when UsersProject destroyed" do | ||
| 50 | + subject.should_receive(:after_destroy) | ||
| 51 | + UsersProject.observers.enable :users_project_observer do | ||
| 52 | + UsersProject.bulk_delete( | ||
| 53 | + users_project.project, | ||
| 54 | + [users_project.user.id] | ||
| 55 | + ) | ||
| 56 | + end | ||
| 57 | + end | ||
| 58 | + it "should create new event" do | ||
| 59 | + Event.should_receive(:create).with( | ||
| 60 | + project_id: users_project.project.id, | ||
| 61 | + action: Event::Left, | ||
| 62 | + author_id: users_project.user.id | ||
| 63 | + ) | ||
| 64 | + subject.after_destroy(users_project) | ||
| 65 | + end | ||
| 66 | + end | ||
| 40 | end | 67 | end |