feat(art advisor): enable Milestone 1 #10724

anandaroop · 2024-09-06T14:47:26Z

This PR resolves ONYX-1267

Description

Implements the following logic to determine when to serve the new home view:

A regular user on an app store build sees the legacy home screen by default
A user on a simulator/beta/TestFlight build sees the new home screen by default
A user can toggle a feature flag (ARPreferLegacyHomeScreen) to revert to the legacy home screen for dev/testing/QA

Or, in truth table form:

	isBetaOrDev=false	isBetaOrDev =true
ARPreferLegacyHomeScreen=false	Old screen	New screen
ARPreferLegacyHomeScreen=true	Old screen	Old screen

In addition the new info button presents some helpful links for reporting issues related to the new home screen, or reverting back to the old one.

m1.out.mov

PR Checklist

I have tested my changes on iOS and Android.
I hid my changes behind a feature flag, or they don't need one.
I have included screenshots or videos, or I have not changed the UI.
I have added tests, or my changes don't require any.
I added an app state migration, or my changes do not require one.
I have documented any follow-up work that this PR will require, or it does not require any.
I have added a changelog entry below, or my changes do not require one.

To the reviewers 👀

I would like at least one of the reviewers to run this PR on the simulator or device.

Changelog updates

Cross-platform user-facing changes

iOS user-facing changes

Android user-facing changes

Dev changes

Prefer the new home screen by default in dev/test environment, but allow override - Roop

Need help with something? Have a look at our docs, or get in touch with us.

src/app/Scenes/HomeView/Components/AlphaVersionIndicator.tsx

anandaroop · 2024-09-06T19:15:08Z

src/app/Scenes/Home/HomeContainer.tests.tsx

