PaywallsTester: allow for configuration for demos #3260

aboedo · 2023-09-28T21:27:31Z

This introduces buttons that allow you to reconfigure the app with a different API key.

This will be useful when doing demos for customers, since the Testing app's Offerings are pretty cluttered at this point.

tonidero · 2023-09-29T06:30:59Z

Tests/TestingApps/PaywallsTester/ci_scripts/ci_pre_xcodebuild.sh

-    echo "PaywallsTester API key environment variable is not set."
+if [ -z "$PAYWALLS_TESTER_API_KEY_FOR_TESTING" ]; then
+    echo "PaywallsTester API key for testing environment variable is not set."
+elif


Hmm this elif here seems wrong. Are we trying to set both keys at the same time? If so, I think we need a different condition to check both?

lol bad copy/paste 😂 fixing

aboedo · 2023-09-28T21:28:09Z

Tests/TestingApps/PaywallsTester/PaywallsTester/SimpleApp.swift

-                ? Purchases.shared.customerInfoStream
-                : nil
-            )
+            AppContentView()


updated so that it uses the Purchases.isConfigured method and gets the customer info stream from purchases whenever needed

aboedo · 2023-09-28T21:29:24Z

Tests/TestingApps/PaywallsTester/ci_scripts/ci_pre_xcodebuild.sh

@@ -1,14 +1,19 @@
 #!/bin/bash -e

-PAYWALLS_TESTER_API_KEY=$REVENUECAT_XCODE_CLOUD_SIMPLE_APP_API_KEY


added another API key.
Truthfully we should be leveraging our fastlane stuff here, but I'm just going for the fastest possible thing.
This is using Xcode cloud, so we'd need to set that up appropriately as well if we wanted to go the fastlane route

aboedo · 2023-09-29T13:20:15Z

Tests/TestingApps/PaywallsTester/ci_scripts/ci_pre_xcodebuild.sh

-    echo "PaywallsTester API key environment variable is not set."
+if [ -z "$PAYWALLS_TESTER_API_KEY_FOR_TESTING" ]; then
+    echo "PaywallsTester API key for testing environment variable is not set."
+elif


lol bad copy/paste 😂 fixing

codecov · 2023-09-29T18:21:49Z

Codecov Report

All modified lines are covered by tests ✅

Comparison is base (7e94c28) 85.86% compared to head (55cda38) 85.86%.
Report is 1 commits behind head on main.

❗ Current head 55cda38 differs from pull request most recent head b1fbb92. Consider uploading reports for the commit b1fbb92 to get more accurate results

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #3260   +/-   ##
=======================================
  Coverage   85.86%   85.86%           
=======================================
  Files         236      236           
  Lines       16790    16790           
=======================================
  Hits        14416    14416           
  Misses       2374     2374

see 2 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

NachoSoto

Thanks for doing this!

Very simple approach, the main issue is that this kinda broke the app for developers that simply want to launch this app to see example paywalls.

Before this PR the app was working as a simple paywall list, now it's showing this which won't make sense to regular users:

And the "All paywalls" tab shows an error because of the API key.

I think moving the Purchases.shared.isConfigured check to AppContentView, so I'd just make sure we don't configure Purchases if the API key isn't set.

Tests/TestingApps/PaywallsTester/PaywallsTester/Configuration.swift

NachoSoto · 2023-10-02T16:16:06Z

Tests/TestingApps/PaywallsTester/PaywallsTester/Configuration.swift


