PaywallsTester: allow for configuration for demos by aboedo · Pull Request #3260 · RevenueCat/purchases-ios

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

-    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

-                ? 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

@@ -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

-    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.

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

+    private let apiKey = ""

-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?

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

+        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

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

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


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


-}
+    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

…ng on the current mode

@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

Comment thread Tests/TestingApps/PaywallsTester/PaywallsTester/Configuration.swift Outdated

added explicit self

b1fbb92

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

aboedo merged commit 87182db into main Oct 4, 2023

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

		@@ -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

Conversation

aboedo commented Sep 28, 2023

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

codecov Bot commented Sep 29, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

NachoSoto left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

codecov Bot commented Sep 29, 2023 •

edited

Loading