Add feature specs for multiple environment feature flag

and improve validation

Add default value for active

Add spec

Fix coding offence

Add more spec

Concrete specs

Improve coding style

Fix coding offence

Add frozen_string_literal

ee/app/models/operations/feature_flag.rb

@@ -11,6 +11,8 @@ module Operations
 
     belongs_to :project
 
+    default_value_for :active, true
+
     has_many :scopes, class_name: 'Operations::FeatureFlagScope'
     has_one :default_scope, -> { where(environment_scope: '*') }, class_name: 'Operations::FeatureFlagScope'
 
@@ -24,8 +26,9 @@ module Operations
       }
     validates :name, uniqueness: { scope: :project_id }
     validates :description, allow_blank: true, length: 0..255
+    validate :first_default_scope, on: :create, if: :has_scopes?
 
-    before_create :build_default_scope
+    before_create :build_default_scope, unless: :has_scopes?
     after_update :update_default_scope
 
     accepts_nested_attributes_for :scopes, allow_destroy: true
@@ -87,10 +90,20 @@ module Operations
 
     private
 
+    def first_default_scope
+      unless scopes.first.environment_scope == '*'
+        errors.add(:default_scope, 'has to be the first element')
+      end
+    end
+
     def build_default_scope
       scopes.build(environment_scope: '*', active: self.active)
     end
 
+    def has_scopes?
+      scopes.any?
+    end
+
     def update_default_scope
       default_scope.update(active: self.active) if self.active_changed?
     end

ee/spec/controllers/projects/feature_flags_controller_spec.rb

@@ -393,7 +393,8 @@ describe Projects::FeatureFlagsController do
           operations_feature_flag: {
             name: 'my_feature_flag',
             active: true,
-            scopes_attributes: [{ environment_scope: 'production', active: false }]
+            scopes_attributes: [{ environment_scope: '*', active: true },
+                                { environment_scope: 'production', active: false }]
           }
         })
       end
@@ -403,6 +404,34 @@ describe Projects::FeatureFlagsController do
 
         expect(response).to have_gitlab_http_status(200)
       end
+
+      it 'creates feature flag scopes in a correct order' do
+        subject
+
+        expect(json_response['scopes'].first['environment_scope']).to eq('*')
+        expect(json_response['scopes'].second['environment_scope']).to eq('production')
+      end
+
+      context 'when default scope is not placed first' do
+        let(:params) do
+          view_params.merge({
+            operations_feature_flag: {
+              name: 'my_feature_flag',
+              active: true,
+              scopes_attributes: [{ environment_scope: 'production', active: false },
+                                  { environment_scope: '*', active: true }]
+            }
+          })
+        end
+
+        it 'returns 400' do
+          subject
+
+          expect(response).to have_gitlab_http_status(400)
+          expect(json_response['message'])
+            .to include('Default scope has to be the first element')
+        end
+      end
     end
   end
 

ee/spec/features/projects/feature_flags/user_creates_feature_flag_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe 'User creates feature flag', :js do
+  include FeatureFlagHelpers
+
+  let(:user) { create(:user) }
+  let(:project) { create(:project, namespace: user.namespace) }
+
+  before do
+    project.add_developer(user)
+    stub_licensed_features(feature_flags: true)
+    sign_in(user)
+  end
+
+  context 'when creates without changing scopes' do
+    before do
+      visit(new_project_feature_flag_path(project))
+      set_feature_flag_info('ci_live_trace', 'For live trace')
+      click_button 'Create feature flag'
+    end
+
+    it 'shows the created feature flag' do
+      within_feature_flag_row(1) do
+        expect(page.find('.feature-flag-name')).to have_content('ci_live_trace')
+        expect(page).to have_css('.js-feature-flag-status .badge-success')
+
+        within_feature_flag_scopes do
+          expect(page.find('.badge:nth-child(1)')).to have_content('*')
+          expect(page.find('.badge:nth-child(1)')['class']).to include('badge-active')
+        end
+      end
+    end
+  end
+
+  context 'when creates with disabling the default scope' do
+    before do
+      visit(new_project_feature_flag_path(project))
+      set_feature_flag_info('ci_live_trace', 'For live trace')
+
+      within_scope_row(1) do
+        within_status { find('.project-feature-toggle').click }
+      end
+
+      click_button 'Create feature flag'
+    end
+
+    it 'shows the created feature flag' do
+      within_feature_flag_row(1) do
+        expect(page.find('.feature-flag-name')).to have_content('ci_live_trace')
+        expect(page).to have_css('.js-feature-flag-status .badge-danger')
+
+        within_feature_flag_scopes do
+          expect(page.find('.badge:nth-child(1)')).to have_content('*')
+          expect(page.find('.badge:nth-child(1)')['class']).to include('badge-inactive')
+        end
+      end
+    end
+  end
+
+  context 'when creates with an additional scope' do
+    before do
+      visit(new_project_feature_flag_path(project))
+      set_feature_flag_info('mr_train', '')
+
+      within_scope_row(2) do
+        within_environment_spec { find('.js-new-scope-name').set("review/*") }
+      end
+
+      within_scope_row(2) do
+        within_status { find('.project-feature-toggle').click }
+      end
+
+      click_button 'Create feature flag'
+    end
+
+    it 'shows the created feature flag' do
+      within_feature_flag_row(1) do
+        expect(page.find('.feature-flag-name')).to have_content('mr_train')
+        expect(page).to have_css('.js-feature-flag-status .badge-success')
+
+        within_feature_flag_scopes do
+          expect(page.find('.badge:nth-child(1)')).to have_content('*')
+          expect(page.find('.badge:nth-child(1)')['class']).to include('badge-active')
+          expect(page.find('.badge:nth-child(2)')).to have_content('review/*')
+          expect(page.find('.badge:nth-child(2)')['class']).to include('badge-active')
+        end
+      end
+    end
+  end
+
+  private
+
+  def set_feature_flag_info(name, description)
+    fill_in 'Name', with: name
+    fill_in 'Description', with: description
+  end
+end

