Remove all uses of report.ownerEmail and replace with ownerAccountID where appropriate#22075
Conversation
report.ownerEmail and replace with ownerAccountID where appropriate
|
@chiragsalian Please copy/paste the Reviewer Checklist from here into a new comment on this PR and complete it. If you have the K2 extension, you can simply click: [this button] |
|
Requested C+ Review: https://expensify.slack.com/archives/C02NK2DQWUX/p1688658735225979 |
|
Thanks for volunteering to review & test @rushatgabhane ! |
Reviewer Checklist
Screenshots/Videos |
There was a problem hiding this comment.
Bug: assigned person is blank.
Steps:
- From account
A, create a task and assign it to accountB. - And "Share somewhere" to account
C. - Login using account
Cand go to the report containing the task.
Expected: B's email or display name should be shown as the person assigned to the task.
Actual: assigned person is blank.
Note that after clicking on the task, assignee is not blank
Screen.Recording.2023-07-07.at.09.48.27.mov
|
Hmmm I just reproduced it on |
This comment was marked as off-topic.
This comment was marked as off-topic.
|
@PauloGasparSv Please copy/paste the Reviewer Checklist from here into a new comment on this PR and complete it. If you have the K2 extension, you can simply click: [this button] |
|
All you @chiragsalian (removing you @PauloGasparSv since chirag was already assigned to review - but feel free to review if you want!) |
|
Looks like you've got a merge conflict to address. |
|
@chiragsalian conflicts fixed! |
|
retesting |
rushatgabhane
left a comment
There was a problem hiding this comment.
only ReportUtils was changed, and it tests well.
|
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
|
🚀 Deployed to staging by https://github.com/chiragsalian in version: 1.3.39-0 🚀
|
|
@anmurali FYI we need to pay @rushatgabhane out $1k for internal PR review for this PR. |
|
🚀 Deployed to production by https://github.com/francoisl in version: 1.3.39-11 🚀
|

















Details
Fixed Issues
$ Related to https://github.com/Expensify/Expensify/issues/294647
Tests
To cover our bases, let's create new accounts while performing these tasks:
For each of ^ tests, verify:
Finally, these two tests:
Offline tests
All of the above should be able to be done offline too, except logging in to a brand new account
QA Steps
Same as above
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectiontoggleReportand notonIconClick)myBool && <MyComponent />.src/languages/*files and using the translation methodWaiting 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)ScrollViewcomponent to make it scrollable when more elements are added to the page.mainbranch was merged into this PR after a review, I tested again and verified the outcome was still expected according to theTeststeps.Screenshots/Videos
Web
Mobile Web - Chrome
Mobile Web - Safari
Desktop
iOS
Android