Merge branch 'master' into rs-sign_in

8 files changed, 219 insertions, 36 deletions

diff --git a/spec/features/group_variables_spec.rb b/spec/features/group_variables_spec.rb
new file mode 100644
index 00000000000..37814ba6238
--- /dev/null
+++ b/spec/features/group_variables_spec.rb
@@ -0,0 +1,78 @@
+require 'spec_helper'
+
+feature 'Group variables', js: true do
+  let(:user) { create(:user) }
+  let(:group) { create(:group) }
+
+  background do
+    group.add_master(user)
+    gitlab_sign_in(user)
+  end
+
+  context 'when user creates a new variable' do
+    background do
+      visit group_settings_ci_cd_path(group)
+      fill_in 'variable_key', with: 'AAA'
+      fill_in 'variable_value', with: 'AAA123'
+      find(:css, "#variable_protected").set(true)
+      click_on 'Add new variable'
+    end
+
+    scenario 'user sees the created variable' do
+      page.within('.variables-table') do
+        expect(find(".variable-key")).to have_content('AAA')
+        expect(find(".variable-value")).to have_content('******')
+        expect(find(".variable-protected")).to have_content('Yes')
+      end
+      click_on 'Reveal Values'
+      page.within('.variables-table') do
+        expect(find(".variable-value")).to have_content('AAA123')
+      end
+    end
+  end
+
+  context 'when user edits a variable' do
+    background do
+      create(:ci_group_variable, key: 'AAA', value: 'AAA123', protected: true,
+                                 group: group)
+
+      visit group_settings_ci_cd_path(group)
+
+      page.within('.variable-menu') do
+        click_on 'Update'
+      end
+
+      fill_in 'variable_key', with: 'BBB'
+      fill_in 'variable_value', with: 'BBB123'
+      find(:css, "#variable_protected").set(false)
+      click_on 'Save variable'
+    end
+
+    scenario 'user sees the updated variable' do
+      page.within('.variables-table') do
+        expect(find(".variable-key")).to have_content('BBB')
+        expect(find(".variable-value")).to have_content('******')
+        expect(find(".variable-protected")).to have_content('No')
+      end
+    end
+  end
+
+  context 'when user deletes a variable' do
+    background do
+      create(:ci_group_variable, key: 'BBB', value: 'BBB123', protected: false,
+                                 group: group)
+
+      visit group_settings_ci_cd_path(group)
+
+      page.within('.variable-menu') do
+        page.accept_alert 'Are you sure?' do
+          click_on 'Remove'
+        end
+      end
+    end
+
+    scenario 'user does not see the deleted variable' do
+      expect(page).to have_no_css('.variables-table')
+    end
+  end
+end
diff --git a/spec/features/groups/milestone_spec.rb b/spec/features/groups/milestone_spec.rb
index bd5a8645609..0f3f005040f 100644
--- a/spec/features/groups/milestone_spec.rb
+++ b/spec/features/groups/milestone_spec.rb
@@ -33,4 +33,32 @@ feature 'Group milestones', :feature, :js do
       expect(find('.start_date')).to have_content(Date.today.at_beginning_of_month.strftime('%b %-d, %Y'))
     end
   end
+
+  context 'milestones list' do
+    let!(:other_project) { create(:project_empty_repo, group: group) }
+
+    let!(:active_group_milestone) { create(:milestone, group: group, state: 'active') }
+    let!(:active_project_milestone1) { create(:milestone, project: project, state: 'active', title: 'v1.0') }
+    let!(:active_project_milestone2) { create(:milestone, project: other_project, state: 'active', title: 'v1.0') }
+    let!(:closed_group_milestone) { create(:milestone, group: group, state: 'closed') }
+    let!(:closed_project_milestone1) { create(:milestone, project: project, state: 'closed', title: 'v2.0') }
+    let!(:closed_project_milestone2) { create(:milestone, project: other_project, state: 'closed', title: 'v2.0') }
+
+    before do
+      visit group_milestones_path(group)
+    end
+
+    it 'counts milestones correctly' do
+      expect(find('.top-area .active .badge').text).to eq("2")
+      expect(find('.top-area .closed .badge').text).to eq("2")
+      expect(find('.top-area .all .badge').text).to eq("4")
+    end
+
+    it 'lists legacy group milestones and group milestones' do
+      legacy_milestone = GroupMilestone.build_collection(group, group.projects, { state: 'active' }).first
+
+      expect(page).to have_selector("#milestone_#{active_group_milestone.id}", count: 1)
+      expect(page).to have_selector("#milestone_#{legacy_milestone.milestones.first.id}", count: 1)
+    end
+  end
 end