ee/spec/features/projects/feature_flags/user_sees_feature_flag_list_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe 'User sees feature flag list', :js do
+  include FeatureFlagHelpers
+
+  let(:user) { create(:user) }
+  let(:project) { create(:project, namespace: user.namespace) }
+
+  before do
+    project.add_developer(user)
+    stub_licensed_features(feature_flags: true)
+    sign_in(user)
+  end
+
+  context 'when there are feature flags and scopes' do
+    before do
+      create_flag(project, 'ci_live_trace', false).tap do |feature_flag|
+        create_scope(feature_flag, 'review/*', true)
+      end
+      create_flag(project, 'drop_legacy_artifacts', false)
+      create_flag(project, 'mr_train', true).tap do |feature_flag|
+        create_scope(feature_flag, 'production', false)
+      end
+
+      visit(project_feature_flags_path(project))
+    end
+
+    it 'user sees the first flag' do
+      within_feature_flag_row(1) do
+        expect(page.find('.feature-flag-name')).to have_content('ci_live_trace')
+        expect(page).to have_css('.js-feature-flag-status .badge-success')
+
+        within_feature_flag_scopes do
+          expect(page.find('.badge:nth-child(1)')).to have_content('*')
+          expect(page.find('.badge:nth-child(1)')['class']).to include('badge-inactive')
+          expect(page.find('.badge:nth-child(2)')).to have_content('review/*')
+          expect(page.find('.badge:nth-child(2)')['class']).to include('badge-active')
+        end
+      end
+    end
+
+    it 'user sees the second flag' do
+      within_feature_flag_row(2) do
+        expect(page.find('.feature-flag-name')).to have_content('drop_legacy_artifacts')
+        expect(page).to have_css('.js-feature-flag-status .badge-danger')
+
+        within_feature_flag_scopes do
+          expect(page.find('.badge:nth-child(1)')).to have_content('*')
+          expect(page.find('.badge:nth-child(1)')['class']).to include('badge-inactive')
+        end
+      end
+    end
+
+    it 'user sees the third flag' do
+      within_feature_flag_row(3) do
+        expect(page.find('.feature-flag-name')).to have_content('mr_train')
+        expect(page).to have_css('.js-feature-flag-status .badge-success')
+
+        within_feature_flag_scopes do
+          expect(page.find('.badge:nth-child(1)')).to have_content('*')
+          expect(page.find('.badge:nth-child(1)')['class']).to include('badge-active')
+          expect(page.find('.badge:nth-child(2)')).to have_content('production')
+          expect(page.find('.badge:nth-child(2)')['class']).to include('badge-inactive')
+        end
+      end
+    end
+  end
+
+  context 'when there are no feature flags' do
+    before do
+      visit(project_feature_flags_path(project))
+    end
+
+    it 'shows empty page' do
+      expect(page).to have_text 'Get started with feature flags'
+      expect(page).to have_selector('.btn-success', text: 'New Feature Flag')
+      expect(page).to have_selector('.btn-primary.btn-inverted', text: 'Configure')
+    end
+  end
+end

