-
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
Add workspace upgrade flow #43822
Add workspace upgrade flow #43822
Conversation
onyxMethod: Onyx.METHOD.MERGE, | ||
key: `policy_${policyID}`, | ||
value: { | ||
isPendingUpgrade: true, |
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.
let's add these optimistic values and their keys in CONST if they're not there:
type=CORPORATE
maxExpenseAge=CONST.POLICY.DEFAULT_MAX_EXPENSE_AGE=90
maxExpenseAmount=CONST.POLICY.DEFAULT_MAX_EXPENSE_AMOUNT=200000
maxExpenseAmountNoReceipt=CONST.POLICY.DEFAULT_MAX_AMOUNT_NO_RECEIPT=2500
glCodes=true
And then add these values to the optimistic data if instant submit is enabled:
if (isInstantSubmitEnabled(policy)) {
// set autoReporting=false
// set autoReportingFrequency=CONST.POLICY.AUTO_REPORTING_FREQUENCIES.MANUAL
}
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.
Handled.
…ns-fixButtonWidthDisableTaxSorting
@ishpaul777 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] |
I assume this needs a c+ review ? |
Yes. |
Okay thanks for clarifying, i'll get this done, when i start tomorrow. 😄 |
Reviewer Checklist
Screenshots/VideosAndroid: NativeRecord_2024-07-02-01-27-57.mp4Android: mWeb ChromeRecord_2024-07-02-01-37-13.mp4iOS: NativeScreen.Recording.2024-07-01.at.3.05.38.AM.moviOS: mWeb SafariScreen.Recording.2024-07-01.at.3.22.28.AM.movMacOS: Chrome / SafariScreen.Recording.2024-07-01.at.2.49.21.AM.movMacOS: DesktopScreen.Recording.2024-07-01.at.4.04.00.AM.mov |
@allroundexperts There are prettier diffs |
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! just some minor comments
…ns-fixButtonWidthDisableTaxSorting
And to clarify, is Report Fields the only upgrade screen at the moment, or are there others I should/can review as well? Thanks! |
🧪🧪 Use the links below to test this adhoc build on Android, iOS, Desktop, and Web. Happy testing! 🧪🧪
|
@shawnborton I think we should have NetSuite and Intacct ready for review this week, too, but they're not ready yet. |
Also @garrettmknight is that the copy we settled on for report fields? Feels a bit short - I was thinking we could use it as an opportunity to sell the feature a bit. We also don't have any indication of price or anything here, which I could have sworn we did from the mocks. |
@shawnborton Handled all your comments and updated the copy. Please review the following. |
Wonderful, thanks! |
we have conflicts @allroundexperts |
const {translate} = useLocalize(); | ||
|
||
return ( | ||
<View style={styles.p5}> |
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 not noticing this before but we do have a Section
component can we reuse it here @allroundexperts
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.
Can you provide the full name of this component?
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.
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.
Hm... Would this be plug in play? I think we'll need to do some changes within this component to meet our needs.
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.
Hm... Would this be plug in play? I think we'll need to do some changes within this component to meet our needs.
Ideally we should reuse but we def. need to make changes to component to meet our needs here, I dont think this is super important change, current component is readable as it is. Consider this as a optional comment 👍
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!
✋ 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/youssef-lr in version: 9.0.5-0 🚀
|
🚀 Deployed to staging by https://github.com/youssef-lr in version: 9.0.5-2 🚀
|
🚀 Deployed to production by https://github.com/Julesssss in version: 9.0.5-13 🚀
|
🚀 Deployed to production by https://github.com/thienlnam in version: 9.0.6-8 🚀
|
description: `Report fields let you specify header-level details, distinct from tags that pertain to expenses on individual line items. These details can encompass specific project names, business trip information, locations, and more.`, | ||
pricing: { | ||
onlyAvailableOnPlan: 'Report fields are only available on the Control plan, starting at ', | ||
amount: '$9 ', |
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.
@@ -791,6 +791,7 @@ const CONST = { | |||
UPDATE_TIME_ENABLED: 'POLICYCHANGELOG_UPDATE_TIME_ENABLED', | |||
UPDATE_TIME_RATE: 'POLICYCHANGELOG_UPDATE_TIME_RATE', | |||
LEAVE_POLICY: 'POLICYCHANGELOG_LEAVE_POLICY', | |||
CORPORATE_UPGRADE: 'POLICYCHANGELOG_CORPORATE_UPGRADE', |
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.
Coming from #54397. We should translate the system message for POLICYCHANGELOG_CORPORATE_UPGRADE.
...(PolicyUtils.isInstantSubmitEnabled(policy) && { | ||
autoReporting: true, | ||
autoReportingFrequency: CONST.POLICY.AUTO_REPORTING_FREQUENCIES.MANUAL, | ||
}), |
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.
Details
This PR adds ability to upgrade the workspace to corporate plan.
Fixed Issues
$ #43916
Tests
upgrade/report-fields
to the url. (eg settings/workspaces/670843E4FAC26E3A/upgrade/report-fields)Offline tests
N/A
QA Steps
upgrade/report-fields
to the url. (eg settings/workspaces/670843E4FAC26E3A/upgrade/report-fields)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