-
Notifications
You must be signed in to change notification settings - Fork 2.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Update "submitted by harvesting" report action copy #48939
Update "submitted by harvesting" report action copy #48939
Conversation
@mollfpr are you able to test this? A.k.a. report "harvesting" / "Delayed submission" should work for you, right? Maybe you can try to just have 1 report harvest / auto-submit, then you can test these changes on that "Submitted" report action? 🙏 |
…mittedByHarvestingCopy
Sure, I'm testing it right now 👍 |
…mittedByHarvestingCopy
@mollfpr any update here? :D 🙏 |
…mittedByHarvestingCopy
Actually this def prob most likely isn't necessary at all |
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.
Sorry for the delay Alex 🙏
The changes looks good to me! I just need to complete the recordings.
Co-authored-by: Luthfi <[email protected]>
Good catch! Thanks @mollfpr ! |
I'm waiting for the expense harvested! |
@Beamanator Is there any way to triggered the harvesting manually? My expense from yesterday didn't get harvested. |
Shoot @mollfpr - does your test policy have |
…mittedByHarvestingCopy
…xpensify/App into beaman-updateSubmittedByHarvestingCopy
@Beamanator Yeah, that's a newly submitted expense. Some of my expense is harvested now, but I don't see the report action for harvested expenses. I wondering if the |
Oh @mollfpr you're looking at a different report than me! |
It's weird only this report ID Update: I might know what is happening. I submitted the 2 previous expenses before changing the policy to approval mode. So it's got harvested and ready to pay instead of for waiting approval. The third expense, it's created after the approval mode and has the action name I think we missing a step in making the policy in approval mode. |
…mittedByHarvestingCopy
oOooOoohhh submitted closed is an interesting one to think about!! For now ya let's move forward mentioning that the policy needs to be on advanced approvals, not submit & close 😬 |
I updated the test steps & will bring up "Submitted & Closed" in the issue! |
Reviewer Checklist
Screenshots/VideosAndroid: Native48939.Android.mp4Android: mWeb Chrome48939.mWeb-Chrome.mp4iOS: Native48939.iOS.moviOS: mWeb Safari48939.mWeb-Safari.movMacOS: Chrome / Safari48939.Web.mp4MacOS: Desktop48939.Desktop.mp4 |
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.
LGTM 🚀
@neil-marcellini 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] |
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.
Looks great and seems to test well. Thanks.
✋ 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/neil-marcellini in version: 9.0.41-0 🚀
|
@Beamanator, what does "Let report auto-harvest" mean? Is this auto submission? |
@kavimuru Yes! It should be submitted automatically after setting the delayed submission. |
@mollfpr Then should we wait for 24 hours if it is set to "Daily"? |
@kavimuru yes, correct!! Thanks! |
@@ -439,7 +439,7 @@ const ContextMenuActions: ContextMenuAction[] = [ | |||
setClipboardMessage(CONST.ACTIONABLE_TRACK_EXPENSE_WHISPER_MESSAGE); | |||
} else if (ReportActionsUtils.isRenamedAction(reportAction)) { | |||
setClipboardMessage(ReportActionsUtils.getRenamedAction(reportAction)); | |||
} else if (reportAction?.actionName === CONST.REPORT.ACTIONS.TYPE.SUBMITTED) { | |||
} else if (ReportActionsUtils.isActionOfType(reportAction, CONST.REPORT.ACTIONS.TYPE.SUBMITTED)) { |
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.
Hmm I might need to update this to copy the "automatically..." text, not just "submitted..." 🙃
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.
fixing in #49909
@@ -3713,7 +3713,7 @@ function getReportName( | |||
} | |||
const parentReportActionMessage = ReportActionsUtils.getReportActionMessage(parentReportAction); | |||
|
|||
if (parentReportAction?.actionName === CONST.REPORT.ACTIONS.TYPE.SUBMITTED) { | |||
if (ReportActionsUtils.isActionOfType(parentReportAction, CONST.REPORT.ACTIONS.TYPE.SUBMITTED)) { | |||
return getIOUSubmittedMessage(parentReportAction); |
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.
hmm same here??
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 - create a thread off of this auto-submitted action, you'll see the report name is the old text
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.
fixing in #49909
@Beamanator Tester has to wait for 24 hours, is there anyway this could be verified without waiting? |
@Beamanator @mollfpr could this be verified internally? |
Yes @kavimuru i can verify internally! |
🚀 Deployed to production by https://github.com/jasperhuangg in version: 9.0.41-10 🚀
|
Details
This PR helps us identify report actions where "submission" happened automatically (via delayed submission) vs manually by a human
Fixed Issues
$ #35091
Tests
automatically submitted <amount> via delayed submission
where the "Delayed Submission" test links out to https://help.expensify.com/articles/expensify-classic/reports/Automatically-submit-employee-reportsOffline tests
N/A - delayed submission doesn't work offline
QA Steps
Same as above
PR Author Checklist
### Fixed Issues
section aboveTests
sectionOffline steps
sectionQA steps
sectiontoggleReport
and notonIconClick
)myBool && <MyComponent />
.src/languages/*
files and using the translation methodSTYLE.md
) were followedAvatar
, I verified the components usingAvatar
are working as expected)StyleUtils.getBackgroundAndBorderStyle(theme.componentBG)
)Avatar
is modified, I verified thatAvatar
is working as expected in all cases)Design
label and/or tagged@Expensify/design
so the design team can review the changes.ScrollView
component to make it scrollable when more elements are added to the page.main
branch was merged into this PR after a review, I tested again and verified the outcome was still expected according to theTest
steps.Screenshots/Videos
Android: Native
Android: mWeb Chrome
iOS: Native
iOS: mWeb Safari
MacOS: Chrome / Safari
MacOS: Desktop