-
Notifications
You must be signed in to change notification settings - Fork 3k
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 Report Next Steps to fallback to "an admin" instead of policy owner if no reimburser set #49424
Conversation
@Pujan92 I think you can just approve and we'll handle this internally, thank you! |
@roryabraham 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] |
really wrestling with my VM trying to test this |
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.
Documenting my test steps in a way that's a bit more explicit/clear to me:
- Sign into NewDot with a new account,
BossOwner
- Select
Manage my team's expenses
during onboarding to create a collect policy - Go to
Settings
->Workspaces
-><your_workspace>
->Members
- Invite four more new accounts:
MiniBoss
,Manager
,Employee1
,Employee2
- Go to
Settings
->Workspaces
-><your_workspace>
->More features
and try to enableRules
. - Click
Upgrade
when prompted
....
Reviewing these QA steps, I'm getting confused about what needs to happen here to set things up how you'd like. I started trying to reverse-engineer it, but I figure if I'm confused then the QA tester likely will be too.
@dangrous please update the test/QA steps with more detail on how to set this up from scratch.
I updated the steps - let me know if this works! It's actually way less complicated than that - I had copied the repro steps QA had given me but you don't need that much to actually get the issue. Let me know if this works! |
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.
So @dangrous I've updated the test/QA steps with the steps I followed but AFAICT it's still not passing
hrm weird, i tested it right before changing the steps! let me try again shortly |
@roryabraham I just tried it again using those exact steps and am still getting the correct behavior for the optimistic next step... Do you mind trying one more time? The only difference now is a semi-conflict with another PR that's happening - the server response is now "to finish setting up a business bank account", but the "an admin" part of this one is still a necessary change so focus on that. |
bump when you have a moment @roryabraham thanks! |
DM'd you, but I'd like to clarify the precondition steps before running through the tests again |
Mentioned this in the DM but putting it here for the record - preconditions no longer necessary, I have removed them from the test steps |
Reviewer Checklist
Screenshots/VideosAndroid: NativeAndroid: mWeb ChromeiOS: NativeiOS: mWeb SafariMacOS: Desktop |
re-tested and it worked as expected. Sorry it took a while @dangrous |
✋ 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/roryabraham in version: 9.0.43-0 🚀
|
1 similar comment
🚀 Deployed to staging by https://github.com/roryabraham in version: 9.0.43-0 🚀
|
🚀 Deployed to production by https://github.com/jasperhuangg in version: 9.0.43-6 🚀
|
Details
Being fixed on BE as well - https://github.com/Expensify/Web-Expensify/pull/43483
Fixed Issues
$ #47278
PROPOSAL:
Tests
Same as QA - if https://github.com/Expensify/Web-Expensify/pull/43483 has not been merged/deployed yet, please check only the optimistic action!
Offline tests
QA Steps
Manage my team's expenses
during onboarding to create a collect policySettings
->Workspaces
-><your_workspace>
->Members
Employee
Settings
->Workspaces
-><your_workspace>
->More features
and enableWorkflows
.Workflows
and toggleAdd approvals
Waiting for an admin to pay expense(s).
NOT a specific email address.NOTE: the server next step is being changed as we speak so it will not match. Only review the optimistic next step.
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