Fix upgrade endpoint

* don't check activations on upgrade
* delete old activations that are successors/predecessors
* activate new product

app/controllers/api/connect/v3/systems/products_controller.rb

@@ -48,18 +48,15 @@ def offline_migrations
   end
 
   def upgrade
-    activation = @system.activations.joins(:service).find_by('services.product_id': [@product.id, @product.predecessor_ids, @product.successor_ids].flatten)
+    obsoleted_product_ids = ([@product] + @product.predecessors + @product.successors).map(&:id)
+    obsoleted_service = @system.services.find_by(product_id: obsoleted_product_ids)
+    @obsolted_service_name = obsoleted_service.name if obsoleted_service
 
-    unless activation
-      raise ActionController::TranslatedError.new(
-        N_("No activation with product '%s' was found."),
-        @product.friendly_name
-      )
+    ActiveRecord::Base.transaction do
+      remove_previous_product_activation(obsoleted_product_ids)
+      create_product_activation
     end
 
-    activation.service = @product.service
-    activation.save!
-
     render_service
   end
 
@@ -105,6 +102,10 @@ def create_product_activation
     @system.activations.where(service_id: @product.service.id).first_or_create
   end
 
+  def remove_previous_product_activation(product_ids)
+    @system.activations.includes(:product).where('products.id' => product_ids).destroy_all
+  end
+
   # Check if extension base product is already activated
   def check_base_product_dependencies
     # TODO: For APIv5 and future. We skip this check for second level extensions. E.g. HA-GEO

spec/requests/api/connect/v3/systems/products_controller_spec.rb

@@ -169,72 +169,56 @@
   end
 
   describe '#upgrade' do
+    let(:request) { put url, headers: headers, params: payload }
+    let(:new_product) { FactoryGirl.create(:product, :with_mirrored_repositories) }
+    let(:payload) do
+      {
+        identifier: new_product.identifier,
+        version: new_product.version,
+        arch: new_product.arch
+      }
+    end
+    let(:serialized_json) do
+      V3::ServiceSerializer.new(
+        new_product.service,
+        base_url: URI::HTTP.build({ scheme: response.request.scheme, host: response.request.host }).to_s
+      ).to_json
+    end
+
     it_behaves_like 'products controller action' do
       let(:verb) { 'put' }
     end
 
-    context 'with not activated product' do
-      before { put url, headers: headers, params: payload }
-      subject { response }
+    before do
+      request
+      system.reload
+    end
 
-      let(:product) { FactoryGirl.create(:product, :with_mirrored_repositories) }
-      let(:payload) do
-        {
-          identifier: product.identifier,
-          version: product.version,
-          arch: product.arch
-        }
-      end
+    context 'new product' do
+      subject { response }
 
-      let(:error_response) do
-        {
-          type: 'error',
-          error: "No activation with product '#{product.friendly_name}' was found.",
-          localized_error: "No activation with product '#{product.friendly_name}' was found."
-        }
-      end
+      specify { expect(system.activations.count).to eq(1) }
 
+      its(:code) { is_expected.to eq('201') }
+      its(:body) { is_expected.to eq(serialized_json) }
 
-      its(:code) { is_expected.to eq('422') }
-      its(:body) { is_expected.to eq(error_response.to_json) }
+      it 'activates new product' do
+        expect(system.activations.first.reload.service_id).to equal(new_product.service.id)
+      end
     end
 
-    context 'with activated product' do
-      let(:request) { put url, headers: headers, params: payload }
-
+    context 'with activated previous product' do
       let!(:old_product) { FactoryGirl.create(:product, :with_mirrored_repositories, :activated, system: system) }
       let(:new_product) { FactoryGirl.create(:product, :with_mirrored_repositories, predecessors: [old_product]) }
 
-      let(:payload) do
-        {
-          identifier: new_product.identifier,
-          version: new_product.version,
-          arch: new_product.arch
-        }
-      end
-
-      let(:serialized_json) do
-        V3::ServiceSerializer.new(
-          new_product.service,
-          base_url: URI::HTTP.build({ scheme: response.request.scheme, host: response.request.host }).to_s
-        ).to_json
-      end
-
-      describe 'response' do
-        before { request }
-        subject { response }
+      subject { response }
+      specify { expect(system.activations.count).to eq(1) }
 
-        its(:code) { is_expected.to eq('201') }
-        its(:body) { is_expected.to eq(serialized_json) }
-      end
+      its(:code) { is_expected.to eq('201') }
+      its(:body) { is_expected.to eq(serialized_json) }
 
-      describe 'activations' do
-        specify { expect { request }.not_to change { system.activations.count } }
-        it "updates the system's activation with the new product" do
-          expect { request }.to change { system.activations.first.reload.service_id }
-            .from(old_product.service.id)
-            .to(new_product.service.id)
-        end
+      it 'deactivates old product and activates new product' do
+        expect(system.activations.first.reload.service_id).to equal(new_product.service.id)
       end
     end
   end