Merge branch '32790-pipeline_schedules-pages-throwing-error-500' into 'master'

Resolve "pipeline_schedules pages throwing error 500"

Closes #32790

See merge request !11706

5 files changed, 41 insertions, 8 deletions

diff --git a/app/models/ci/pipeline_schedule.rb b/app/models/ci/pipeline_schedule.rb
index cf6e53c4ca4..07213ca608a 100644
--- a/app/models/ci/pipeline_schedule.rb
+++ b/app/models/ci/pipeline_schedule.rb
@@ -10,9 +10,9 @@ module Ci
     has_one :last_pipeline, -> { order(id: :desc) }, class_name: 'Ci::Pipeline'
     has_many :pipelines
 
-    validates :cron, unless: :importing_or_inactive?, cron: true, presence: { unless: :importing_or_inactive? }
-    validates :cron_timezone, cron_timezone: true, presence: { unless: :importing_or_inactive? }
-    validates :ref, presence: { unless: :importing_or_inactive? }
+    validates :cron, unless: :importing?, cron: true, presence: { unless: :importing? }
+    validates :cron_timezone, cron_timezone: true, presence: { unless: :importing? }
+    validates :ref, presence: { unless: :importing? }
     validates :description, presence: true
 
     before_save :set_next_run_at
@@ -32,10 +32,6 @@ module Ci
       update_attribute(:active, false)
     end
 
-    def importing_or_inactive?
-      importing? || inactive?
-    end
-
     def runnable_by_owner?
       Ability.allowed?(owner, :create_pipeline, project)
     end
diff --git a/app/views/projects/pipeline_schedules/_pipeline_schedule.html.haml b/app/views/projects/pipeline_schedules/_pipeline_schedule.html.haml
index 082a6bcbb2a..7bde839e26f 100644
--- a/app/views/projects/pipeline_schedules/_pipeline_schedule.html.haml
+++ b/app/views/projects/pipeline_schedules/_pipeline_schedule.html.haml
@@ -4,7 +4,8 @@
       = pipeline_schedule.description
     %td.branch-name-cell
       = icon('code-fork')
-      = link_to pipeline_schedule.ref, project_ref_path(@project, pipeline_schedule.ref), class: "ref-name"
+      - if pipeline_schedule.ref
+        = link_to pipeline_schedule.ref, project_ref_path(@project, pipeline_schedule.ref), class: "ref-name"
     %td
       - if pipeline_schedule.last_pipeline
         .status-icon-container{ class: "ci-status-icon-#{pipeline_schedule.last_pipeline.status}" }
diff --git a/changelogs/unreleased/32790-pipeline_schedules-pages-throwing-error-500.yml b/changelogs/unreleased/32790-pipeline_schedules-pages-throwing-error-500.yml
new file mode 100644
index 00000000000..a58f3a7429e
--- /dev/null
+++ b/changelogs/unreleased/32790-pipeline_schedules-pages-throwing-error-500.yml
@@ -0,0 +1,4 @@
+---
+title: Fix pipeline_schedules pages throwing error 500
+merge_request: 11706
+author: dosuken123
diff --git a/spec/features/projects/pipeline_schedules_spec.rb b/spec/features/projects/pipeline_schedules_spec.rb
index f40e1bc4930..317949d6b56 100644
--- a/spec/features/projects/pipeline_schedules_spec.rb
+++ b/spec/features/projects/pipeline_schedules_spec.rb
@@ -65,6 +65,17 @@ feature 'Pipeline Schedules', :feature do
         expect(page).not_to have_content('pipeline schedule')
       end
     end
+
+    context 'when ref is nil' do
+      before do
+        pipeline_schedule.update_attribute(:ref, nil)
+        visit_pipelines_schedules
+      end
+
+      it 'shows a list of the pipeline schedules with empty ref column' do
+        expect(first('.branch-name-cell').text).to eq('')
+      end
+    end
   end
 
   describe 'POST /projects/pipeline_schedules/new', js: true do
@@ -108,6 +119,19 @@ feature 'Pipeline Schedules', :feature do
 
       expect(page).to have_content('my brand new description')
     end
+
+    context 'when ref is nil' do
+      before do
+        pipeline_schedule.update_attribute(:ref, nil)
+        edit_pipeline_schedule
+      end
+
+      it 'shows the pipeline schedule with default ref' do
+        page.within('.git-revision-dropdown-toggle') do
+          expect(first('.dropdown-toggle-text').text).to eq('master')
+        end
+      end
+    end
   end
 
   def visit_new_pipeline_schedule
diff --git a/spec/models/ci/pipeline_schedule_spec.rb b/spec/models/ci/pipeline_schedule_spec.rb
index 822b98c5f6c..b00e7a73571 100644
--- a/spec/models/ci/pipeline_schedule_spec.rb
+++ b/spec/models/ci/pipeline_schedule_spec.rb
@@ -25,6 +25,14 @@ describe Ci::PipelineSchedule, models: true do
 
       expect(pipeline_schedule).not_to be_valid
     end
+
+    context 'when active is false' do
+      it 'does not allow nullified ref' do
+        pipeline_schedule = build(:ci_pipeline_schedule, :inactive, ref: nil)
+
+        expect(pipeline_schedule).not_to be_valid
+      end
+    end
   end
 
   describe '#set_next_run_at' do