Merge branch 'master' into issue-board-sidebar

CHANGELOG.md

@@ -1,6 +1,8 @@
 Please view this file on the master branch, on stable branches it's out of date.
 
 ## 8.14.0 (2016-11-22)
+  - Adds user project membership expired event to clarify why user was removed (Callum Dryden)
+  - Simpler arguments passed to named_route on toggle_award_url helper method 
 
 ## 8.13.0 (2016-10-22)
 
@@ -133,6 +135,7 @@ Please view this file on the master branch, on stable branches it's out of date.
   - Delete dynamic environments
   - Fix buggy iOS tooltip layering behavior.
   - Make guests unable to view MRs on private projects
+  - Fix broken Project API docs (Takuya Noguchi)
 
 ## 8.12.7
 

app/assets/stylesheets/pages/boards.scss

@@ -1,4 +1,3 @@
-lex
 [v-cloak] {
   display: none;
 }
@@ -145,7 +144,7 @@ lex
 }
 
 .board-blank-state {
-  height: 100%;
+  height: calc(100% - 49px);
   padding: $gl-padding;
   background-color: #fff;
 }

app/assets/stylesheets/pages/pipelines.scss

@@ -643,6 +643,10 @@
 
   &.pipelines {
 
+    .ci-table {
+      min-width: 900px;
+    }
+
     .content-list.pipelines {
       overflow: auto;
     }

app/helpers/award_emoji_helper.rb

@@ -3,8 +3,8 @@ module AwardEmojiHelper
     return url_for([:toggle_award_emoji, awardable]) unless @project
 
     if awardable.is_a?(Note)
-      # We render a list of notes very frequently and calling the specific method is a lot faster than the generic one (6.5x)
-      toggle_award_emoji_namespace_project_note_url(namespace_id: @project.namespace, project_id: @project, id: awardable.id)
+      # We render a list of notes very frequently and calling the specific method is a lot faster than the generic one (4.5x)
+      toggle_award_emoji_namespace_project_note_url(@project.namespace, @project, awardable.id)
     else
       url_for([:toggle_award_emoji, @project.namespace.becomes(Namespace), @project, awardable])
     end

app/models/concerns/expirable.rb

@@ -5,11 +5,15 @@ module Expirable
     scope :expired, -> { where('expires_at <= ?', Time.current) }
   end
 
+  def expired?
+    expires? && expires_at <= Time.current
+  end
+
   def expires?
     expires_at.present?
   end
 
   def expires_soon?
-    expires_at < 7.days.from_now
+    expires? && expires_at < 7.days.from_now
   end
 end

app/models/event.rb

@@ -12,6 +12,7 @@ class Event < ActiveRecord::Base
   JOINED    = 8 # User joined project
   LEFT      = 9 # User left project
   DESTROYED = 10
+  EXPIRED   = 11 # User left project due to expiry
 
   RESET_PROJECT_ACTIVITY_INTERVAL = 1.hour
 
@@ -115,6 +116,10 @@ class Event < ActiveRecord::Base
     action == LEFT
   end
 
+  def expired?
+    action == EXPIRED
+  end
+
   def destroyed?
     action == DESTROYED
   end
@@ -124,7 +129,7 @@ class Event < ActiveRecord::Base
   end
 
   def membership_changed?
-    joined? || left?
+    joined? || left? || expired?
   end
 
   def created_project?
@@ -184,6 +189,8 @@ class Event < ActiveRecord::Base
       'joined'
     elsif left?
       'left'
+    elsif expired?
+      'removed due to membership expiration from'
     elsif destroyed?
       'destroyed'
     elsif commented?

app/models/members/project_member.rb

@@ -121,7 +121,11 @@ class ProjectMember < Member
   end
 
   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
   end

app/services/event_create_service.rb

@@ -62,6 +62,10 @@ class EventCreateService
     create_event(project, current_user, Event::LEFT)
   end
 
+  def expired_leave_project(project, current_user)
+    create_event(project, current_user, Event::EXPIRED)
+  end
+
   def create_project(project, current_user)
     create_event(project, current_user, Event::CREATED)
   end

doc/api/projects.md

@@ -1333,8 +1333,6 @@ Parameters:
 
 | Attribute | Type | Required | Description |
 | --------- | ---- | -------- | ----------- |
-| `query` (required) - A string contained in the project name
-| `per_page` (optional) - number of projects to return per page
-| `page` (optional) - the page to retrieve
-| `order_by` (optional) - Return requests ordered by `id`, `name`, `created_at` or `last_activity_at` fields
+| `query` | string | yes | A string contained in the project name |
+| `order_by` | string | no | Return requests ordered by `id`, `name`, `created_at` or `last_activity_at` fields |
 | `sort` | string | no | Return requests sorted in `asc` or `desc` order |

features/dashboard/dashboard.feature

@@ -31,19 +31,6 @@ Feature: Dashboard
     And I click "Create Merge Request" link
     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
   Scenario: Sorting Issues
     Given I visit dashboard issues page

features/steps/dashboard/dashboard.rb

@@ -33,33 +33,6 @@ class Spinach::Features::Dashboard < Spinach::FeatureSteps
     expect(find("input#merge_request_target_branch").value).to eq "master"
   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
     @group   = create(:group)
     @project = create(:project, namespace: @group)

lib/event_filter.rb

@@ -45,9 +45,16 @@ class EventFilter
     when EventFilter.comments
       actions = [Event::COMMENTED]
     when EventFilter.team
-      actions = [Event::JOINED, Event::LEFT]
+      actions = [Event::JOINED, Event::LEFT, Event::EXPIRED]
     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
 
     events.where(action: actions)

spec/features/dashboard/project_member_activity_index_spec.rb

@@ -0,0 +1,41 @@
+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

spec/models/concerns/expirable_spec.rb

@@ -0,0 +1,31 @@
+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

spec/models/event_spec.rb

@@ -40,6 +40,33 @@ describe Event, models: true do
     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
     subject { Event.new(project: target.project, target: target) }
 

spec/models/members/project_member_spec.rb

@@ -54,6 +54,17 @@ describe ProjectMember, models: true do
       master_todos
     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
       expect(owner.user.todos.size).to eq(2)
       expect(master.user.todos.size).to eq(3)

spec/services/event_create_service_spec.rb

@@ -110,4 +110,23 @@ describe EventCreateService, services: true do
       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