ee/spec/features/projects/feature_flags/user_updates_feature_flag_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe 'User updates feature flag', :js do
+  include FeatureFlagHelpers
+
+  let(:user) { create(:user) }
+  let(:project) { create(:project, namespace: user.namespace) }
+
+  let!(:feature_flag) do
+    create_flag(project, 'ci_live_trace', false,
+                description: 'For live trace feature')
+  end
+
+  let!(:scope) { create_scope(feature_flag, 'review/*', true) }
+
+  before do
+    project.add_developer(user)
+    stub_licensed_features(feature_flags: true)
+    sign_in(user)
+
+    visit(edit_project_feature_flag_path(project, feature_flag))
+  end
+
+  it 'user sees persisted default scope' do
+    within_scope_row(1) do
+      within_environment_spec do
+        expect(page).to have_content('* (All Environments)')
+      end
+
+      within_status do
+        expect(find('.project-feature-toggle')['aria-label'])
+          .to eq('Toggle Status: OFF')
+      end
+    end
+  end
+
+  context 'when user updates a status of a scope' do
+    before do
+      within_scope_row(2) do
+        within_status { find('.project-feature-toggle').click }
+      end
+
+      click_button 'Save changes'
+    end
+
+    it 'shows the updated feature flag' do
+      within_feature_flag_row(1) do
+        expect(page.find('.feature-flag-name')).to have_content('ci_live_trace')
+        expect(page).to have_css('.js-feature-flag-status .badge-danger')
+
+        within_feature_flag_scopes do
+          expect(page.find('.badge:nth-child(1)')).to have_content('*')
+          expect(page.find('.badge:nth-child(1)')['class']).to include('badge-inactive')
+          expect(page.find('.badge:nth-child(2)')).to have_content('review/*')
+          expect(page.find('.badge:nth-child(2)')['class']).to include('badge-inactive')
+        end
+      end
+    end
+  end
+
+  context 'when user adds a new scope' do
+    before do
+      within_scope_row(3) do
+        within_environment_spec { find('.js-new-scope-name').set('production') }
+      end
+
+      click_button 'Save changes'
+    end
+
+    it 'shows the newly created scope' do
+      within_feature_flag_row(1) do
+        within_feature_flag_scopes do
+          expect(page.find('.badge:nth-child(3)')).to have_content('production')
+          expect(page.find('.badge:nth-child(3)')['class']).to include('badge-inactive')
+        end
+      end
+    end
+  end
+
+  context 'when user deletes a scope' do
+    before do
+      within_scope_row(2) do
+        within_delete { find('.js-delete-scope').click }
+      end
+
+      click_button 'Save changes'
+    end
+
+    it 'shows the updated feature flag' do
+      within_feature_flag_row(1) do
+        within_feature_flag_scopes do
+          expect(page).to have_css('.badge:nth-child(1)')
+          expect(page).not_to have_css('.badge:nth-child(2)')
+        end
+      end
+    end
+  end
+end

ee/spec/models/operations/feature_flag_spec.rb

@@ -16,6 +16,42 @@ describe Operations::FeatureFlag do
     it { is_expected.to validate_uniqueness_of(:name).scoped_to(:project_id) }
   end
 
+  describe 'Scope creation' do
+    subject { described_class.new(**params) }
+
+    let(:project) { create(:project) }
+
+    let(:params) do
+      { name: 'test', project: project, scopes_attributes: scopes_attributes }
+    end
+
+    let(:scopes_attributes) do
+      [{ environment_scope: '*', active: false },
+       { environment_scope: 'review/*', active: true }]
+    end
+
+    it { is_expected.to be_valid }
+
+    context 'when the first scope is not wildcard' do
+      let(:scopes_attributes) do
+        [{ environment_scope: 'review/*', active: true },
+         { environment_scope: '*', active: false }]
+      end
+
+      it { is_expected.not_to be_valid }
+    end
+
+    context 'when scope is empty' do
+      let(:scopes_attributes) { [] }
+
+      it 'creates a default scope' do
+        subject.save
+
+        expect(subject.scopes.first.environment_scope).to eq('*')
+      end
+    end
+  end
+
   describe '.enabled' do
     subject { described_class.enabled }
 

ee/spec/support/helpers/feature_flag_helpers.rb

 # frozen_string_literal: true
 
 module FeatureFlagHelpers
+  def create_flag(project, name, active, description: nil)
+    create(:operations_feature_flag, name: name, active: active,
+                                     description: description, project: project)
+  end
+
   def create_scope(feature_flag, environment_scope, active)
     create(:operations_feature_flag_scope,
       feature_flag: feature_flag,
       environment_scope: environment_scope,
       active: active)
   end
+
+  def within_feature_flag_row(index)
+    within ".gl-responsive-table-row:nth-child(#{index + 1})" do
+      yield
+    end
+  end
+
+  def within_feature_flag_scopes
+    within '.js-feature-flag-environments' do
+      yield
+    end
+  end
+
+  def within_scope_row(index)
+    within ".gl-responsive-table-row:nth-child(#{index + 1})" do
+      yield
+    end
+  end
+
+  def within_environment_spec
+    within '.table-section:nth-child(1)' do
+      yield
+    end
+  end
+
+  def within_status
+    within '.table-section:nth-child(2)' do
+      yield
+    end
+  end
+
+  def within_delete
+    within '.table-section:nth-child(3)' do
+      yield
+    end
+  end
 end