Improve pipeline triggers UI

3 files changed, 319 insertions, 11 deletions

diff --git a/spec/features/triggers_spec.rb b/spec/features/triggers_spec.rb
index 4a7511589d6..c1ae6db00c6 100644
--- a/spec/features/triggers_spec.rb
+++ b/spec/features/triggers_spec.rb
@@ -1,28 +1,175 @@
 require 'spec_helper'
 
-describe 'Triggers' do
+feature 'Triggers', feature: true, js: true do
+  let(:trigger_title) { 'trigger desc' }
   let(:user) { create(:user) }
+  let(:user2) { create(:user) }
+  let(:guest_user) { create(:user) }
   before { login_as(user) }
 
   before do
-    @project = FactoryGirl.create :empty_project
+    @project = create(:empty_project)
     @project.team << [user, :master]
+    @project.team << [user2, :master]
+    @project.team << [guest_user, :guest]
     visit namespace_project_settings_ci_cd_path(@project.namespace, @project)
   end
 
-  context 'create a trigger' do
-    before do
-      click_on 'Add trigger'
-      expect(@project.triggers.count).to eq(1)
+  describe 'create trigger workflow' do
+    scenario 'prevents adding new trigger with no description' do
+      fill_in 'trigger_description', with: ''
+      click_button 'Add trigger'
+
+      # See if input has error due to empty value
+      expect(page.find('form.gl-show-field-errors .gl-field-error')['style']).to eq 'display: block;'
+    end
+
+    scenario 'adds new trigger with description' do
+      fill_in 'trigger_description', with: 'trigger desc'
+      click_button 'Add trigger'
+
+      # See if "trigger creation successful" message displayed and description and owner are correct
+      expect(page.find('.flash-notice')).to have_content 'Trigger was created successfully.'
+      expect(page.find('.triggers-list')).to have_content 'trigger desc'
+      expect(page.find('.triggers-list .trigger-owner')).to have_content @user.name
+    end
+  end
+
+  describe 'edit trigger workflow' do
+    let(:new_trigger_title) { 'new trigger' }
+
+    scenario 'click on edit trigger opens edit trigger page' do
+      create(:ci_trigger, owner: user, project: @project, description: trigger_title)
+      visit namespace_project_settings_ci_cd_path(@project.namespace, @project)
+
+      # See if edit page has correct descrption
+      find('a[title="Edit"]').click
+      expect(page.find('#trigger_description').value).to have_content 'trigger desc'
+    end
+
+    scenario 'edit trigger and save' do
+      create(:ci_trigger, owner: user, project: @project, description: trigger_title)
+      visit namespace_project_settings_ci_cd_path(@project.namespace, @project)
+
+      # See if edit page opens, then fill in new description and save
+      find('a[title="Edit"]').click
+      fill_in 'trigger_description', with: new_trigger_title
+      click_button 'Save trigger'
+
+      # See if "trigger updated successfully" message displayed and description and owner are correct
+      expect(page.find('.flash-notice')).to have_content 'Trigger was successfully updated.'
+      expect(page.find('.triggers-list')).to have_content new_trigger_title
+      expect(page.find('.triggers-list .trigger-owner')).to have_content @user.name
+    end
+
+    scenario 'edit "legacy" trigger and save' do
+      # Create new trigger without owner association, i.e. Legacy trigger
+      create(:ci_trigger, owner: nil, project: @project)
+      visit namespace_project_settings_ci_cd_path(@project.namespace, @project)
+
+      # See if the trigger can be edited and description is blank
+      find('a[title="Edit"]').click
+      expect(page.find('#trigger_description').value).to have_content ''
+
+      # See if trigger can be updated with description and saved successfully
+      fill_in 'trigger_description', with: new_trigger_title
+      click_button 'Save trigger'
+      expect(page.find('.flash-notice')).to have_content 'Trigger was successfully updated.'
+      expect(page.find('.triggers-list')).to have_content new_trigger_title
+    end
+  end
+
+  describe 'trigger "Take ownership" workflow' do
+    before(:each) do
+      create(:ci_trigger, owner: user2, project: @project, description: trigger_title)
+      visit namespace_project_settings_ci_cd_path(@project.namespace, @project)
+    end
+
+    scenario 'button "Take ownership" has correct alert' do
+      expected_alert = 'By taking ownership you will bind this trigger to your user account. With this the trigger will have access to all your projects as if it was you. Are you sure?'
+      expect(page.find('a.btn-trigger-take-ownership')['data-confirm']).to eq expected_alert
     end
 
