Merge branch '58706-fix-protected-environments-settings-links' into 'master'

Generate correct link when listing protected environments

Closes gitlab-ce#58706

See merge request gitlab-org/gitlab-ee!10470

ee/app/controllers/ee/projects/settings/ci_cd_controller.rb

@@ -24,12 +24,10 @@ module EE
 
         private
 
-        # rubocop: disable CodeReuse/ActiveRecord
         def define_protected_env_variables
-          @protected_environments = @project.protected_environments.order(:name)
+          @protected_environments = @project.protected_environments.with_environment_id.sorted_by_name
           @protected_environment = ProtectedEnvironment.new(project: @project)
         end
-        # rubocop: enable CodeReuse/ActiveRecord
 
         def assign_variables_to_gon
           gon.push(current_project_id: project.id)

ee/app/models/protected_environment.rb

@@ -10,6 +10,15 @@ class ProtectedEnvironment < ApplicationRecord
   validates :deploy_access_levels, length: { minimum: 1 }
   validates :name, :project, presence: true
 
+  scope :sorted_by_name, -> { order(:name) }
+
+  scope :with_environment_id, -> do
+    select('protected_environments.*, environments.id AS environment_id')
+      .joins('LEFT OUTER JOIN environments ON' \
+             ' protected_environments.name = environments.name ' \
+             ' AND protected_environments.project_id = environments.project_id')
+  end
+
   def accessible_to?(user)
     deploy_access_levels
       .any? { |deploy_access_level| deploy_access_level.check_access(user) }

ee/app/views/projects/protected_environments/_protected_environment.html.haml

 %tr.js-protected-environment-edit-form{ data: { url: namespace_project_protected_environment_path(@project.namespace, @project, protected_environment) } }
   %td
-    %span.ref-name= link_to protected_environment.name, namespace_project_environment_path(@project.namespace, @project, protected_environment.name)
+    - if protected_environment.environment_id?
+      %span.ref-name= link_to protected_environment.name, namespace_project_environment_path(@project.namespace, @project, protected_environment.environment_id)
+    - else
+      %span.ref-name= protected_environment.name
   %td
     = render partial: 'projects/protected_environments/update_deploy_access_level_dropdown', locals: { protected_environment: protected_environment, access_levels: protected_environment.deploy_access_levels, disabled: !can_admin_project }
 

ee/changelogs/unreleased/58706-fix-protected-environments-settings-links.yml

+---
+title: Fix broken links to protected environments on the CI/CD settings page.
+merge_request: 10470
+author:
+type: fixed

ee/spec/features/projects/settings/protected_environments_spec.rb

@@ -15,6 +15,7 @@ describe 'Protected Environments' do
     end
 
     create(:protected_environment, project: project, name: 'production')
+    create(:protected_environment, project: project, name: 'removed environment')
 
     sign_in(user)
   end
@@ -45,6 +46,7 @@ describe 'Protected Environments' do
     it 'allows seeing a list of protected environments' do
       within('.protected-environments-list') do
         expect(page).to have_content('production')
+        expect(page).to have_content('removed environment')
       end
     end
 
@@ -64,7 +66,7 @@ describe 'Protected Environments' do
     end
 
     it 'allows updating access to a protected environment', :js do
-      within('.protected-environments-list') do
+      within('.protected-environments-list tr', text: 'production') do
         set_allowed_to_deploy('Developers + Maintainers')
       end
 
@@ -76,7 +78,7 @@ describe 'Protected Environments' do
     end
 
     it 'allows unprotecting an environment', :js do
-      within('.protected-environments-list') do
+      within('.protected-environments-list tr', text: 'production') do
         accept_alert { click_on('Unprotect') }
       end
 

ee/spec/models/protected_environment_spec.rb

@@ -90,6 +90,32 @@ describe ProtectedEnvironment do
     end
   end
 
+  describe '.sorted_by_name' do
+    subject(:protected_environments) { described_class.sorted_by_name }
+
+    it "sorts protected environments by name" do
+      %w(staging production development).each {|name| create(:protected_environment, name: name)}
+
+      expect(protected_environments.map(&:name)).to eq %w(development production staging)
+    end
+  end
+
+  describe '.with_environment_id' do
+    subject(:protected_environments) { described_class.with_environment_id }
+
+    it "sets corresponding environment id if there is environment matching by name and project" do
+      project = create(:project)
+      environment = create(:environment, project: project, name: 'production')
+
+      production = create(:protected_environment, project: project, name: 'production')
+      removed_environment = create(:protected_environment, project: project, name: 'removed environment')
+
+      expect(protected_environments).to match_array [production, removed_environment]
+      expect(protected_environments.find {|e| e.name == 'production'}.environment_id).to eq environment.id
+      expect(protected_environments.find {|e| e.name == 'removed environment'}.environment_id).to be_nil
+    end
+  end
+
   def create_deploy_access_level(**opts)
     protected_environment.deploy_access_levels.create(**opts)
   end

ee/spec/views/projects/protected_environments/_protected_environment.html.haml_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe 'projects/protected_environments/_protected_environment' do
+  it 'displays protected environments without matching environment as text' do
+    protected_environment = build_stubbed(:protected_environment, name: 'production')
+    expect(protected_environment).to receive(:environment_id?).and_return(false)
+    assign :project, protected_environment.project
+
+    render partial: 'projects/protected_environments/protected_environment', locals: { protected_environment: protected_environment, can_admin_project: true }
+
+    expect(rendered).to have_text('production')
+    expect(rendered).not_to have_link('production')
+  end
+
+  it 'displays protected environments with matching environment as link' do
+    protected_environment = build_stubbed(:protected_environment, name: 'production')
+    expect(protected_environment).to receive(:environment_id?).and_return(true)
+    expect(protected_environment).to receive(:environment_id).and_return(314)
+    assign :project, protected_environment.project
+
+    render partial: 'projects/protected_environments/protected_environment', locals: { protected_environment: protected_environment, can_admin_project: true }
+
+    expect(rendered).to have_link('production', href: project_environment_path(protected_environment.project, 314))
+  end
+end