diff --git a/spec/features/issues/form_spec.rb b/spec/features/issues/form_spec.rb
index 60e6e431c9e..05742004f06 100644
--- a/spec/features/issues/form_spec.rb
+++ b/spec/features/issues/form_spec.rb
@@ -23,7 +23,7 @@ describe 'New/edit issue', :feature, :js do
       visit new_project_issue_path(project)
     end
 
-    describe 'shorten users API pagination limit (CE)' do
+    describe 'shorten users API pagination limit' do
       before do
         # Using `allow_any_instance_of`/`and_wrap_original`, `original` would
         # somehow refer to the very block we defined to _wrap_ that method, instead of
@@ -63,7 +63,7 @@ describe 'New/edit issue', :feature, :js do
       end
     end
 
-    describe 'single assignee (CE)' do
+    describe 'single assignee' do
       before do
         click_button 'Unassigned'
 
diff --git a/spec/features/milestone_spec.rb b/spec/features/milestone_spec.rb
index 189b277cd16..ce0c27cbe77 100644
--- a/spec/features/milestone_spec.rb
+++ b/spec/features/milestone_spec.rb
@@ -1,10 +1,12 @@
 require 'rails_helper'
 
 feature 'Milestone', feature: true do
-  let(:project) { create(:empty_project, :public) }
+  let(:group) { create(:group, :public) }
+  let(:project) { create(:empty_project, :public, namespace: group) }
   let(:user)   { create(:user) }
 
   before do
+    create(:group_member, group: group, user: user)
     project.team << [user, :master]
     sign_in(user)
   end
@@ -37,8 +39,8 @@ feature 'Milestone', feature: true do
     end
   end
 
-  feature 'Open a milestone with an existing title' do
-    scenario 'displays validation message' do
+  feature 'Open a project milestone with an existing title' do
+    scenario 'displays validation message when there is a project milestone with same title' do
       milestone = create(:milestone, project: project, title: 8.7)
 
       visit new_project_milestone_path(project)
@@ -47,7 +49,20 @@ feature 'Milestone', feature: true do
       end
       find('input[name="commit"]').click
 
-      expect(find('.alert-danger')).to have_content('Title has already been taken')
+      expect(find('.alert-danger')).to have_content('already being used for another group or project milestone.')
+    end
+
+    scenario 'displays validation message when there is a group milestone with same title' do
+      milestone = create(:milestone, project_id: nil, group: project.group, title: 8.7)
+
+      visit new_group_milestone_path(project.group)
+
+      page.within '.milestone-form' do
+        fill_in "milestone_title", with: milestone.title
+      end
+      find('input[name="commit"]').click
+
+      expect(find('.alert-danger')).to have_content('already being used for another group or project milestone.')
     end
   end
 end
diff --git a/spec/features/projects/members/anonymous_user_sees_members_spec.rb b/spec/features/projects/members/anonymous_user_sees_members_spec.rb
index 4958d5594ac..28c8d20aad5 100644
--- a/spec/features/projects/members/anonymous_user_sees_members_spec.rb
+++ b/spec/features/projects/members/anonymous_user_sees_members_spec.rb
@@ -11,10 +11,10 @@ feature 'Projects > Members > Anonymous user sees members', feature: true do
   end
 
   scenario "anonymous user visits the project's members page and sees the list of members" do
-    visit project_settings_members_path(project)
+    visit project_project_members_path(project)
 
     expect(current_path).to eq(
-      project_settings_members_path(project))
+      project_project_members_path(project))
     expect(page).to have_content(user.name)
   end
 end
diff --git a/spec/features/projects/members/user_requests_access_spec.rb b/spec/features/projects/members/user_requests_access_spec.rb
index 1cc4627a7e3..ab86e2da4f6 100644
--- a/spec/features/projects/members/user_requests_access_spec.rb
+++ b/spec/features/projects/members/user_requests_access_spec.rb
@@ -46,10 +46,11 @@ feature 'Projects > Members > User requests access', feature: true do
 
     expect(project.requesters.exists?(user_id: user)).to be_truthy
 
-    open_project_settings_menu
-    click_link 'Members'
+    page.within('.layout-nav .nav-links') do
+      click_link('Members')
+    end
 
-    visit project_settings_members_path(project)
+    visit project_project_members_path(project)
     page.within('.content') do
       expect(page).not_to have_content(user.name)
     end
diff --git a/spec/features/projects/pipeline_schedules_spec.rb b/spec/features/projects/pipeline_schedules_spec.rb
index 60e6549f703..992a68b25a5 100644
--- a/spec/features/projects/pipeline_schedules_spec.rb
+++ b/spec/features/projects/pipeline_schedules_spec.rb
@@ -1,6 +1,6 @@
 require 'spec_helper'
 
