Fix prompting again after closing notification permission prompt if site has its own ServiceWorker

[jkasten2]

Description

1 Line Summary

Fix prompting again after closing notification permission prompt if site has its own ServiceWorker.

Details

This PR fixes two things related to this issue:

1. requestNotificationPermission() - this was always returning undefined due to a conversion bug
2. getRegistration() - did not check if the service worker was a OneSignal one, resulting in wrong assumptions being made.

Validation

Tests

Added a unit test for requestNotificationPermission(), this was always returning undefined before the changes in the first commit.

Checklist

• All the automated tests pass or I explained why that is not possible
• I have personally tested this on my machine or explained why that is not possible
• I have included test coverage for these changes or explained why they are not needed

Programming Checklist
Interfaces:

• Don't use default export
• New interfaces are in model files

Functions:

• Don't use default export
• All function signatures have return types
• Helpers should not access any data but rather be given the data to operate on.

Typescript:

• No Typescript warnings
• Avoid silencing null/undefined warnings with the exclamation point

Other:

• Iteration: refrain from using elem of array syntax. Prefer forEach or use map
• Avoid using global OneSignal accessor for context if possible. Instead, we can pass it to function/constructor so that we don't call OneSignal.context

Screenshots

Info

Checklist

• I have included screenshots/recordings of the intended results or explained why they are not needed

---

Related Tickets

---

---

This change is 

[shepherd-l]

Can confirm that the native permission prompt does not show again after dismissing with "X" with this fix. Tested with the example web app using a using a dummy non-OneSignal service worker.

Bug reproduced by:
Using the example web app using a dummy non-OneSignal service worker.

1. Clear web cache
2. Open website
3. Click "Subscribe" on slide down push prompt
4. Dismiss native permission prompt with "X"

• Main: Shows native permission prompt again
• This PR: Does not show native permission prompt again

Reviewable status: 0 of 7 files reviewed, 2 unresolved discussions (waiting on @emawby, @jennantilla, @jinliu9508, @jkasten2, @nan-li, and @rgomezp)

---

src/shared/managers/ServiceWorkerManager.ts line 42 at r3 (raw file):

    ServiceWorkerRegistration | null | undefined
  > {
    return await ServiceWorkerUtilHelper.getRegistration(

Why was this await removed? Isn't getRegistration() an async method that should be awaited?

---

src/shared/managers/SubscriptionManager.ts line 336 at r1 (raw file):

    // TODO: Clean up our custom NotificationPermission enum
    //         in favor of TS union type NotificationPermission instead of converting
    return results as NotificationPermission;

Nice find! I noticed that this fixes Error: OneSignal service worker not found! from the tests: registerForPushNotifications: before OneSignal.initialized and registerForPushNotifications: after OneSignal.initialized even though they still passed before, and now log PushPermissionNotGrantedError: The user dismissed the permission prompt.

[jkasten2]

Thanks for testing!

Reviewable status: 0 of 7 files reviewed, 2 unresolved discussions (waiting on @emawby, @jennantilla, @jinliu9508, @nan-li, @rgomezp, and @shepherd-l)

---

src/shared/managers/ServiceWorkerManager.ts line 42 at r3 (raw file):

Previously, shepherd-l (Shepherd) wrote…

Why was this await removed? Isn't getRegistration() an async method that should be awaited?

The function doesn't read the value, nor does it do anything after which would affect any order of operations.

Here is an example I put together to show there is no-side effects here:

async function asyncResult(): Promise<Number> {
  return Promise.resolve(123);
}

// Preferred (more concise) - This method doesn't consume the value, so we don't need 'await'.
async function noAwaitExample(): Promise<Number> {
  return asyncResult();
}

// Unnecessary (more verbose) - You can call 'await' but there isn't a point if the function doesn't need to read the value, or wait on order of operations
async function awaitExample(): Promise<Number> {
  return await asyncResult();
}

// Code showing both work the same, no-side effects.
async function printResults(): Promise<void> {
  console.log("awaitExample: " + await awaitExample());
  console.log("noAwaitExample: " + await noAwaitExample());
}
printResults();

Sandbox to try it out:
https://www.typescriptlang.org/play/?#code/IYZwngdgxgBAZgV2gFwJYHsI1JKAlAUxAQBtkAKASgC4YAFAJ3QFtUQCAeAOQWYCMCDAHwwA3gCgYMBgWQIGWRizYEAdDJDoSANwLkAjACYAzJQDc4gL7jxAelv0ZcQTIAmMcs3QyYUTFBVKGABaGAAVAAs2GGZZCPR3V3QiCAByZF9MYliYZAiCGG1gEgQCABoYTRgAdwKktIyIAgJ3VOBq4FRkVNVxHGh4JCg0TBgIdABBDq6AUQAPYGYABxI9Gkdldm5eAWExSWlZeSx+-CJSCnMrG3sYAFUIJqgiEGAGMA8vH10GPnR2IKhACa6AQvmAWCgxRIMDa026MD4CAyeUEBTYDWwMCW6FQEAyqDguXygxQGCwSRS6TGzXcyHQh2AdJJRRK5Rg3hqnQyo28rkEHKJ6CWgmAIwgID64AGiDJo3a3PmixWa1oSlYWx4-EEIgkUhkcgU2Hh2GlZ2IZCoFmsdgcAGEEgUQPFqniAOaI9B5GreADWxKdi3Z42CIFQ-JgBDgzmGIF6p1Jw3J2IYeOQhAtyBAVDVTA1nG0uNcuoOfglWjUJHQbvIACIFbMFstVrRazAANTG7ldxvK1ZUK5SMuaVaqKs12vjKaKpsq1sdnuNSbwpXNtZXaxLVP4jMXbNXIA

---

src/shared/managers/SubscriptionManager.ts line 336 at r1 (raw file):

Previously, shepherd-l (Shepherd) wrote…

Nice find! I noticed that this fixes Error: OneSignal service worker not found! from the tests: registerForPushNotifications: before OneSignal.initialized and registerForPushNotifications: after OneSignal.initialized even though they still passed before, and now log PushPermissionNotGrantedError: The user dismissed the permission prompt.

ya, this is why I created a new unit test file to make sure the conversion is done correctly.