-
Notifications
You must be signed in to change notification settings - Fork 3.7k
Fix : Reports - Create report button does not show upgrade page when account has no workspace #78995
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
base: main
Are you sure you want to change the base?
Fix : Reports - Create report button does not show upgrade page when account has no workspace #78995
Changes from all commits
be2e732
e7255f1
47c974e
a442f00
6be40c0
d8ec835
df0fb46
0ecce10
db5cf3e
e861d89
2f2628c
9cbf2d2
cce4c9a
e56d95f
5f10d0c
93c9809
e1fe4bc
c8744b8
2dd9072
fc8b21b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -36,7 +36,7 @@ function UpgradeConfirmation({policyName, afterUpgradeAcknowledged, isReporting, | |
| }, [updateSubscriptionLink]); | ||
|
|
||
| const description = useMemo(() => { | ||
| if (isCategorizing ?? isReporting) { | ||
| if (isCategorizing || isReporting) { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ❌ CONSISTENCY-2The Suggested fix: Keep the nullish coalescing operator if the intent was to only check for null/undefined: if (isCategorizing ?? isReporting) {Or if you intended OR logic, use explicit checks: if (isCategorizing || isReporting) {Note: This appears to be a logical change rather than just a style fix - ensure the new behavior is correct. Please rate this suggestion with 👍 or 👎 to help us improve! Reactions are used to monitor reviewer efficiency. |
||
| return <Text style={[styles.textAlignCenter, styles.w100]}>{translate('workspace.upgrade.completed.categorizeMessage')}</Text>; | ||
| } | ||
|
|
||
|
|
@@ -56,7 +56,7 @@ function UpgradeConfirmation({policyName, afterUpgradeAcknowledged, isReporting, | |
| }, [isDistanceRateUpgrade, isCategorizing, isReporting, isTravelUpgrade, policyName, styles.renderHTML, styles.textAlignCenter, styles.w100, translate, subscriptionLink]); | ||
|
|
||
| const heading = useMemo(() => { | ||
| if (isCategorizing ?? isReporting) { | ||
| if (isCategorizing || isReporting) { | ||
| return translate('workspace.upgrade.completed.createdWorkspace'); | ||
| } | ||
| return translate('workspace.upgrade.completed.headline'); | ||
|
|
||
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.
In this upgrade flow you pass
allTransactionsfrom the Onyx TRANSACTION collection intochangeTransactionsReport, but the selected items here come from SearchContext (snapshot search results) and aren’t guaranteed to exist in that Onyx collection.changeTransactionsReportfilters missing entries and returns early when none are found, so the post-upgrade bulk move can become a no-op for search-based selections. Consider building theallTransactionsmap fromselectedTransactions(as the search change-report flow does) or otherwise ensuring the selected IDs are present before calling.Useful? React with 👍 / 👎.