+describe("conditional rendering of old vs new home screen", () => {
+  beforeEach(() => {
+    jest.clearAllMocks()
+  })
+
+  describe("when running in beta or simulator", () => {
+    beforeEach(() => (ArtsyNativeModule.ArtsyNativeModule.isBetaOrDev = true))


This test run produces:

conditional rendering of old vs new home screen when running in beta or simulator when ARPreferLegacyHomeScreen is false ✅ renders the NEW screen when ARPreferLegacyHomeScreen is true ✅ renders the old screen when NOT running in beta or simulator when ARPreferLegacyHomeScreen is false ✅ renders the old screen when ARPreferLegacyHomeScreen is true ✅ renders the old screen

This matches the table above: the new screen is shown only when you are running in beta/simulator and the "Prefer legacy home screen" has not been toggled on.

For dev/testing purposes you would just toggle that flag on (cc @brainbicycle)

anandaroop · 2024-09-06T19:16:19Z

src/app/Scenes/HomeView/Components/AlphaVersionIndicator.tsx

+            <LinkText
+              onPress={() =>
+                Linking.openURL("https://www.notion.so/artsy/e9958451ea9d44c6b357aba6505c0abc")
+              }
+            >
+              See more on how to switch back
+            </LinkText>


Need to flesh this page out with the some non-technical instructions.

anandaroop · 2024-09-06T19:16:41Z

src/app/Scenes/HomeView/Components/AlphaVersionIndicator.tsx

+              <LinkText
+                fontWeight="bold"
+                onPress={() =>
+                  Linking.openURL("https://www.notion.so/artsy/abc1123548504ae58051405627fb6c9f")
+                }
+              >
+                Notion feedback board
+              </LinkText>


And feedback board is set up now.

anandaroop · 2024-09-06T19:17:31Z

src/app/store/config/features.ts

-  ARUseNewHomeView: {
-    description: "Use new home view",
+  ARPreferLegacyHomeScreen: {
+    description: "Prefer legacy home screen",


Note that we've replaced the old flag with this new one, with reversed semantics. Now you opt out of the new screen (when running in beta or sim).

ArtsyOpenSource · 2024-09-06T19:21:02Z

This PR contains the following changes:

Dev changes (Prefer the new home screen by default in dev/test environment, but allow override - Roop)

Generated by 🚫 dangerJS against be597a9

MounirDhahri

By default, ARPreferLegacyHomeScreen will be falsy, this means that we will be already rendering the new home view for all users which I don't think makes sense now since we have not QA'd the home properly yet and some tracking events are still missing.

anandaroop · 2024-09-07T17:12:44Z

src/app/Scenes/Home/HomeContainer.tsx

+  const preferLegacyHomeScreen = useFeatureFlag("ARPreferLegacyHomeScreen")
+
+  const shouldDisplayNewHomeView = ArtsyNativeModule.isBetaOrDev && !preferLegacyHomeScreen


@MounirDhahri regarding your comment, I think this shouldDisplayNewHomeView condition can only be true in a beta build or simulator, no?

I am assuming that the low-level computation of isBetaOrDev means strictly that you are either in the simulator or beta, but we should be absolutely certain of that (for iOS as well as Android):

eigen/ios/Artsy/App/ARAppStatus.m

Lines 11 to 38 in 8c5e288

+ (BOOL)isDev;

{

#if TARGET_IPHONE_SIMULATOR

return YES;

#elif DEBUG

return YES;

#else

return NO;

#endif

}

+ (BOOL)isBeta;

{

static BOOL isBeta = NO;

static dispatch_once_t onceToken = 0;

dispatch_once(&onceToken, ^{

NSURL *receiptURL = [[NSBundle mainBundle] appStoreReceiptURL];

NSString *receiptURLString = [receiptURL path];

isBeta = [receiptURLString rangeOfString:@"sandboxReceipt"].location != NSNotFound;

});

return isBeta;

}

+ (BOOL)isBetaOrDev;

{

return [self isDev] || [self isBeta];

}

We rely on this to default to staging in betas and prod in app and play store, so if it is not working we are in trouble 😅. It works

nickskalkin

👏

brainbicycle

🙌

dblandin

Awesome 💯

MounirDhahri

LGTM - thanks! I got it now 😄

anandaroop · 2024-09-09T15:52:30Z

Cool, thanks for all the 👀

As discussed in Onyx planning today — the last change I'll smuggle into this PR is to change the initial fetch from 5 sections to 10, to make that spinner a little less obtrusive. (b25a41c)

(UPDATE: rebasing ~~to see if that fixes some unrelated test failures~~ to pick up the fix for those spurious failures)

Also inlines the Notion feedback board link.

anandaroop self-assigned this Sep 6, 2024

artsy-peril bot added the Jira Synced label Sep 6, 2024

dblandin reviewed Sep 6, 2024

View reviewed changes

src/app/Scenes/HomeView/Components/AlphaVersionIndicator.tsx Show resolved Hide resolved

anandaroop commented Sep 6, 2024

View reviewed changes

anandaroop requested review from a team and brainbicycle September 6, 2024 19:18

anandaroop marked this pull request as ready for review September 6, 2024 19:18

MounirDhahri reviewed Sep 7, 2024

View reviewed changes

anandaroop commented Sep 7, 2024

View reviewed changes

nickskalkin previously approved these changes Sep 9, 2024

View reviewed changes

brainbicycle previously approved these changes Sep 9, 2024

View reviewed changes

dblandin previously approved these changes Sep 9, 2024

View reviewed changes

MounirDhahri previously approved these changes Sep 9, 2024

View reviewed changes

anandaroop dismissed stale reviews from MounirDhahri, dblandin, brainbicycle, and nickskalkin via b25a41c September 9, 2024 15:50

anandaroop force-pushed the anandaroop/ONYX-1267-milestone-1 branch from b25a41c to f719b2e Compare September 9, 2024 16:38

anandaroop and others added 6 commits September 9, 2024 13:38

feat: display new home view by default, and allow override

539be51

refactor: extract Alpha indicator

60b4b70

feat: update alpha help text

a59047d

feat: add a hyperlink to the #pdde-art-advisor channel

3eabe95

Also inlines the Notion feedback board link.

test: conditional rendering of old/new home screen

db63fac

feat: request 10 sections initially

be597a9

anandaroop force-pushed the anandaroop/ONYX-1267-milestone-1 branch from f719b2e to be597a9 Compare September 9, 2024 17:38

anandaroop merged commit 863e0f1 into main Sep 9, 2024
7 checks passed

anandaroop deleted the anandaroop/ONYX-1267-milestone-1 branch September 9, 2024 20:56

anandaroop added the Art Advisor label Sep 13, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(art advisor): enable Milestone 1 #10724

feat(art advisor): enable Milestone 1 #10724

anandaroop commented Sep 6, 2024 •

edited

Loading

anandaroop Sep 6, 2024 •

edited

Loading

anandaroop Sep 6, 2024

anandaroop Sep 6, 2024

anandaroop Sep 6, 2024

ArtsyOpenSource commented Sep 6, 2024 •

edited

Loading

MounirDhahri left a comment

anandaroop Sep 7, 2024 •

edited

Loading

brainbicycle Sep 9, 2024

nickskalkin left a comment

brainbicycle left a comment

dblandin left a comment

MounirDhahri left a comment

anandaroop commented Sep 9, 2024 •

edited

Loading

		const preferLegacyHomeScreen = useFeatureFlag("ARPreferLegacyHomeScreen")

		const shouldDisplayNewHomeView = ArtsyNativeModule.isBetaOrDev && !preferLegacyHomeScreen

	+ (BOOL)isDev;
	{
	#if TARGET_IPHONE_SIMULATOR
	return YES;
	#elif DEBUG
	return YES;
	#else
	return NO;
	#endif
	}

	+ (BOOL)isBeta;
	{
	static BOOL isBeta = NO;
	static dispatch_once_t onceToken = 0;
	dispatch_once(&onceToken, ^{
	NSURL *receiptURL = [[NSBundle mainBundle] appStoreReceiptURL];
	NSString *receiptURLString = [receiptURL path];
	isBeta = [receiptURLString rangeOfString:@"sandboxReceipt"].location != NSNotFound;

	});
	return isBeta;
	}

	+ (BOOL)isBetaOrDev;
	{
	return [self isDev] \|\| [self isBeta];
	}

feat(art advisor): enable Milestone 1 #10724

feat(art advisor): enable Milestone 1 #10724

Conversation

anandaroop commented Sep 6, 2024 • edited Loading

Description

PR Checklist

To the reviewers 👀

Changelog updates

Cross-platform user-facing changes

iOS user-facing changes

Android user-facing changes

Dev changes

anandaroop Sep 6, 2024 • edited Loading

Choose a reason for hiding this comment

anandaroop Sep 6, 2024

Choose a reason for hiding this comment

anandaroop Sep 6, 2024

Choose a reason for hiding this comment

anandaroop Sep 6, 2024

Choose a reason for hiding this comment

ArtsyOpenSource commented Sep 6, 2024 • edited Loading

This PR contains the following changes:

MounirDhahri left a comment

Choose a reason for hiding this comment

anandaroop Sep 7, 2024 • edited Loading

Choose a reason for hiding this comment

brainbicycle Sep 9, 2024

Choose a reason for hiding this comment

nickskalkin left a comment

Choose a reason for hiding this comment

brainbicycle left a comment

Choose a reason for hiding this comment

dblandin left a comment

Choose a reason for hiding this comment

MounirDhahri left a comment

Choose a reason for hiding this comment

anandaroop commented Sep 9, 2024 • edited Loading

anandaroop commented Sep 6, 2024 •

edited

Loading

anandaroop Sep 6, 2024 •

edited

Loading

ArtsyOpenSource commented Sep 6, 2024 •

edited

Loading

anandaroop Sep 7, 2024 •

edited

Loading

anandaroop commented Sep 9, 2024 •

edited

Loading