Add login list format update (Array -> Object)#10959
Conversation
|
It was agreed this isn't needed, so closing |
|
Removing migration since that's not needed, but we do need to handle login lists as if they're objects |
mananjadhav
left a comment
There was a problem hiding this comment.
One small comment, rest looks fine. @Beamanator Is this ready to test?
|
yes @mananjadhav this is ready to test! I'll move that string to a CONST but feel free to test in the meantime 👍 |
|
Thanks for patience here @Beamanator and @srikarparsi. It took a bit longer to test as I needed to go through all the screens. Also, I would like to request a raise in the compensation for this one. While it had okay number of files, QA effort was a bit more on this one. Let me know what you think about it? 🎀 👀 🎀
ScreenshotsWebweb-login-list-object.movMobile Web - Chromemweb-chrome-login-list-object.movMobile Web - Safarimweb-safari-login-list-object.movDesktopdesktop-login-list-object.moviOSios-login-list-object.movAndroidandroid-login-list-object.mov |
|
@srikarparsi looks like this was merged without the checklist test passing. Please add a note explaining why this was done and remove the |
|
@mananjadhav, I'm not too sure about the compensation structure currently, @Beamanator what are your thoughts? |
|
Finished checklist:
|
|
🚀 Deployed to staging by @srikarparsi in version: 1.2.21-3 🚀
|
|
@mananjadhav if this took you a long time to test, I think it's fair to increase payment 👍 What do you think is fair for the work you did on this one? |
|
Our standard for PR Reviews start with |
|
Making it easy @mananjadhav , thanks. Can you add to the sheet for now plz? https://docs.google.com/spreadsheets/d/1GSOIuYXcDePpJpEi_ci5vqv_WGgQ7Ttb6_onuBtCI0U/edit#gid=0 I'll be issuing payments next week then proposing a process update for internal PR reviews |
|
Thanks @mallenexpensify. Updated. |
|
🚀 Deployed to production by @Julesssss in version: 1.2.21-4 🚀
|
Details
We need to update LOGIN_LIST to be an object, not an array. This is similar to the format of
personalDetailsat the moment. Why do we need this? This is useful so we can update one individual login at a time, in Onyx - this will be very useful in the near future when we have red brick road errors that can show up on any login (if resending a verification message, if deleting, if adding, or if setting a login as default)Note: I had to handle the case where login list is an object OR array b/c the server is currently sending login list as an array. After this PR gets to production, I'll update the server (Web-E) to only send the login list as an object, then I will come back here to clean up the "this or that" workaround
Fixed Issues
$ #10960
Tests
PR Review Checklist
Contributor (PR Author) Checklist
### Fixed Issuessection aboveTestssectionQA stepssectiontoggleReportand notonIconClick)src/languages/*filesWaiting for Copylabel for a copy review on the original GH to get the correct copy.STYLE.md) were followedAvatar, I verified the components usingAvatarare working as expected)/** comment above it */thisproperly so there are no scoping issues (i.e. foronClick={this.submit}the methodthis.submitshould be bound tothisin the constructor)thisare necessary to be bound (i.e. avoidthis.submit = this.submit.bind(this);ifthis.submitis never passed to a component event handler likeonClick)StyleUtils.getBackgroundAndBorderStyle(themeColors.componentBG)Avataris modified, I verified thatAvataris working as expected in all cases)PR Reviewer Checklist
The Contributor+ will copy/paste it into a new comment and complete it after the author checklist is completed
### Fixed Issuessection aboveTestssectionQA stepssectiontoggleReportand notonIconClick).src/languages/*filesSTYLE.md) were followedAvatar, I verified the components usingAvatarhave been tested & I retested again)/** comment above it */displayNamepropertythisproperly so there are no scoping issues (i.e. foronClick={this.submit}the methodthis.submitshould be bound tothisin the constructor)thisare necessary to be bound (i.e. avoidthis.submit = this.submit.bind(this);ifthis.submitis never passed to a component event handler likeonClick)StyleUtils.getBackgroundAndBorderStyle(themeColors.componentBG)Avataris modified, I verified thatAvataris working as expected in all cases)QA Steps
Same as above!
Screenshots
Web
Screen.Recording.2022-10-24.at.5.13.02.PM.mov
Mobile Web
Screen.Recording.2022-10-24.at.5.16.45.PM.mov
Desktop
Screen.Recording.2022-10-24.at.5.20.16.PM.mov
iOS
Screen.Recording.2022-10-24.at.5.24.00.PM.mov
Android
Screen.Recording.2022-10-24.at.5.27.46.PM.mov