Get rid of ScheduleInstallationService

- deletes schedule_installation_service.rb
- moves schedule_installation_service.rb logic to create_service.rb
- moves specs as well

Removes code duplication

Remove unecessary spec block

Abide review suggestions

Test installable applications which are not associated to a cluster

Fix a typo

Removes duplciated expectation

Reuse variable instead of redefining

Remove method in favor of a local scoped lambda

Improve 'failing service' shared examples

Test the increase of status count

Remove duplicated test

Enable fronzen literal

2 files changed, 147 insertions, 152 deletions

diff --git a/spec/services/clusters/applications/create_service_spec.rb b/spec/services/clusters/applications/create_service_spec.rb
index 3f621ed5944..cbdef008b07 100644
--- a/spec/services/clusters/applications/create_service_spec.rb
+++ b/spec/services/clusters/applications/create_service_spec.rb
@@ -26,12 +26,6 @@ describe Clusters::Applications::CreateService do
       end.to change(cluster, :application_helm)
     end
 
-    it 'schedules an install via worker' do
-      expect(ClusterInstallAppWorker).to receive(:perform_async).with('helm', anything).once
-
-      subject
-    end
-
     context 'application already installed' do
       let!(:application) { create(:clusters_applications_helm, :installed, cluster: cluster) }
 
@@ -42,88 +36,101 @@ describe Clusters::Applications::CreateService do
       end
 
       it 'schedules an upgrade for the application' do
-        expect(Clusters::Applications::ScheduleInstallationService).to receive(:new).with(application).and_call_original
+        expect(ClusterUpgradeAppWorker).to receive(:perform_async)
 
         subject
       end
     end
 
-    context 'cert manager application' do
-      let(:params) do
-        {
-          application: 'cert_manager',
-          email: 'test@example.com'
-        }
-      end
-
+    context 'known applications' do
       before do
-        allow_any_instance_of(Clusters::Applications::ScheduleInstallationService).to receive(:execute)
+        create(:clusters_applications_helm, :installed, cluster: cluster)
       end
 
-      it 'creates the application' do
-        expect do
-          subject
+      context 'cert manager application' do
+        let(:params) do
+          {
+            application: 'cert_manager',
+            email: 'test@example.com'
+          }
+        end
 
-          cluster.reload
-        end.to change(cluster, :application_cert_manager)
-      end
+        before do
+          expect_any_instance_of(Clusters::Applications::CertManager)
+            .to receive(:make_scheduled!)
+            .and_call_original
+        end
 
-      it 'sets the email' do
-        expect(subject.email).to eq('test@example.com')
-      end
-    end
+        it 'creates the application' do
+          expect do
+            subject
 
-    context 'jupyter application' do
-      let(:params) do
-        {
-          application: 'jupyter',
-          hostname: 'example.com'
-        }
-      end
+            cluster.reload
+          end.to change(cluster, :application_cert_manager)
+        end
 
-      before do
-        allow_any_instance_of(Clusters::Applications::ScheduleInstallationService).to receive(:execute)
+        it 'sets the email' do
+          expect(subject.email).to eq('test@example.com')
+        end
       end
 
-      it 'creates the application' do
-        expect do
-          subject
+      context 'jupyter application' do
+        let(:params) do
+          {
+            application: 'jupyter',
+            hostname: 'example.com'
+          }
+        end
 
-          cluster.reload
-        end.to change(cluster, :application_jupyter)
-      end
+        before do
+          create(:clusters_applications_ingress, :installed, external_ip: "127.0.0.0", cluster: cluster)
+          expect_any_instance_of(Clusters::Applications::Jupyter)
+            .to receive(:make_scheduled!)
+            .and_call_original
+        end
 
-      it 'sets the hostname' do
-        expect(subject.hostname).to eq('example.com')
-      end
+        it 'creates the application' do
+          expect do
+            subject
 
-      it 'sets the oauth_application' do
-        expect(subject.oauth_application).to be_present
-      end
-    end
+            cluster.reload
+          end.to change(cluster, :application_jupyter)
+        end
 
-    context 'knative application' do
-      let(:params) do
-        {
-          application: 'knative',
-          hostname: 'example.com'
-        }
-      end
+        it 'sets the hostname' do
+          expect(subject.hostname).to eq('example.com')
+        end
 
-      before do
-        allow_any_instance_of(Clusters::Applications::ScheduleInstallationService).to receive(:execute)
+        it 'sets the oauth_application' do
+          expect(subject.oauth_application).to be_present
+        end
       end
 
-      it 'creates the application' do
-        expect do
-          subject
+      context 'knative application' do
+        let(:params) do
+          {
+            application: 'knative',
+            hostname: 'example.com'
+          }
+        end
 
-          cluster.reload
-        end.to change(cluster, :application_knative)
-      end
+        before do
+          expect_any_instance_of(Clusters::Applications::Knative)
+            .to receive(:make_scheduled!)
+            .and_call_original
+        end
 
-      it 'sets the hostname' do
-        expect(subject.hostname).to eq('example.com')
+        it 'creates the application' do
+          expect do
+            subject
+
+            cluster.reload
+          end.to change(cluster, :application_knative)
+        end
+
+        it 'sets the hostname' do
+          expect(subject.hostname).to eq('example.com')
+        end
       end
     end
 
@@ -140,19 +147,21 @@ describe Clusters::Applications::CreateService do
 
       using RSpec::Parameterized::TableSyntax
 
