Order Deployments by Finish Time

If manual deployments are deployed out of order, the ordering of the
table gets murky and it is hard to tell which deployment was most
recently deployed.

Instead we can update the deployment ordering so that deployments are
ordered by when they finished at, which forces the finished deployments
to be sorted by most recently finished (read: deployed). Postgres ORDER
BY pushes NULL to the top of the list, so created/running (upcoming)
deployments are first

Changelog: changed

app/controllers/projects/environments_controller.rb

@@ -70,11 +70,9 @@ class Projects::EnvironmentsController < Projects::ApplicationController
   end
   # rubocop: enable CodeReuse/ActiveRecord
 
-  # rubocop: disable CodeReuse/ActiveRecord
   def show
-    @deployments = environment.deployments.order(id: :desc).page(params[:page])
+    @deployments = environment.deployments.ordered.page(params[:page])
   end
-  # rubocop: enable CodeReuse/ActiveRecord
 
   def new
     @environment = project.environments.new

app/models/deployment.rb

@@ -51,6 +51,8 @@ class Deployment < ApplicationRecord
   scope :finished_after, ->(date) { where('finished_at >= ?', date) }
   scope :finished_before, ->(date) { where('finished_at < ?', date) }
 
+  scope :ordered, -> { order(finished_at: :desc) }
+
   FINISHED_STATUSES = %i[success failed canceled].freeze
 
   state_machine :status, initial: :created do

app/views/projects/deployments/_deployment.html.haml

@@ -6,7 +6,7 @@
 
   .table-section.section-10{ role: 'gridcell' }
     .table-mobile-header{ role: 'rowheader' }= _("ID")
-    %strong.table-mobile-content ##{deployment.iid}
+    %strong.table-mobile-content{ data: { testid: 'deployment-id' } } ##{deployment.iid}
 
   .table-section.section-10{ role: 'gridcell' }
     .table-mobile-header{ role: 'rowheader' }= _("Triggerer")

spec/features/projects/environments/environment_spec.rb

@@ -23,10 +23,6 @@ RSpec.describe 'Environment' do
     let!(:action) { }
     let!(:cluster) { }
 
-    before do
-      visit_environment(environment)
-    end
-
     context 'with auto-stop' do
       let!(:environment) { create(:environment, :will_auto_stop, name: 'staging', project: project) }
 
@@ -52,12 +48,20 @@ RSpec.describe 'Environment' do
     end
 
     context 'without deployments' do
+      before do
+        visit_environment(environment)
+      end
+
       it 'does not show deployments' do
         expect(page).to have_content('You don\'t have any deployments right now.')
       end
     end
 
     context 'with deployments' do
+      before do
+        visit_environment(environment)
+      end
+
       context 'when there is no related deployable' do
         let(:deployment) do
           create(:deployment, :success, environment: environment, deployable: nil)
@@ -108,6 +112,26 @@ RSpec.describe 'Environment' do
         end
       end
 
+      context 'with many deployments' do
+        let(:pipeline) { create(:ci_pipeline, project: project) }
+        let(:build) { create(:ci_build, pipeline: pipeline) }
+
+        let!(:second) { create(:deployment, environment: environment, deployable: build, status: :success, finished_at: Time.current) }
+        let!(:first) { create(:deployment, environment: environment, deployable: build, status: :running) }
+        let!(:last) { create(:deployment, environment: environment, deployable: build, status: :success, finished_at: 2.days.ago) }
+        let!(:third) { create(:deployment, environment: environment, deployable: build, status: :canceled, finished_at: 1.day.ago) }
+
+        before do
+          visit_environment(environment)
+        end
+
+        it 'shows all of them in ordered way' do
+          ids = find_all('[data-testid="deployment-id"]').map { |e| e.text }
+          expected_ordered_ids = [first, second, third, last].map { |d| "##{d.iid}" }
+          expect(ids).to eq(expected_ordered_ids)
+        end
+      end
+
       context 'with related deployable present' do
         let(:pipeline) { create(:ci_pipeline, project: project) }
         let(:build) { create(:ci_build, pipeline: pipeline) }
@@ -116,6 +140,10 @@ RSpec.describe 'Environment' do
           create(:deployment, :success, environment: environment, deployable: build)
         end
 
+        before do
+          visit_environment(environment)
+        end
+
         it 'does show build name' do
           expect(page).to have_link("#{build.name} (##{build.id})")
         end

spec/models/deployment_spec.rb

@@ -456,6 +456,17 @@ RSpec.describe Deployment do
       end
     end
 
+    describe '.ordered' do
+      let!(:deployment1) { create(:deployment, status: :running) }
+      let!(:deployment2) { create(:deployment, status: :success, finished_at: Time.current) }
+      let!(:deployment3) { create(:deployment, status: :canceled, finished_at: 1.day.ago) }
+      let!(:deployment4) { create(:deployment, status: :success, finished_at: 2.days.ago) }
+
+      it 'sorts by finished at' do
+        expect(described_class.ordered).to eq([deployment1, deployment2, deployment3, deployment4])
+      end
+    end
+
     describe 'visible' do
       subject { described_class.visible }