Only consider personal details with logins set in OptionsListUtils.getOptions
#21159
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -603,6 +603,12 @@ function getOptions( | |
| }; | ||
| } | ||
|
|
||
| // We're only picking personal details that have logins set | ||
| // This is a temporary fix for all the logic that's been breaking because of the new privacy changes | ||
| // See https://github.com/Expensify/Expensify/issues/293465 for more context | ||
| // eslint-disable-next-line no-param-reassign | ||
| personalDetails = _.pick(personalDetails, (detail) => Boolean(detail.login)); | ||
|
Comment on lines
+606
to
+610
There was a problem hiding this comment. What are the effects of excluding personal details from this object? They don't appear in the option list? Do you know where this was causing problems down the line? I imagine we where trying to use There was a problem hiding this comment. Because Tasks takes the login from the option and does a lot more stuff with it, so we can't just do that Also, it's not guaranteed that personalDetails.displayName is equal to their login. If they have a display name set already then it won't be equal. That only applies to new users that haven't set their personal details yet. There was a problem hiding this comment. I assume that we actually needed the login (emails) for the flows that broke downstream, so using the There was a problem hiding this comment.
Then it can be that the In the cases I have seen so far, when the There was a problem hiding this comment.
Is this still true if the user sets their display name? There was a problem hiding this comment. that's definitely possible - maybe you can check with Puneet's updated query here? https://github.com/Expensify/Auth/pull/8153 There was a problem hiding this comment. ok, this Auth fix works for 1:1 DM's There was a problem hiding this comment. It's intended to also work for group DMs There was a problem hiding this comment. Now to the last part of the issue, where if I have DM some random new users and send some messages. The chat appears in LHN. Again, when I go to chat switcher and type full email, and click on option, it creates a new report with same user There was a problem hiding this comment. I reported here, in case it is already known There was a problem hiding this comment. Hi, we should have placed this code just before creating an option It would have prevented bug - Tooltip content only have comma in content when hovering group chat with non-register users More details - #22593 (comment) |
||
|
|
||
| let recentReportOptions = []; | ||
| let personalDetailsOptions = []; | ||
| const reportMapForAccountIDs = {}; | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there any follow up issues to address the temp fix?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep, we just recently merged the PR that handles when you try to chat with someone whose login you don't have in Onyx, so we should be able to undo this now. I'll create an issue for a contributor to do it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Or maybe actually I can just revert this.