#7618: increase managed storage delay and link extension immediately for CWS installs #7628
Conversation
| @@ -52,3 +53,17 @@ describe("readManagedStorageByKey", () => { | |||
| ); | |||
| }); | |||
| }); | |||
|
|
|||
| describe("watchStorageInitialization", () => { | |||
There was a problem hiding this comment.
FIXME: this test doesn't work
There was a problem hiding this comment.
Why not use useFakeTimers and runOnlyPendingTimers?
There was a problem hiding this comment.
Yep, that's what I will try to use now - was just trying to get a quick and dirty test originally without breaking the other tests in the module
src/background/installer.ts
Outdated
| * Returns true if this appears to be a Chrome Web Store install and/or the user has an app URL where they're | ||
| * authenticated so the extension can be linked. | ||
| */ | ||
| async function isEndUserInstall(): Promise<boolean> { |
There was a problem hiding this comment.
There's no great way to test this method in Jest, b/c it would just involve mocking browser.tabs.query. So, we will rely on the Rainforest QA tests and manual testing
There was a problem hiding this comment.
nit: I might add "likely" to the name of this func, e.g. isLikelyEndUserInstall
twschiller
changed the title
twschiller
changed the title
twschiller
changed the title
twschiller
changed the title
| @@ -421,7 +421,7 @@ export async function updateDeployments(): Promise<void> { | |||
| sso: false, | |||
| }); | |||
|
|
|||
| void browser.runtime.openOptionsPage(); | |||
| await browser.runtime.openOptionsPage(); | |||
There was a problem hiding this comment.
No reason for it to be void because that's the standard behavior
|
When the PR is merged, the first loom link found on this PR will be posted to |
|
|
||
| /** | ||
| * The managedStorageState, or undefined if it hasn't been initialized yet. | ||
| */ | ||
| let managedStorageState: ManagedStorageState | undefined; | ||
| let managedStorageSnapshot: Nullishable<ManagedStorageState>; |
There was a problem hiding this comment.
non-blocking: out of curiosity, why use the word snapshot here?
There was a problem hiding this comment.
Snapshot is the terminology that React useExternalStore uses and is the method here:
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #7628 +/- ##
==========================================
- Coverage 72.80% 72.77% -0.04%
==========================================
Files 1225 1225
Lines 38182 38212 +30
Branches 7180 7185 +5
==========================================
+ Hits 27798 27807 +9
- Misses 10384 10405 +21 ☔ View full report in Codecov by Sentry. |
What does this PR do?
handleInstallto callopenInstallPageimmediately if the installation appears to be an end-user installationRemaining Work
handleInstallto immediately look for app onboarding page, which indicates user is not an enterprise userDiscussion
browser.storage.managedinitialization semantics and provide initialization event w3c/webextensions#547Demo
Future Work
Checklist
src/tsconfig.strictNullChecks.json(if possible)