Rename source (connected with session context) to logInActionSource to avoid redacting it from reports.
_task: ENG-2131
Details
Details
- Reviewers
tomek atul ashoat - Commits
- rCOMM36084420e00c: rename `source` to `logInActionSource`
- Typecheck
- Check if logInActionSource is properly passed to keyserver
- Trrigger one of logInActionSource actions and trigger app crash in dev tools, check if logInActionSource is present in the action payload in the latest crash report from the database
example:
{
"type": "SET_NEW_SESSION",
"payload": {
"sessionChange": {
"cookieInvalidated": false,
"currentUserInfo": {
"id": "[redacted-3087747816]",
"username": "[redacted-642983437]"
}
},
"preRequestUserState": null,
"logInActionSource": "SOCKET_AUTH_ERROR_RESOLUTION_ATTEMPT"
}
},- Check if existing clients can log in (as described here: https://phab.comm.dev/D5517#163191)
Diff Detail
Diff Detail
- Repository
- rCOMM Comm
- Branch
- stop-redacting-login-source
- Lint
No Lint Coverage - Unit
No Test Coverage
Event Timeline
| keyserver/src/responders/user-responders.js | ||
|---|---|---|
| 193 ↗ | (On Diff #18019) | This change will cause literally all existing clients to fail to log in!! |
| 193 ↗ | (On Diff #18019) | Can we rename loginActionSources to logInActionSources for consistency? (capitalization change) |
| lib/actions/user-actions.js | ||
| 121 ↗ | (On Diff #18019) | We should not change the name of the parameter passed to keyserver. We could do this and make keyserver look at either source or logInActionSource, but that seems complicated and unnecessary |
| keyserver/src/responders/user-responders.js | ||
|---|---|---|
| 193 ↗ | (On Diff #18019) | ah right, I missed the { strict: true } mode, thanks for pointing |
Comment Actions
Looks good to me!!
@kamil, can you modify the Test Plan before landing to make sure you're testing existing clients? To do this, I usually checkout master and then build a Release version of the app (so that it won't reload when I change the files in Git), and then checkout my version of the code and run the keyserver with that.
Then I configure the Release build to connect to my local dev server. To do this, you may need to modify some configs:
- You'll need to configure the Release build to connect to the local dev server.
- On the iOS simulator, if you've previously deployed a Debug build to the same device, redux-persist should persist the urlPrefix (URL to dev server), so you shouldn't have to do anything
- If you haven't previously deployed a Debug build, then you might have do some more work. Read here for some details
- On iOS, you'll need to update native/ios/Comm/Info.release.plist.
- NSAppTransportSecurity should look like it does in native/ios/Comm/Info.debug.plist
- On Android, you'll need to update native/android/app/src/release/res/xml/network_security_config.xml.
- It should look like native/android/app/src/debug/res/xml/network_security_config.xml
(Note – no need to test multiple environments... you can just test it once, on iOS or Android.)
Comment Actions
It seems like having this process described on Notion might be beneficial. @kamil can you take care of it?
Comment Actions
It seems like having this process described on Notion might be beneficial. @kamil can you take care of it?
Done: https://www.notion.so/commapp/Testing-existing-clients-934ee4f593584d35863e53924fed5414 (currently in Dev Notes -> Unsorted, not sure if there is a better place for this).