Support join URLs to generate OPcodes on phone, Fix URL scheme, "Paste" improvements
#1187
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
e-mission/e-mission-docs#1076 With the generation of OPcodes now occuring on the phone, several of the functions related to opcode generation will be moved from the join page to the phone. I put these in a separate file "opcode.ts" which will also pull in several of the functions from dynamicConfig that are related to validating and parsing opcodes While doing this, I adjusted the names of these functions to a unified convention, so let me describe where each one came from: generateRandomString: from join page's _getRandomString getStudyNameFromToken: from dynamicConfig's extractStudyName getSubgroupFromToken: from dynamicConfig's extractSubgroup getStudyNameFromUrl: from _getStudyName which was on both join page and dynamicConfig generate generateOpcodeFromUrl: from part of join page's validateAndGenerateToken (For reference, here is the current version of the join page that some of these functions are sourced from: https://github.com/e-mission/nrel-openpath-join-page/blob/main/frontend/index.html#L515) getTokenFromUrl and joinWithTokenOrUrl are new and help us handle 3 types of input: i) an actual token, ii) 'login_token' link that contains a token, and iii) 'join' link that gives parameters for a token to be generated on the phone
In the current version of the app, QR codes have to be scanned from within the app; scanning the QR code from the device's camera or any other external app will not successfully launch openpath. This used to work and was the method we used before we made a lot of changes to the onboarding flow. I found that the plugin relies on the existence of a global function (ie existing on the 'window' object). If we just supply this, the app can handle any emission:// link (or nrelopenpath:// link for the NREL version of the app) I'm also putting this function in the AppContext so it can be invoked in a "React"-friendly way downstream
Now that reading URLs or tokens, generating a token (if applicable), and joining a study is handled by handleOpenURL (and the functions in opcode.ts it calls), we can use this on the WelcomePage. Scanning or entering an opcode or URL on the WelcomePage should do the exact same thing as handling an emission:// link that was opened from some external location. getCode and loginWithToken are not needed anymore, scanCode just needs to call handleOpenURL. I saw an opportunity to make the "paste" flow easier by providing a shortcut; when "Paste code" is clicked the app will read the user's clipboard and try to join using that. Only if that fails will the paste modal need to be opened, as was the previous UX. I used cordova-clipboard for this.
I had this change locally and did not mean to commit it
-catch error that could be thrown from getTokenFromUrl -pass return value of false on either of the catch blocks -only show "proceeding with OPcode ..." if result from initByUser was true -ability to pass style through to AlertBar; wordBreak: break-all so that long OPcodes will not be forced onto one line
JGreenlee
changed the title
join URLs to generate OPcodes on phone, Fix URL scheme, "Paste" improvements
Instead of hardcoding `emission://`, we can grab it from the package json. This way, the 'emission' string will only need to be changed in one place for other versions of the app. And since QrCode.tsx is able to handle either URLs or plain tokens, we can also simplify PopOpCode to just accept the plain token and pass this through to QrCode.tsx
Looking at these 2 functions I realized they can be combined and error handling unified, so I combined these to make a new function using async/await syntax. I chose to put the new function in dynamicConfig because this avoids a circular dependency at the module level (which is allowed but not tidy) Now, `dynamicConfig.ts` imports from `opcode.ts` but `opcode.ts` no longer imports anything from `dynamicConfig.ts`. Updated the tests for dynamicConfig to validate that the correct error messages are shown.
Because joinWithTokenOrUrl in dynamicConfig.ts calls functions from opcode.ts and there are tests for that, most of opcode.ts is already covered. However, I am adding a few more dedicated tests to cover more scenarios.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #1187 +/- ##
==========================================
+ Coverage 29.41% 29.98% +0.56%
==========================================
Files 122 123 +1
Lines 4919 4933 +14
Branches 1125 1091 -34
==========================================
+ Hits 1447 1479 +32
+ Misses 3470 3454 -16
+ Partials 2 0 -2
Flags with carried forward coverage won't be shown. Click here to find out more.
|
|
@JGreenlee can we add some stats to see how people actually provide us with the opcode - launch via URL or scan from app? That will help us smooth the onboarding flow further. |
shankari
approved these changes
Oct 23, 2024
We have multiple ways to initiate onboarding:
- scan QR in the app
- paste from clipboard in the app
- entering to a textbox in the app
- launching a nrelopenpath:// link from an external app (browser, camera)
We want to keep track of how often these methods are used, as well as how often they are successful.
First I added the 'onboard' stat which represents an attempt to onboard. The reading has 'configUpdated' (to indicate whether the attempt was successful), and 'joinMethod' ('scan', 'paste', 'textbox', or 'external')
I also added "open_qr_scanner" and "paste_token" events (no readings) because the user might open the QR scanner but not scan any QR codes. An 'onboard' stat would not be recorded, but we still want to know if the user clicked "Scan QR"
Lastly, I added 'onboarding_state' as a more general stat for evaluating our onboarding process.
We should be able to compare the timestamp at each reading to see how long users take at each stage of the onboarding process – as well as how long the entire onboarding process takes
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.
add opcode.ts, extracting functions from dynamicConfig and the join page
e-mission/e-mission-docs#1076
With the generation of OPcodes now occuring on the phone, several of the functions related to opcode generation will be moved from the join page to the phone.
I put these in a separate file "opcode.ts" which will also pull in several of the functions from dynamicConfig that are related to validating and parsing opcodes
While doing this, I adjusted the names of these functions to a unified convention, so let me describe where each one came from:
generateRandomString: from join page's _getRandomString
getStudyNameFromToken: from dynamicConfig's extractStudyName
getSubgroupFromToken: from dynamicConfig's extractSubgroup
getStudyNameFromUrl: from _getStudyName which was on both join page and dynamicConfig
generate
generateOpcodeFromUrl: from part of join page's validateAndGenerateToken
(For reference, here is the current version of the join page that some of these functions are sourced from: https://github.com/e-mission/nrel-openpath-join-page/blob/main/frontend/index.html#L515)
getTokenFromUrl and joinWithTokenOrUrl are new and help us handle 3 types of input: i) an actual token, ii) 'login_token' link that contains a token, and iii) 'join' link that gives parameters for a token to be generated on the phone
implement global 'handleOpenURL' function
In the current version of the app, QR codes have to be scanned from within the app; scanning the QR code from the device's camera or any other external app will not successfully launch openpath. This used to work and was the method we used before we made a lot of changes to the onboarding flow.
I found that the plugin relies on the existence of a global function (ie existing on the 'window' object).
If we just supply this, the app can handle any emission:// link (or nrelopenpath:// link for the NREL version of the app)
I'm also putting this function in the AppContext so it can be invoked in a "React"-friendly way downstream
WelcomePage: handle tokens, 'join' / 'login_token' URLs, autopaste
Now that reading URLs or tokens, generating a token (if applicable), and joining a study is handled by handleOpenURL (and the functions in opcode.ts it calls), we can use this on the WelcomePage.
Scanning or entering an opcode or URL on the WelcomePage should do the exact same thing as handling an emission:// link that was opened from some external location.
getCode and loginWithToken are not needed anymore, scanCode just needs to call handleOpenURL.
I saw an opportunity to make the "paste" flow easier by providing a shortcut; when "Paste code" is clicked the app will read the user's clipboard and try to join using that. Only if that fails will the paste modal need to be opened, as was the previous UX.
I used cordova-clipboard for this.
revert commenting out '10.0.2.2' downloadURL
I had this change locally and did not mean to commit it
better error handling in opcode.ts
-catch error that could be thrown from getTokenFromUrl
-pass return value of false on either of the catch blocks
-only show "proceeding with OPcode ..." if result from initByUser was true
-ability to pass style through to AlertBar; wordBreak: break-all so that long OPcodes will not be forced onto one line
use URL_SCHEME from package.json; simplify PopOpCode.tsx
Instead of hardcoding
emission://, we can grab it from the package json. This way, the 'emission' string will only need to be changed in one place for other versions of the app.And since QrCode.tsx is able to handle either URLs or plain tokens, we can also simplify PopOpCode to just accept the plain token and pass this through to QrCode.tsx
combine opcode.joinWithTokenOrUrl with dynamicConfig.initByUser
Looking at these 2 functions I realized they can be combined and error handling unified, so I combined these to make a new function using async/await syntax.
I chose to put the new function in dynamicConfig because this avoids a circular dependency at the module level (which is allowed but not tidy)
Now,
dynamicConfig.tsimports fromopcode.tsbutopcode.tsno longer imports anything fromdynamicConfig.ts.Updated the tests for dynamicConfig to validate that the correct error messages are shown.
add tests for opcode.ts
Because joinWithTokenOrUrl in dynamicConfig.ts calls functions from opcode.ts and there are tests for that, most of opcode.ts is already covered. However, I am adding a few more dedicated tests to cover more scenarios.