-extension Configuration {
+    enum Mode {


Reminder that this app is now in TestingApps for customers, and this might be confusing for anyone looking to how this test app works.
Maybe add a brief docstring?

Tests/TestingApps/PaywallsTester/PaywallsTester/Views/AppContentView.swift

NachoSoto · 2023-10-02T16:22:09Z

Tests/TestingApps/PaywallsTester/PaywallsTester/Views/AppContentView.swift

+        switch self.configuration.currentMode {
+        case .custom:
+            return "the API set locally in Configuration.swift"
+        case .testing:
+            return "the Paywalls Tester app in RevenueCat Dashboard"
+        case .demos:
+            return "Demos"
+        }


Nit: I'd make this an extension on Configuration.Mode and just use self.configuration.currentMode.description instead of creating a separate function

I had it like that initially, but it felt pretty awkward because it's not really a generic description of the object that I have here, it's the copy for a label that includes more stuff

Tests/TestingApps/PaywallsTester/PaywallsTester/Views/AppContentView.swift

NachoSoto · 2023-10-02T22:19:14Z

Tests/TestingApps/PaywallsTester/PaywallsTester/Configuration.swift

+            Self.apiKeyFromCIForTesting
+        case .demos:
+            Self.apiKeyFromCIForDemos
+        case .listOnly:


NachoSoto · 2023-10-02T22:19:32Z

Tests/TestingApps/PaywallsTester/PaywallsTester/Configuration.swift


-}
+    private init() {
+        self.currentMode = Self.apiKey.isEmpty ? .testing : .custom


Should this be

Suggested change

self.currentMode = Self.apiKey.isEmpty ? .testing : .custom

self.currentMode = Self.apiKey.isEmpty ? .listOnly : .custom

good catch, this is wrong. It's slightly more complex in that there's 3 initial states, list only, testing and custom. It didn't jump out at me because in list only mode the configuration tab doesn't even show, but I'll fix it in any case

Tests/TestingApps/PaywallsTester/PaywallsTester/Views/AppContentView.swift

…ng on the current mode

Tests/TestingApps/PaywallsTester/PaywallsTester/Configuration.swift

@NachoSoto

**This is an automatic release.** ### RevenueCatUI * `Paywalls`: added shimmer effect to `LoadingPaywallView` (#3267) via NachoSoto (@NachoSoto) ### Bugfixes * `Paywalls`: fixed `macOS` compilation (#3272) via NachoSoto (@NachoSoto) ### Other Changes * Update `SwiftLint` (#3273) via NachoSoto (@NachoSoto) * PaywallsTester: allow for configuration for demos (#3260) via Andy Boedo (@aboedo) * `Paywalls`: simplified `LoadingPaywallView` (#3265) via NachoSoto (@NachoSoto)

aboedo requested a review from a team September 28, 2023 21:27

aboedo self-assigned this Sep 28, 2023

aboedo added the test label Sep 28, 2023

tonidero approved these changes Sep 29, 2023

View reviewed changes

aboedo commented Sep 29, 2023

View reviewed changes

NachoSoto suggested changes Oct 2, 2023

View reviewed changes

aboedo force-pushed the andy/pwl-298-paywallstester-allow-for-configuration-for-gtm branch from 802ff60 to 9a8fb57 Compare October 2, 2023 20:59

aboedo requested a review from NachoSoto October 2, 2023 21:24

NachoSoto approved these changes Oct 2, 2023

View reviewed changes

aboedo added 18 commits October 3, 2023 17:22

added ugly initial version that works.

0d3f577

update env variable names

8551647

fixes from previous commit

c1a905b

slight cleanup

d188caa

clean up customer info stream lifecycle

60178f6

fix sh script

51490ba

refactor and cleanup from previous PR

8cf27f9

lots of refactors, but the buttons now get enabled / disabled dependi…

45bddac

…ng on the current mode

small style update

04e5623

fix bug with default config

1005525

small improvements from PR comments

028b379

more refactors from PR comments

82777b5

more updates from PR comments

3a1ef34

and more

5b261a7

remove extra line break

d7701ce

fixed use case when the app is configured without an API key

6a6f624

cleaned up configuration flow

8d535be

fixed current mode logic

55cda38

aboedo force-pushed the andy/pwl-298-paywallstester-allow-for-configuration-for-gtm branch from 09ef581 to 55cda38 Compare October 3, 2023 21:03

NachoSoto approved these changes Oct 3, 2023

View reviewed changes

Tests/TestingApps/PaywallsTester/PaywallsTester/Configuration.swift Outdated Show resolved Hide resolved

added explicit self

b1fbb92

aboedo enabled auto-merge (squash) October 4, 2023 13:27

aboedo merged commit 87182db into main Oct 4, 2023
18 of 19 checks passed

aboedo deleted the andy/pwl-298-paywallstester-allow-for-configuration-for-gtm branch October 4, 2023 13:50

RCGitBot mentioned this pull request Oct 4, 2023

[AUTOMATIC] Release/4.27.1 #3275

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

PaywallsTester: allow for configuration for demos #3260

PaywallsTester: allow for configuration for demos #3260

aboedo commented Sep 28, 2023

tonidero Sep 29, 2023

aboedo Sep 29, 2023

aboedo Sep 28, 2023

aboedo Sep 28, 2023

aboedo Sep 29, 2023

codecov bot commented Sep 29, 2023 •

edited

Loading

NachoSoto left a comment

NachoSoto Oct 2, 2023

NachoSoto Oct 2, 2023

aboedo Oct 2, 2023

NachoSoto Oct 2, 2023

NachoSoto Oct 2, 2023

aboedo Oct 3, 2023

		@@ -1,14 +1,19 @@
		#!/bin/bash -e

		PAYWALLS_TESTER_API_KEY=$REVENUECAT_XCODE_CLOUD_SIMPLE_APP_API_KEY

	self.currentMode = Self.apiKey.isEmpty ? .testing : .custom
	self.currentMode = Self.apiKey.isEmpty ? .listOnly : .custom

PaywallsTester: allow for configuration for demos #3260

PaywallsTester: allow for configuration for demos #3260

Conversation

aboedo commented Sep 28, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov bot commented Sep 29, 2023 • edited Loading

Codecov Report

NachoSoto left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov bot commented Sep 29, 2023 •

edited

Loading