-    it 'contains trigger token' do
-      expect(page).to have_content(@project.triggers.first.token)
+    scenario 'take trigger ownership' do
+      # See if "Take ownership" on trigger works post trigger creation
+      find('a.btn-trigger-take-ownership').click
+      page.accept_confirm do
+        expect(page.find('.flash-notice')).to have_content 'Trigger was re-assigned.'
+        expect(page.find('.triggers-list')).to have_content trigger_title
+        expect(page.find('.triggers-list .trigger-owner')).to have_content @user.name
+      end
     end
+  end
+
+  describe 'trigger "Revoke" workflow' do
+    before(:each) do
+      create(:ci_trigger, owner: user2, project: @project, description: trigger_title)
+      visit namespace_project_settings_ci_cd_path(@project.namespace, @project)
+    end
+
+    scenario 'button "Revoke" has correct alert' do
+      expected_alert = 'By revoking a trigger you will break any processes making use of it. Are you sure?'
+      expect(page.find('a.btn-trigger-revoke')['data-confirm']).to eq expected_alert
+    end
+
+    scenario 'revoke trigger' do
+      # See if "Revoke" on trigger works post trigger creation
+      find('a.btn-trigger-revoke').click
+      page.accept_confirm do
+        expect(page.find('.flash-notice')).to have_content 'Trigger removed'
+        expect(page).to have_selector('p.settings-message.text-center.append-bottom-default')
+      end
+    end
+  end
+
+  describe 'show triggers workflow' do
+    scenario 'contains trigger description placeholder' do
+      expect(page.find('#trigger_description')['placeholder']).to eq 'Trigger description'
+    end
+
+    scenario 'show "legacy" badge for legacy trigger' do
+      create(:ci_trigger, owner: nil, project: @project)
+      visit namespace_project_settings_ci_cd_path(@project.namespace, @project)
+
+      # See if trigger without owner (i.e. legacy) shows "legacy" badge and is editable
+      expect(page.find('.triggers-list')).to have_content 'legacy'
+      expect(page.find('.triggers-list')).to have_selector('a[title="Edit"]')
+    end
+
+    scenario 'show "invalid" badge for trigger with owner having insufficient permissions' do
+      create(:ci_trigger, owner: guest_user, project: @project, description: trigger_title)
+      visit namespace_project_settings_ci_cd_path(@project.namespace, @project)
+
+      # See if trigger without owner (i.e. legacy) shows "legacy" badge and is non-editable
+      expect(page.find('.triggers-list')).to have_content 'invalid'
+      expect(page.find('.triggers-list')).not_to have_selector('a[title="Edit"]')
+    end
+
+    scenario 'do not show "Edit" or full token for not owned trigger' do
+      # Create trigger with user different from current_user
+      create(:ci_trigger, owner: user2, project: @project, description: trigger_title)
+      visit namespace_project_settings_ci_cd_path(@project.namespace, @project)
+
+      # See if trigger not owned by current_user shows only first few token chars and doesn't have copy-to-clipboard button
+      expect(page.find('.triggers-list')).to have_content(@project.triggers.first.token[0..3])
+      expect(page.find('.triggers-list')).not_to have_selector('button.btn-clipboard')
+
+      # See if trigger owner name doesn't match with current_user and trigger is non-editable
+      expect(page.find('.triggers-list .trigger-owner')).not_to have_content @user.name
+      expect(page.find('.triggers-list')).not_to have_selector('a[title="Edit"]')
+    end
+
+    scenario 'show "Edit" and full token for owned trigger' do
+      create(:ci_trigger, owner: user, project: @project, description: trigger_title)
+      visit namespace_project_settings_ci_cd_path(@project.namespace, @project)
+
+      # See if trigger shows full token and has copy-to-clipboard button
+      expect(page.find('.triggers-list')).to have_content @project.triggers.first.token
+      expect(page.find('.triggers-list')).to have_selector('button.btn-clipboard')
 
-    it 'revokes the trigger' do
-      click_on 'Revoke'
-      expect(@project.triggers.count).to eq(0)
+      # See if trigger owner name matches with current_user and is editable
+      expect(page.find('.triggers-list .trigger-owner')).to have_content @user.name
+      expect(page.find('.triggers-list')).to have_selector('a[title="Edit"]')
     end
   end
 end
diff --git a/spec/models/ci/trigger_spec.rb b/spec/models/ci/trigger_spec.rb
index 074cf1a0bd7..1bcb673cb16 100644
--- a/spec/models/ci/trigger_spec.rb
+++ b/spec/models/ci/trigger_spec.rb
@@ -22,4 +22,62 @@ describe Ci::Trigger, models: true do
       expect(trigger.token).to eq('token')
     end
   end