-feature 'Pipeline Schedules', :feature do
+feature 'Pipeline Schedules', :feature, js: true do
   include PipelineSchedulesHelper
 
   let!(:project) { create(:project) }
@@ -11,27 +11,20 @@ feature 'Pipeline Schedules', :feature do
 
   before do
     project.add_master(user)
-
     sign_in(user)
-    visit_page
   end
 
   describe 'GET /projects/pipeline_schedules' do
-    let(:visit_page) { visit_pipelines_schedules }
-
-    it 'avoids N + 1 queries' do
-      control_count = ActiveRecord::QueryRecorder.new { visit_pipelines_schedules }.count
-
-      create_list(:ci_pipeline_schedule, 2, project: project)
-
-      expect { visit_pipelines_schedules }.not_to exceed_query_limit(control_count)
+    before do
+      visit_pipelines_schedules
     end
 
     describe 'The view' do
       it 'displays the required information description' do
         page.within('.pipeline-schedule-table-row') do
           expect(page).to have_content('pipeline schedule')
-          expect(page).to have_content(pipeline_schedule.real_next_run.strftime('%b %d, %Y'))
+          expect(find(".next-run-cell time")['data-original-title'])
+            .to include(pipeline_schedule.real_next_run.strftime('%b %-d, %Y'))
           expect(page).to have_link('master')
           expect(page).to have_link("##{pipeline.id}")
         end
@@ -62,7 +55,7 @@ feature 'Pipeline Schedules', :feature do
       it 'deletes the pipeline' do
         click_link 'Delete'
 
-        expect(page).not_to have_content('pipeline schedule')
+        expect(page).not_to have_css(".pipeline-schedule-table-row")
       end
     end
 
@@ -78,8 +71,10 @@ feature 'Pipeline Schedules', :feature do
     end
   end
 
-  describe 'POST /projects/pipeline_schedules/new', js: true do
-    let(:visit_page) { visit_new_pipeline_schedule }
+  describe 'POST /projects/pipeline_schedules/new' do
+    before do
+      visit_new_pipeline_schedule
+    end
 
     it 'sets defaults for timezone and target branch' do
       expect(page).to have_button('master')
@@ -100,8 +95,8 @@ feature 'Pipeline Schedules', :feature do
     end
   end
 
-  describe 'PATCH /projects/pipelines_schedules/:id/edit', js: true do
-    let(:visit_page) do
+  describe 'PATCH /projects/pipelines_schedules/:id/edit' do
+    before do
       edit_pipeline_schedule
     end
 
@@ -134,6 +129,72 @@ feature 'Pipeline Schedules', :feature do
     end
   end
 
