Make Consumer Apps dictate aasa results (#14015)

* Makes Consumer Apps dictate aasa results

* progress with ConsumerApp query

* Adds Team ID migration + Stimulus consumer_app_controller.js

* Adds cypress tests

* Adds Backfill data_update_script + more specs & tweaks

* Remove file added by mistake

* Comment typo

* Small tweaks + improved specs

* Update lib/data_update_scripts/20210622145212_backfill_forem_consumer_app_team_id.rb

Co-authored-by: Jamie Gaskins <jamie@forem.com>

* Update spec/lib/data_update_scripts/backfill_forem_consumer_app_team_id_spec.rb

Co-authored-by: rhymes <github@rhymes.dev>

* Make use of create! and log errors to ForemStatsClient

* Fix specs

* Add mock_rpush call as suggested in review

* Add Review suggestions

* Fix tests

* Remove redundant assert in spec

Co-authored-by: Jamie Gaskins <jamie@forem.com>
Co-authored-by: rhymes <github@rhymes.dev>
This commit is contained in:
Fernando Valverde 2021-06-24 08:36:11 -06:00 committed by GitHub
parent f9621150d7
commit f34b2203d3
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
17 changed files with 161 additions and 20 deletions

View file

@ -4,10 +4,18 @@ class DeepLinksController < ApplicationController
# Apple Application Site Association - based on Apple docs guidelines
# https://developer.apple.com/library/archive/documentation/General/Conceptual/AppSearch/UniversalLinks.html
def aasa
# TODO: [@fdoxyz] Replace these hardcoded identifiers with configurations
# creators can use to customize their Forems - `/admin/consumer_apps`
supported_apps = ["R9SWHSQNV8.com.forem.app"]
supported_apps << "R9SWHSQNV8.to.dev.ios" if ForemInstance.dev_to?
# This query plucks :team_id & :app_bundle so we get an array or arrays
# Example: [['TEAM1', 'app.bundle.one'], ['TEAM2', 'app.bundle.two']]
consumer_apps = ConsumerApps::FindOrCreateAllQuery.call
.where(platform: Device::IOS)
.where.not(team_id: nil)
.order(:created_at)
.pluck(:team_id, :app_bundle)
# Now restructure the array of arrays into valid AASA App ID's
# Example: ['TEAM1.app.bundle.one', 'TEAM2.app.bundle.two']
supported_apps = consumer_apps.map { |result| result.join(".") }
render json: {
applinks: {
apps: [],

View file

@ -0,0 +1,17 @@
import { Controller } from 'stimulus';
export default class ConsumerAppController extends Controller {
static targets = ['platform', 'teamId'];
connect() {
this.checkPlatform();
}
checkPlatform() {
if (this.platformTarget.value === 'ios') {
this.teamIdTarget.classList.remove('hidden');
} else {
this.teamIdTarget.classList.add('hidden');
}
}
}

View file

@ -3,6 +3,7 @@ class ConsumerApp < ApplicationRecord
FOREM_BUNDLE = "com.forem.app".freeze
FOREM_APP_PLATFORMS = %w[ios].freeze
FOREM_TEAM_ID = "R9SWHSQNV8".freeze
enum platform: { android: Device::ANDROID, ios: Device::IOS }

View file

@ -5,7 +5,16 @@ module ConsumerApps
existing_platforms = ConsumerApp.where(app_bundle: forem_bundle).pluck(:platform)
(ConsumerApp::FOREM_APP_PLATFORMS - existing_platforms).each do |platform|
# Re-create the supported Forem apps if they're missing
ConsumerApp.create(app_bundle: forem_bundle, platform: platform, active: true)
ConsumerApp.create!(app_bundle: forem_bundle,
platform: platform,
team_id: ConsumerApp::FOREM_TEAM_ID)
rescue StandardError => e
error_tags = [
"error:#{e.message}",
"app_bundle:#{forem_bundle}",
"platform:#{platform}",
]
ForemStatsClient.increment("consumer_apps.create", tags: error_tags)
end
ConsumerApp.limit(50)

View file

@ -28,7 +28,9 @@ module ConsumerApps
end
def forem_consumer_app
relation.create_or_find_by(app_bundle: ConsumerApp::FOREM_BUNDLE)
# iOS Forem Consumer App requires a team_id value but Android (future) doesn't
team_id = platform == :ios ? ConsumerApp::FOREM_TEAM_ID : nil
relation.create_or_find_by(app_bundle: ConsumerApp::FOREM_BUNDLE, team_id: team_id)
end
end
end

View file

@ -5,10 +5,15 @@
<div class="form-group">
<%= label_tag :platform, "Platform:" %>
<%= select_tag :platform, options_for_select(ConsumerApp.platforms.invert, selected: @app.platform, class: "crayons-select"), class: "crayons-select" %>
<%= select_tag :platform, options_for_select(ConsumerApp.platforms.invert, selected: @app.platform, class: "crayons-select"), class: "crayons-select", data: { "consumer-app-target" => "platform", "action" => "consumer-app#checkPlatform" } %>
</div>
<div class="form-group">
<%= label_tag :auth_key, "Authentication Key:" %>
<%= text_area_tag :auth_key, @app.auth_key, size: "100x10", class: "form-control" %>
</div>
<div class="form-group hidden" data-consumer-app-target="teamId">
<%= label_tag :team_id, "Team ID (Universal Links Support):" %>
<%= text_field_tag :team_id, @app.team_id, class: "form-control" %>
</div>

View file

@ -1,6 +1,6 @@
<h2 class="crayons-title mb-6">Edit Consumer App</h2>
<div class="crayons-card p-6">
<%= form_for([:admin, @app], method: :patch) do %>
<%= form_for([:admin, @app], method: :patch, data: { controller: "consumer-app" }) do %>
<%= render "form" %>
<%= submit_tag "Update Consumer App", class: "crayons-btn" %>
<% end %>

View file

@ -1,6 +1,6 @@
<h2 class="crayons-title mb-6">New Consumer App</h2>
<div class="crayons-card p-6">
<%= form_for([:admin, @app], method: :post) do %>
<%= form_for([:admin, @app], method: :post, data: { controller: "consumer-app" }) do %>
<%= render "form" %>
<%= submit_tag "Create Consumer App", class: "crayons-btn" %>
<% end %>

View file

@ -0,0 +1,46 @@
describe('Consumer Apps', () => {
beforeEach(() => {
cy.testSetup();
cy.fixture('users/adminUser.json').as('user');
cy.get('@user').then((user) => {
cy.loginUser(user);
cy.visit('/admin/apps/consumer_apps');
});
});
it('creates a new iOS Consumer App', () => {
const test_app_bundle = 'com.app.bundle';
cy.get('.crayons-btn').contains('New Consumer App').click();
cy.get('#new_consumer_app').as('consumerAppForm');
cy.get('@consumerAppForm').find('#app_bundle').type(test_app_bundle);
cy.get('@consumerAppForm').find('#platform').select('iOS');
// iOS apps need to provide the option to add a Team ID value
cy.get('@consumerAppForm').find('#team_id').should('be.visible');
cy.get('@consumerAppForm').find('#team_id').type('ABC123');
cy.get('@consumerAppForm')
.get('.crayons-btn')
.contains('Create Consumer App')
.click();
cy.findByText(`${test_app_bundle} has been created!`).should('be.visible');
});
it('creates a new Android Consumer App', () => {
const test_app_bundle = 'com.app.bundle';
cy.get('.crayons-btn').contains('New Consumer App').click();
cy.get('#new_consumer_app').as('consumerAppForm');
cy.get('@consumerAppForm').find('#app_bundle').type(test_app_bundle);
cy.get('@consumerAppForm').find('#platform').select('Android');
// Android apps don't need a Team ID value
cy.get('@consumerAppForm').find('#team_id').should('not.be.visible');
cy.get('@consumerAppForm')
.get('.crayons-btn')
.contains('Create Consumer App')
.click();
cy.findByText(`${test_app_bundle} has been created!`).should('be.visible');
});
});

View file

@ -0,0 +1,5 @@
class AddTeamIdToConsumerApps < ActiveRecord::Migration[6.1]
def change
add_column :consumer_apps, :team_id, :string
end
end

View file

@ -10,7 +10,7 @@
#
# It's strongly recommended that you check this file into your version control system.
ActiveRecord::Schema.define(version: 2021_06_09_164958) do
ActiveRecord::Schema.define(version: 2021_06_22_002941) do
# These are extensions that must be enabled in order to support this database
enable_extension "citext"
@ -409,6 +409,7 @@ ActiveRecord::Schema.define(version: 2021_06_09_164958) do
t.datetime "created_at", precision: 6, null: false
t.string "last_error"
t.string "platform", null: false
t.string "team_id"
t.datetime "updated_at", precision: 6, null: false
t.index ["app_bundle", "platform"], name: "index_consumer_apps_on_app_bundle_and_platform", unique: true
end

View file

@ -0,0 +1,9 @@
module DataUpdateScripts
class BackfillForemConsumerAppTeamId
def run
ConsumerApp
.where(app_bundle: ConsumerApp::FOREM_BUNDLE, platform: :ios)
.update(team_id: ConsumerApp::FOREM_TEAM_ID)
end
end
end

View file

@ -0,0 +1,34 @@
require "rails_helper"
require Rails.root.join(
"lib/data_update_scripts/20210622145212_backfill_forem_consumer_app_team_id.rb",
)
describe DataUpdateScripts::BackfillForemConsumerAppTeamId do
def forem_ios_consumer_app
ConsumerApp.find_or_create_by(app_bundle: ConsumerApp::FOREM_BUNDLE, platform: :ios)
end
before do
consumer_app = forem_ios_consumer_app
consumer_app.update_columns(team_id: nil)
mock_rpush(consumer_app)
end
it "adds the team_id to a the Forem iOS Consumer App" do
# The team_id is expected be nil at the start of the test
consumer_app = forem_ios_consumer_app
expect(consumer_app.team_id).to be_nil
expect { described_class.new.run }
.to change { consumer_app.reload.team_id }
.from(nil).to(ConsumerApp::FOREM_TEAM_ID)
end
it "doesn't affect other Consumer App Team IDs", :aggregate_failures do
custom_team_id = "ABC123"
custom_consumer_app = create(:consumer_app, team_id: custom_team_id)
expect { described_class.new.run }
.not_to change { custom_consumer_app.reload.team_id }
end
end

View file

@ -6,10 +6,13 @@ RSpec.describe ConsumerApps::FindOrCreateAllQuery, type: :query do
it "fetches all ConsumerApp including the Forem apps" do
all_consumer_apps = described_class.call
result_bundles = all_consumer_apps.map(&:app_bundle).uniq
result_team_ids = all_consumer_apps.map(&:team_id).uniq
# Must return consumer_app + every ConsumerApp::FOREM_APP_PLATFORMS
expect(all_consumer_apps.count).to eq(1 + ConsumerApp::FOREM_APP_PLATFORMS.count)
# Must include the consumer_app and Forem App bundles
expect(result_bundles).to include(consumer_app.app_bundle, ConsumerApp::FOREM_BUNDLE)
# Must include the Forem Team ID
expect(result_team_ids).to include(ConsumerApp::FOREM_TEAM_ID)
end
end

View file

@ -11,6 +11,7 @@ RSpec.describe ConsumerApps::FindOrCreateByQuery, type: :query do
platform: :ios,
)
expect(app).to be_instance_of(ConsumerApp)
expect(app.team_id).to eq(ConsumerApp::FOREM_TEAM_ID)
end.to change(ConsumerApp, :count).by(1)
end
end

View file

@ -10,10 +10,7 @@ RSpec.describe ConsumerApps::RpushAppQuery, type: :query do
mock_rpush(consumer_app)
# Fetch rpush app associated to the target
rpush_app = described_class.call(
app_bundle: consumer_app.app_bundle,
platform: consumer_app.platform,
)
rpush_app = described_class.call(app_bundle: 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)

View file

@ -3,16 +3,19 @@ require "rails_helper"
RSpec.describe "Universal Links (Apple)", type: :request do
let(:aasa_route) { "/.well-known/apple-app-site-association" }
let(:forem_app_id) { "R9SWHSQNV8.com.forem.app" }
let(:dev_app_id) { "R9SWHSQNV8.to.dev.ios" }
describe "returns a valid Apple App Site Association file" do
context "with DEV app backwards compatibility" do
it "responds with applinks support for both" do
allow(ForemInstance).to receive(:dev_to?).and_return(true)
context "with multiple ConsumerApps" do
it "responds with applinks support for iOS apps only" do
# This iOS ConsumerApp should appear in the results
ios_app = create(:consumer_app, platform: Device::IOS)
# This Android ConsumerApp shouldn't appear in the results
create(:consumer_app, platform: Device::ANDROID)
get aasa_route
json_response = JSON.parse(response.body)
both_app_ids = [forem_app_id, dev_app_id]
both_app_ids = [forem_app_id, ios_app.app_bundle]
expect(response).to have_http_status(:ok)
expect(json_response.dig("applinks", "apps")).to be_empty
json_response.dig("applinks", "details").each do |hash|
@ -22,7 +25,7 @@ RSpec.describe "Universal Links (Apple)", type: :request do
end
end
context "when non-DEV Forem instance" do
context "without any custom ConsumerApps" do
it "responds with applinks support for Forem app" do
get aasa_route
json_response = JSON.parse(response.body)