Rewrite PushNotify.ts ➡️ PushNotifySettings.ts #1090
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
added 30 commits
October 17, 2023 16:18
first attempt at converting to a ts file, not fully functional, but did find several functions that were no longer used and so have been removed
we are now not using the angular service anymore, so should access everything from the new ts file
we had a bug related to this call, so adding a note to future development to make sure we're aware of the ramifications that this can cause.
was breaking because I spelled cordova wrong
remove unused key, update to single source of truth for INTRO_DONE_KEY
Pre-migration, there was a "consent done" event broadcasted from startprefs whenever the user marked consent. However, Reach and typescript does not handle events like this. Instead, we are extracting the code into a function, retrieving it from the plugin (for now), and then calling those functions when consent is marked. As a part of this, unify the reading of "isIntroDone" to the method in onboardingHelper, rather than storing a variable in startPrefs
we actually need to wait until fully consented, and the user registered before we can take the actions in the plugins I noticed this because of an error message that appeared when logging in, indicating that the user was not registered, so the device settings could not be stored moving these calls to after the user has FULLY consented, resolved that issue
adding descriptions to the functions, and removed some unused imports
after adding more mocked functions, the startprefs now have one test, for getting the consent doc from storage
this test needed a mock of markConsented in the BEMDataCollection plugin, I have added _storage so that when the document is written in, it can also be retrieved -- even though that happens in two different plugins
since testing separately affects the local storage, testing in sequence allows for easier testing. return json from fetch rather than string to make it parseable in storage, had to check for the string undefined, since that was returned in one of the tests
…into startprefs-rewrite also resolved merge conflicts
for consistency, since this is also a function that is async
since we no longer work with the IRB protocol within the app, we don't need to present it to the user
saveQR was a bad place to handle this, moving to markConsented As Jack pointed out, on a reconsent (mark consent after intro done) we want to call registerPush and storeDeviceSettings - that is what is done here centralizing the logic to markConsented eliminated the need for the functions in the other files, since we check for consent done just once
We no longer emit this event, but still need to handle the logic On marking intro done, we will registerPush and storeDeviceSettings e-mission#1072 (comment)
There were several places where I had other log statements, but logDebug was more appropriate, see review for details e-mission#1072 (comment)
added 13 commits
October 30, 2023 10:16
convert the module to ts, update name, and convert to individual functions, update imports (there are no imports of this file)
to continue work with intro and reconsent related events
after removing the angular module, needed to update to include the new way of inclusion with imported functions
getting away from more angular conventions instead of plaform.ready() call startup at the same time as registerPush, which is called on intro done and reconsent don't need _datacollect variable, I think it is more clear to access the plugin each time rather than hold a reference
trying the solution I proposed here: e-mission/e-mission-docs#1024
trialing calling the functions directly rather than relying on the events e-mission/e-mission-docs#1024 (comment)
|
This has been replaced by #1093 since we decided to go with custom events, and I had gone back and forth in this PR. Starting a new PR will hopefully help limit confusing blame. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Rewrite the service that handles the push notification settings.
This PR will be a continuation of #1072, since it affects the "reconsent" and "intro done" actions. I will manage updates made during review to that service as I merge them into this service.