+  context 'when user creates a new pipeline schedule with variables' do
+    background do
+      visit_pipelines_schedules
+      click_link 'New schedule'
+      fill_in_schedule_form
+      all('[name="schedule[variables_attributes][][key]"]')[0].set('AAA')
+      all('[name="schedule[variables_attributes][][value]"]')[0].set('AAA123')
+      all('[name="schedule[variables_attributes][][key]"]')[1].set('BBB')
+      all('[name="schedule[variables_attributes][][value]"]')[1].set('BBB123')
+      save_pipeline_schedule
+    end
+
+    scenario 'user sees the new variable in edit window' do
+      find(".content-list .pipeline-schedule-table-row:nth-child(1) .btn-group a[title='Edit']").click
+      page.within('.pipeline-variable-list') do
+        expect(find(".pipeline-variable-row:nth-child(1) .pipeline-variable-key-input").value).to eq('AAA')
+        expect(find(".pipeline-variable-row:nth-child(1) .pipeline-variable-value-input").value).to eq('AAA123')
+        expect(find(".pipeline-variable-row:nth-child(2) .pipeline-variable-key-input").value).to eq('BBB')
+        expect(find(".pipeline-variable-row:nth-child(2) .pipeline-variable-value-input").value).to eq('BBB123')
+      end
+    end
+  end
+
+  context 'when user edits a variable of a pipeline schedule' do
+    background do
+      create(:ci_pipeline_schedule, project: project, owner: user).tap do |pipeline_schedule|
+        create(:ci_pipeline_schedule_variable, key: 'AAA', value: 'AAA123', pipeline_schedule: pipeline_schedule)
+      end
+
+      visit_pipelines_schedules
+      find(".content-list .pipeline-schedule-table-row:nth-child(1) .btn-group a[title='Edit']").click
+      all('[name="schedule[variables_attributes][][key]"]')[0].set('foo')
+      all('[name="schedule[variables_attributes][][value]"]')[0].set('bar')
+      click_button 'Save pipeline schedule'
+    end
+
+    scenario 'user sees the updated variable in edit window' do
+      find(".content-list .pipeline-schedule-table-row:nth-child(1) .btn-group a[title='Edit']").click
+      page.within('.pipeline-variable-list') do
+        expect(find(".pipeline-variable-row:nth-child(1) .pipeline-variable-key-input").value).to eq('foo')
+        expect(find(".pipeline-variable-row:nth-child(1) .pipeline-variable-value-input").value).to eq('bar')
+      end
+    end
+  end
+
+  context 'when user removes a variable of a pipeline schedule' do
+    background do
+      create(:ci_pipeline_schedule, project: project, owner: user).tap do |pipeline_schedule|
+        create(:ci_pipeline_schedule_variable, key: 'AAA', value: 'AAA123', pipeline_schedule: pipeline_schedule)
+      end
+
+      visit_pipelines_schedules
+      find(".content-list .pipeline-schedule-table-row:nth-child(1) .btn-group a[title='Edit']").click
+      find('.pipeline-variable-list .pipeline-variable-row-remove-button').click
+      click_button 'Save pipeline schedule'
+    end
+
+    scenario 'user does not see the removed variable in edit window' do
+      find(".content-list .pipeline-schedule-table-row:nth-child(1) .btn-group a[title='Edit']").click
+      page.within('.pipeline-variable-list') do
+        expect(find(".pipeline-variable-row:nth-child(1) .pipeline-variable-key-input").value).to eq('')
+        expect(find(".pipeline-variable-row:nth-child(1) .pipeline-variable-value-input").value).to eq('')
+      end
+    end
+  end
+
   def visit_new_pipeline_schedule
     visit new_project_pipeline_schedule_path(project, pipeline_schedule)
   end
diff --git a/spec/features/variables_spec.rb b/spec/features/variables_spec.rb
index 0d2b3a25e93..dd770fe5043 100644
--- a/spec/features/variables_spec.rb
+++ b/spec/features/variables_spec.rb
@@ -24,7 +24,7 @@ describe 'Project variables', js: true do
     fill_in('variable_value', with: 'key value')
     click_button('Add new variable')
 
-    expect(page).to have_content('Variables were successfully updated.')
+    expect(page).to have_content('Variable was successfully created.')
     page.within('.variables-table') do
       expect(page).to have_content('key')
       expect(page).to have_content('No')
@@ -36,7 +36,7 @@ describe 'Project variables', js: true do
     fill_in('variable_value', with: '')
     click_button('Add new variable')
 
-    expect(page).to have_content('Variables were successfully updated.')
+    expect(page).to have_content('Variable was successfully created.')
     page.within('.variables-table') do
       expect(page).to have_content('new_key')
     end
@@ -48,7 +48,7 @@ describe 'Project variables', js: true do
     check('Protected')
     click_button('Add new variable')
 
-    expect(page).to have_content('Variables were successfully updated.')
+    expect(page).to have_content('Variable was successfully created.')
     page.within('.variables-table') do
       expect(page).to have_content('key')
       expect(page).to have_content('Yes')
@@ -82,7 +82,7 @@ describe 'Project variables', js: true do
 
   it 'deletes variable' do
     page.within('.variables-table') do
-      find('.btn-variable-delete').click
+      click_on 'Remove'
     end
 
     expect(page).not_to have_selector('variables-table')
@@ -90,7 +90,7 @@ describe 'Project variables', js: true do
 
   it 'edits variable' do
     page.within('.variables-table') do
-      find('.btn-variable-edit').click
+      click_on 'Update'
     end
 
     expect(page).to have_content('Update variable')
@@ -104,7 +104,7 @@ describe 'Project variables', js: true do
 
   it 'edits variable with empty value' do
     page.within('.variables-table') do
-      find('.btn-variable-edit').click
+      click_on 'Update'
     end
 
     expect(page).to have_content('Update variable')
@@ -117,7 +117,7 @@ describe 'Project variables', js: true do
 
   it 'edits variable to be protected' do
     page.within('.variables-table') do
-      find('.btn-variable-edit').click
+      click_on 'Update'
     end
 
     expect(page).to have_content('Update variable')
@@ -132,7 +132,7 @@ describe 'Project variables', js: true do
     project.variables.first.update(protected: true)
 
     page.within('.variables-table') do
-      find('.btn-variable-edit').click
+      click_on 'Update'
     end
 
     expect(page).to have_content('Update variable')