-      before do
-        allow_any_instance_of(Clusters::Applications::ScheduleInstallationService).to receive(:execute)
-      end
-
-      where(:application, :association, :allowed) do
-        'helm'       | :application_helm       | true
-        'ingress'    | :application_ingress    | true
-        'runner'     | :application_runner     | false
-        'jupyter'    | :application_jupyter    | false
-        'prometheus' | :application_prometheus | false
+      where(:application, :association, :allowed, :pre_create_helm) do
+        'helm'       | :application_helm       | true | false
+        'ingress'    | :application_ingress    | true | true
+        'runner'     | :application_runner     | false | true
+        'jupyter'    | :application_jupyter    | false | true
+        'prometheus' | :application_prometheus | false | true
       end
 
       with_them do
+        before do
+          klass = "Clusters::Applications::#{application.titleize}"
+          allow_any_instance_of(klass.constantize).to receive(:make_scheduled!).and_call_original
+          create(:clusters_applications_helm, :installed, cluster: cluster) if pre_create_helm
+        end
+
         let(:params) { { application: application } }
 
         it 'executes for each application' do
@@ -168,5 +177,68 @@ describe Clusters::Applications::CreateService do
         end
       end
     end
+
+    context 'when application is installable' do
+      shared_examples 'installable applications' do
+        it 'makes the application scheduled' do
+          expect do
+            subject
+          end.to change { Clusters::Applications::Helm.with_status(:scheduled).count }.by(1)
+        end
+
+        it 'schedules an install via worker' do
+          expect(ClusterInstallAppWorker)
+            .to receive(:perform_async)
+            .with(*worker_arguments)
+            .once
+
+          subject
+        end
+      end
+
+      context 'when application is associated with a cluster' do
+        let(:application) { create(:clusters_applications_helm, :installable, cluster: cluster) }
+        let(:worker_arguments) { [application.name, application.id] }
+
+        it_behaves_like 'installable applications'
+      end
+
+      context 'when application is not associated with a cluster' do
+        let(:worker_arguments) { [params[:application], kind_of(Numeric)] }
+
+        it_behaves_like 'installable applications'
+      end
+    end
+
+    context 'when installation is already in progress' do
+      let!(:application) { create(:clusters_applications_helm, :installing, cluster: cluster) }
+
+      it 'raises an exception' do
+        expect { subject }
+          .to raise_exception(StateMachines::InvalidTransition)
+          .and not_change(application.class.with_status(:scheduled), :count)
+      end
+
+      it 'does not schedule a cluster worker' do
+        expect(ClusterInstallAppWorker).not_to receive(:perform_async)
+      end
+    end
+
+    context 'when application is installed' do
+      %i(installed updated).each do |status|
+        let(:application) { create(:clusters_applications_helm, status, cluster: cluster) }
+
+        it 'schedules an upgrade via worker' do
+          expect(ClusterUpgradeAppWorker)
+            .to receive(:perform_async)
+            .with(application.name, application.id)
+            .once
+
+          subject
+
+          expect(application.reload).to be_scheduled
+        end
+      end
+    end
   end
 end
diff --git a/spec/services/clusters/applications/schedule_installation_service_spec.rb b/spec/services/clusters/applications/schedule_installation_service_spec.rb
deleted file mode 100644
index 8380932dfaa..00000000000
--- a/spec/services/clusters/applications/schedule_installation_service_spec.rb
+++ /dev/null
@@ -1,77 +0,0 @@
-require 'spec_helper'
-
-describe Clusters::Applications::ScheduleInstallationService do
-  def count_scheduled
-    application&.class&.with_status(:scheduled)&.count || 0
-  end
-
-  shared_examples 'a failing service' do
-    it 'raise an exception' do
-      expect(ClusterInstallAppWorker).not_to receive(:perform_async)
-      count_before = count_scheduled
-
-      expect { service.execute }.to raise_error(StandardError)
-      expect(count_scheduled).to eq(count_before)
-    end
-  end
-
-  describe '#execute' do
-    let(:service) { described_class.new(application) }
-
-    context 'when application is installable' do
-      let(:application) { create(:clusters_applications_helm, :installable) }
-
-      it 'make the application scheduled' do
-        expect(ClusterInstallAppWorker).to receive(:perform_async).with(application.name, kind_of(Numeric)).once
-
-        expect { service.execute }.to change { application.class.with_status(:scheduled).count }.by(1)
-      end
-    end
-
-    context 'when installation is already in progress' do
-      let(:application) { create(:clusters_applications_helm, :installing) }
-
-      it_behaves_like 'a failing service'
-    end
-
-    context 'when application is nil' do
-      let(:application) { nil }
-
-      it_behaves_like 'a failing service'
-    end
-
-    context 'when application cannot be persisted' do
-      let(:application) { create(:clusters_applications_helm) }
-
-      before do
-        expect(application).to receive(:make_scheduled!).once.and_raise(ActiveRecord::RecordInvalid)
-      end
-
-      it_behaves_like 'a failing service'
-    end
-
-    context 'when application is installed' do
-      let(:application) { create(:clusters_applications_helm, :installed) }
-
-      it 'schedules an upgrade via worker' do
-        expect(ClusterUpgradeAppWorker).to receive(:perform_async).with(application.name, application.id).once
-
-        service.execute
-
-        expect(application).to be_scheduled
-      end
-    end
-
-    context 'when application is updated' do
-      let(:application) { create(:clusters_applications_helm, :updated) }
-
-      it 'schedules an upgrade via worker' do
-        expect(ClusterUpgradeAppWorker).to receive(:perform_async).with(application.name, application.id).once
-
-        service.execute
-
-        expect(application).to be_scheduled
-      end
-    end
-  end
-end