Commit b5cee666 authored by Sean McGivern's avatar Sean McGivern

Merge branch 'add_user_expire_leave_event' into 'master'

Differentiate the expire from leave event

At the moment we cannot see whether a user left a project due to their
membership expiring or if they themselves opted to leave the project.
This adds a new event type that allows us to make this differentiation.
Note that is not really feasible to go back and reliably fix up the
previous events. As a result the events for previous expire removals
will remain the same however events of this nature going forward will be
correctly represented.

Closes #22611 

![Screen_Shot_2016-10-06_at_09.27.52](/uploads/e4de7bd2cdedc64a9b19e56fefa21314/Screen_Shot_2016-10-06_at_09.27.52.png)

See merge request !6717
parents 192a6eaf 9124310f
Please view this file on the master branch, on stable branches it's out of date. Please view this file on the master branch, on stable branches it's out of date.
## 8.14.0 (2016-11-22) ## 8.14.0 (2016-11-22)
- Adds user project membership expired event to clarify why user was removed (Callum Dryden)
## 8.13.0 (2016-10-22) ## 8.13.0 (2016-10-22)
......
...@@ -5,11 +5,15 @@ module Expirable ...@@ -5,11 +5,15 @@ module Expirable
scope :expired, -> { where('expires_at <= ?', Time.current) } scope :expired, -> { where('expires_at <= ?', Time.current) }
end end
def expired?
expires? && expires_at <= Time.current
end
def expires? def expires?
expires_at.present? expires_at.present?
end end
def expires_soon? def expires_soon?
expires_at < 7.days.from_now expires? && expires_at < 7.days.from_now
end end
end end
...@@ -12,6 +12,7 @@ class Event < ActiveRecord::Base ...@@ -12,6 +12,7 @@ class Event < ActiveRecord::Base
JOINED = 8 # User joined project JOINED = 8 # User joined project
LEFT = 9 # User left project LEFT = 9 # User left project
DESTROYED = 10 DESTROYED = 10
EXPIRED = 11 # User left project due to expiry
RESET_PROJECT_ACTIVITY_INTERVAL = 1.hour RESET_PROJECT_ACTIVITY_INTERVAL = 1.hour
...@@ -115,6 +116,10 @@ class Event < ActiveRecord::Base ...@@ -115,6 +116,10 @@ class Event < ActiveRecord::Base
action == LEFT action == LEFT
end end
def expired?
action == EXPIRED
end
def destroyed? def destroyed?
action == DESTROYED action == DESTROYED
end end
...@@ -124,7 +129,7 @@ class Event < ActiveRecord::Base ...@@ -124,7 +129,7 @@ class Event < ActiveRecord::Base
end end
def membership_changed? def membership_changed?
joined? || left? joined? || left? || expired?
end end
def created_project? def created_project?
...@@ -184,6 +189,8 @@ class Event < ActiveRecord::Base ...@@ -184,6 +189,8 @@ class Event < ActiveRecord::Base
'joined' 'joined'
elsif left? elsif left?
'left' 'left'
elsif expired?
'removed due to membership expiration from'
elsif destroyed? elsif destroyed?
'destroyed' 'destroyed'
elsif commented? elsif commented?
......
...@@ -121,7 +121,11 @@ class ProjectMember < Member ...@@ -121,7 +121,11 @@ class ProjectMember < Member
end end
def post_destroy_hook def post_destroy_hook
event_service.leave_project(self.project, self.user) if expired?
event_service.expired_leave_project(self.project, self.user)
else
event_service.leave_project(self.project, self.user)
end
super super
end end
......
...@@ -62,6 +62,10 @@ class EventCreateService ...@@ -62,6 +62,10 @@ class EventCreateService
create_event(project, current_user, Event::LEFT) create_event(project, current_user, Event::LEFT)
end end
def expired_leave_project(project, current_user)
create_event(project, current_user, Event::EXPIRED)
end
def create_project(project, current_user) def create_project(project, current_user)
create_event(project, current_user, Event::CREATED) create_event(project, current_user, Event::CREATED)
end end
......
...@@ -31,19 +31,6 @@ Feature: Dashboard ...@@ -31,19 +31,6 @@ Feature: Dashboard
And I click "Create Merge Request" link And I click "Create Merge Request" link
Then I see prefilled new Merge Request page Then I see prefilled new Merge Request page
@javascript
Scenario: I should see User joined Project event
Given user with name "John Doe" joined project "Shop"
When I visit dashboard activity page
Then I should see "John Doe joined project Shop" event
@javascript
Scenario: I should see User left Project event
Given user with name "John Doe" joined project "Shop"
And user with name "John Doe" left project "Shop"
When I visit dashboard activity page
Then I should see "John Doe left project Shop" event
@javascript @javascript
Scenario: Sorting Issues Scenario: Sorting Issues
Given I visit dashboard issues page Given I visit dashboard issues page
......
...@@ -33,33 +33,6 @@ class Spinach::Features::Dashboard < Spinach::FeatureSteps ...@@ -33,33 +33,6 @@ class Spinach::Features::Dashboard < Spinach::FeatureSteps
expect(find("input#merge_request_target_branch").value).to eq "master" expect(find("input#merge_request_target_branch").value).to eq "master"
end end
step 'user with name "John Doe" joined project "Shop"' do
user = create(:user, { name: "John Doe" })
project.team << [user, :master]
Event.create(
project: project,
author_id: user.id,
action: Event::JOINED
)
end
step 'I should see "John Doe joined project Shop" event' do
expect(page).to have_content "John Doe joined project #{project.name_with_namespace}"
end
step 'user with name "John Doe" left project "Shop"' do
user = User.find_by(name: "John Doe")
Event.create(
project: project,
author_id: user.id,
action: Event::LEFT
)
end
step 'I should see "John Doe left project Shop" event' do
expect(page).to have_content "John Doe left project #{project.name_with_namespace}"
end
step 'I have group with projects' do step 'I have group with projects' do
@group = create(:group) @group = create(:group)
@project = create(:project, namespace: @group) @project = create(:project, namespace: @group)
......
...@@ -45,9 +45,16 @@ class EventFilter ...@@ -45,9 +45,16 @@ class EventFilter
when EventFilter.comments when EventFilter.comments
actions = [Event::COMMENTED] actions = [Event::COMMENTED]
when EventFilter.team when EventFilter.team
actions = [Event::JOINED, Event::LEFT] actions = [Event::JOINED, Event::LEFT, Event::EXPIRED]
when EventFilter.all when EventFilter.all
actions = [Event::PUSHED, Event::MERGED, Event::COMMENTED, Event::JOINED, Event::LEFT] actions = [
Event::PUSHED,
Event::MERGED,
Event::COMMENTED,
Event::JOINED,
Event::LEFT,
Event::EXPIRED
]
end end
events.where(action: actions) events.where(action: actions)
......
require 'spec_helper'
feature 'Project member activity', feature: true, js: true do
include WaitForAjax
let(:user) { create(:user) }
let(:project) { create(:empty_project, :public, name: 'x', namespace: user.namespace) }
before do
project.team << [user, :master]
end
def visit_activities_and_wait_with_event(event_type)
Event.create(project: project, author_id: user.id, action: event_type)
visit activity_namespace_project_path(project.namespace.path, project.path)
wait_for_ajax
end
subject { page.find(".event-title").text }
context 'when a user joins the project' do
before { visit_activities_and_wait_with_event(Event::JOINED) }
it { is_expected.to eq("#{user.name} joined project") }
end
context 'when a user leaves the project' do
before { visit_activities_and_wait_with_event(Event::LEFT) }
it { is_expected.to eq("#{user.name} left project") }
end
context 'when a users membership expires for the project' do
before { visit_activities_and_wait_with_event(Event::EXPIRED) }
it "presents the correct message" do
message = "#{user.name} removed due to membership expiration from project"
is_expected.to eq(message)
end
end
end
require 'spec_helper'
describe Expirable do
describe 'ProjectMember' do
let(:no_expire) { create(:project_member) }
let(:expire_later) { create(:project_member, expires_at: Time.current + 6.days) }
let(:expired) { create(:project_member, expires_at: Time.current - 6.days) }
describe '.expired' do
it { expect(ProjectMember.expired).to match_array([expired]) }
end
describe '#expired?' do
it { expect(no_expire.expired?).to eq(false) }
it { expect(expire_later.expired?).to eq(false) }
it { expect(expired.expired?).to eq(true) }
end
describe '#expires?' do
it { expect(no_expire.expires?).to eq(false) }
it { expect(expire_later.expires?).to eq(true) }
it { expect(expired.expires?).to eq(true) }
end
describe '#expires_soon?' do
it { expect(no_expire.expires_soon?).to eq(false) }
it { expect(expire_later.expires_soon?).to eq(true) }
it { expect(expired.expires_soon?).to eq(true) }
end
end
end
...@@ -40,6 +40,33 @@ describe Event, models: true do ...@@ -40,6 +40,33 @@ describe Event, models: true do
end end
end end
describe '#membership_changed?' do
context "created" do
subject { build(:event, action: Event::CREATED).membership_changed? }
it { is_expected.to be_falsey }
end
context "updated" do
subject { build(:event, action: Event::UPDATED).membership_changed? }
it { is_expected.to be_falsey }
end
context "expired" do
subject { build(:event, action: Event::EXPIRED).membership_changed? }
it { is_expected.to be_truthy }
end
context "left" do
subject { build(:event, action: Event::LEFT).membership_changed? }
it { is_expected.to be_truthy }
end
context "joined" do
subject { build(:event, action: Event::JOINED).membership_changed? }
it { is_expected.to be_truthy }
end
end
describe '#note?' do describe '#note?' do
subject { Event.new(project: target.project, target: target) } subject { Event.new(project: target.project, target: target) }
......
...@@ -54,6 +54,17 @@ describe ProjectMember, models: true do ...@@ -54,6 +54,17 @@ describe ProjectMember, models: true do
master_todos master_todos
end end
it "creates an expired event when left due to expiry" do
expired = create(:project_member, project: project, expires_at: Time.now - 6.days)
expired.destroy
expect(Event.first.action).to eq(Event::EXPIRED)
end
it "creates a left event when left due to leave" do
master.destroy
expect(Event.first.action).to eq(Event::LEFT)
end
it "destroys itself and delete associated todos" do it "destroys itself and delete associated todos" do
expect(owner.user.todos.size).to eq(2) expect(owner.user.todos.size).to eq(2)
expect(master.user.todos.size).to eq(3) expect(master.user.todos.size).to eq(3)
......
...@@ -110,4 +110,23 @@ describe EventCreateService, services: true do ...@@ -110,4 +110,23 @@ describe EventCreateService, services: true do
end end
end end
end end
describe 'Project' do
let(:user) { create :user }
let(:project) { create(:empty_project) }
describe '#join_project' do
subject { service.join_project(project, user) }
it { is_expected.to be_truthy }
it { expect { subject }.to change { Event.count }.from(0).to(1) }
end
describe '#expired_leave_project' do
subject { service.expired_leave_project(project, user) }
it { is_expected.to be_truthy }
it { expect { subject }.to change { Event.count }.from(0).to(1) }
end
end
end end
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment