Fixes a name conflict in Rpush models (#15978)

* Fixes a name conflict in Rpush models

* Removes unnecessary operator

* Review feedback

app/models/consumer_app.rb

@@ -49,11 +49,12 @@ class ConsumerApp < ApplicationRecord
   # [@forem/backend] `.where().first` is necessary because we use Redis data storage
   # https://github.com/rpush/rpush/wiki/Using-Redis#find_by_name-cannot-be-used-in-rpush-redis
   def clear_rpush_app
+    app_name = "#{app_bundle_was}.#{platform}"
     case ConsumerApp.platforms[platform_was]
     when Device::IOS
-      Rpush::Apns2::App.where(name: app_bundle_was).first&.destroy
+      Rpush::Apns2::App.where(name: app_name).first&.destroy
     when Device::ANDROID
-      Rpush::Gcm::App.where(name: app_bundle_was).first&.destroy
+      Rpush::Gcm::App.where(name: app_name).first&.destroy
     end
 
     # This prevents the `destroy` method to return true or false in a callback

app/queries/consumer_apps/rpush_app_query.rb

@@ -7,6 +7,7 @@ module ConsumerApps
     def initialize(app_bundle:, platform:)
       @app_bundle = app_bundle
       @platform = platform
+      @app_name = "#{app_bundle}.#{platform}"
 
       @consumer_app = ConsumerApps::FindOrCreateByQuery.call(
         app_bundle: @app_bundle,
@@ -16,24 +17,24 @@ module ConsumerApps
 
     def call
       if consumer_app.android?
-        android_app = Rpush::Gcm::App.where(name: app_bundle).first
+        android_app = Rpush::Gcm::App.where(name: app_name).first
         android_app || recreate_android_app!
       elsif consumer_app.ios?
-        ios_app = Rpush::Apns2::App.where(name: app_bundle).first
+        ios_app = Rpush::Apns2::App.where(name: app_name).first
         ios_app || recreate_ios_app!
       end
     end
 
     private
 
-    attr_reader :app_bundle, :consumer_app
+    attr_reader :app_bundle, :app_name, :consumer_app, :platform
 
     def recreate_ios_app!
       # If the ConsumerApp doesn't have credentials there's no need to create it
       return if consumer_app.auth_credentials.blank?
 
       app = Rpush::Apns2::App.new
-      app.name = app_bundle
+      app.name = app_name
       app.certificate = consumer_app.auth_credentials.to_s.gsub("\\n", "\n")
       app.environment = Rails.env
       app.password = ""
@@ -48,7 +49,7 @@ module ConsumerApps
       return if consumer_app.auth_credentials.blank?
 
       app = Rpush::Gcm::App.new
-      app.name = app_bundle
+      app.name = app_name
       app.auth_key = consumer_app.auth_credentials.to_s
       app.connections = 1
       app.save!

spec/queries/consumer_apps/rpush_app_query_spec.rb

@@ -1,19 +1,22 @@
 require "rails_helper"
 
 RSpec.describe ConsumerApps::RpushAppQuery, type: :query do
-  let(:consumer_app) do
+  let(:ios_consumer_app) do
     ConsumerApps::FindOrCreateByQuery.call(app_bundle: ConsumerApp::FOREM_BUNDLE, platform: :ios)
   end
+  let(:android_consumer_app) do
+    ConsumerApps::FindOrCreateByQuery.call(app_bundle: ConsumerApp::FOREM_BUNDLE, platform: :android)
+  end
 
   describe "Rpush app" do
     it "is recreated after updating a ConsumerApp" do
-      mock_rpush(consumer_app)
+      mock_rpush(ios_consumer_app)
 
       # Fetch rpush app associated to the target
-      rpush_app = described_class.call(app_bundle: consumer_app.app_bundle, platform: :ios)
+      rpush_app = described_class.call(app_bundle: ios_consumer_app.app_bundle, platform: :ios)
 
       expect(rpush_app).to be_instance_of(Rpush::Apns2::App)
-      expect(rpush_app.name).to eq(consumer_app.app_bundle)
+      expect(rpush_app.name).to eq(ios_consumer_app.app_bundle)
     end
 
     it "returns nil if ConsumerApp is not operational" do
@@ -26,5 +29,23 @@ RSpec.describe ConsumerApps::RpushAppQuery, type: :query do
       expect(bad_consumer_app.operational?).to be false
       expect(rpush_app).to be_nil
     end
+
+    it "works when ConsumerApps have the same bundle but different platform" do
+      # This is a regression test to avoid conflicting `name` in either
+      # Rpush::Apns2::App or Rpush::Gcm::App to break the app
+      allow(ApplicationConfig).to receive(:[]).with("RPUSH_IOS_PEM").and_return("asdf123")
+      allow(ApplicationConfig).to receive(:[]).with("RPUSH_FCM_KEY").and_return("asdf123")
+
+      mock_rpush(ios_consumer_app)
+      mock_rpush(android_consumer_app)
+
+      # Fetch rpush app associated to the target
+      ios_rpush_app = described_class.call(app_bundle: ios_consumer_app.app_bundle, platform: :ios)
+      expect(ios_rpush_app).to be_instance_of(Rpush::Apns2::App)
+
+      # Fetch rpush app associated to the target
+      android_rpush_app = described_class.call(app_bundle: android_consumer_app.app_bundle, platform: :android)
+      expect(android_rpush_app).to be_instance_of(Rpush::Gcm::App)
+    end
   end
 end