+
+  describe '#short_token' do
+    let(:trigger) { create(:ci_trigger, token: '12345678') }
+
+    subject { trigger.short_token }
+
+    it 'returns shortened token' do
+      is_expected.to eq('1234')
+    end
+  end
+
+  describe '#legacy?' do
+    let(:trigger) { create(:ci_trigger, owner: owner, project: project) }
+
+    subject { trigger }
+
+    context 'when owner is blank' do
+      let(:owner) { nil }
+
+      it { is_expected.to be_legacy }
+    end
+
+    context 'when owner is set' do
+      let(:owner) { create(:user) }
+
+      it { is_expected.not_to be_legacy }
+    end
+  end
+
+  describe '#can_access_project?' do
+    let(:trigger) { create(:ci_trigger, owner: owner, project: project) }
+
+    context 'when owner is blank' do
+      let(:owner) { nil }
+
+      subject { trigger.can_access_project? }
+
+      it { is_expected.to eq(true) }
+    end
+
+    context 'when owner is set' do
+      let(:owner) { create(:user) }
+
+      subject { trigger.can_access_project? }
+
+      context 'and is member of the project' do
+        before do
+          project.team << [owner, :developer]
+        end
+
+        it { is_expected.to eq(true) }
+      end
+
+      context 'and is not member of the project' do
+        it { is_expected.to eq(false) }
+      end
+    end
+  end
 end
diff --git a/spec/policies/ci/trigger_policy_spec.rb b/spec/policies/ci/trigger_policy_spec.rb
new file mode 100644
index 00000000000..63ad5eb7322
--- /dev/null
+++ b/spec/policies/ci/trigger_policy_spec.rb
@@ -0,0 +1,103 @@
+require 'spec_helper'
+
+describe Ci::TriggerPolicy, :models do
+  let(:user) { create(:user) }
+  let(:project) { create(:empty_project) }
+  let(:trigger) { create(:ci_trigger, project: project, owner: owner) }
+
+  let(:policies) do
+    described_class.abilities(user, trigger).to_set
+  end
+
+  shared_examples 'allows to admin and manage trigger' do
+    it 'does include ability to admin trigger' do
+      expect(policies).to include :admin_trigger
+    end
+
+    it 'does include ability to manage trigger' do
+      expect(policies).to include :manage_trigger
+    end
+  end
+
+  shared_examples 'allows to manage trigger' do
+    it 'does not include ability to admin trigger' do
+      expect(policies).not_to include :admin_trigger
+    end
+
+    it 'does include ability to manage trigger' do
+      expect(policies).to include :manage_trigger
+    end
+  end
+
+  shared_examples 'disallows to admin and manage trigger' do
+    it 'does not include ability to admin trigger' do
+      expect(policies).not_to include :admin_trigger
+    end
+
+    it 'does not include ability to manage trigger' do
+      expect(policies).not_to include :manage_trigger
+    end
+  end
+
+  describe '#rules' do
+    context 'when owner is undefined' do
+      let(:owner) { nil }
+
+      context 'when user is master of the project' do
+        before do
+          project.team << [user, :master]
+        end
+
+        it_behaves_like 'allows to admin and manage trigger'
+      end
+
+      context 'when user is developer of the project' do
+        before do
+          project.team << [user, :developer]
+        end
+
+        it_behaves_like 'disallows to admin and manage trigger'
+      end
+
+      context 'when user is not member of the project' do
+        it_behaves_like 'disallows to admin and manage trigger'
+      end
+    end
+
+    context 'when owner is an user' do
+      let(:owner) { user }
+
+      context 'when user is master of the project' do
+        before do
+          project.team << [user, :master]
+        end
+
+        it_behaves_like 'allows to admin and manage trigger'
+      end
+    end
+
+    context 'when owner is another user' do
+      let(:owner) { create(:user) }
+
+      context 'when user is master of the project' do
+        before do
+          project.team << [user, :master]
+        end
+
+        it_behaves_like 'allows to manage trigger'
+      end
+
+      context 'when user is developer of the project' do
+        before do
+          project.team << [user, :developer]
+        end
+
+        it_behaves_like 'disallows to admin and manage trigger'
+      end
+
+      context 'when user is not member of the project' do
+        it_behaves_like 'disallows to admin and manage trigger'
+      end
+    end
+  end
+end