From c8084ffbc9476592e3f7203632e42ba36bed92f6 Mon Sep 17 00:00:00 2001 From: Abdelrahman Khattab Date: Sun, 20 Oct 2024 00:44:43 +0200 Subject: [PATCH 01/40] Hide approve button if report has violations --- src/libs/actions/IOU.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/libs/actions/IOU.ts b/src/libs/actions/IOU.ts index fb8cd014ec7b..047d081fbacb 100644 --- a/src/libs/actions/IOU.ts +++ b/src/libs/actions/IOU.ts @@ -6988,8 +6988,9 @@ function canApproveIOU(iouReport: OnyxTypes.OnyxInputOrEntry, const reportNameValuePairs = ReportUtils.getReportNameValuePairs(iouReport?.reportID); const isArchivedReport = ReportUtils.isArchivedRoom(iouReport, reportNameValuePairs); const unheldTotalIsZero = iouReport && iouReport.unheldTotal === 0; + const hasViolations = ReportUtils.hasViolations(iouReport?.reportID ?? '-1', allTransactionViolations); - return isCurrentUserManager && !isOpenExpenseReport && !isApproved && !iouSettled && !isArchivedReport && !unheldTotalIsZero; + return isCurrentUserManager && !isOpenExpenseReport && !isApproved && !iouSettled && !isArchivedReport && !unheldTotalIsZero && !hasViolations; } function canIOUBePaid( From f63804fda2685824aee85d1a1f9eba15d0bed65e Mon Sep 17 00:00:00 2001 From: Bernhard Owen Josephus Date: Fri, 25 Oct 2024 19:24:52 +0800 Subject: [PATCH 02/40] fix hidden shows briefly when mentioning unknown user --- .../HTMLRenderers/MentionUserRenderer.tsx | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/components/HTMLEngineProvider/HTMLRenderers/MentionUserRenderer.tsx b/src/components/HTMLEngineProvider/HTMLRenderers/MentionUserRenderer.tsx index 36586b09e514..96bdf8e9e1e8 100644 --- a/src/components/HTMLEngineProvider/HTMLRenderers/MentionUserRenderer.tsx +++ b/src/components/HTMLEngineProvider/HTMLRenderers/MentionUserRenderer.tsx @@ -4,9 +4,9 @@ import isEmpty from 'lodash/isEmpty'; import React from 'react'; import {StyleSheet} from 'react-native'; import type {TextStyle} from 'react-native'; +import {useOnyx} from 'react-native-onyx'; import type {CustomRendererProps, TPhrasing, TText} from 'react-native-render-html'; import {TNodeChildrenRenderer} from 'react-native-render-html'; -import {usePersonalDetails} from '@components/OnyxProvider'; import {ShowContextMenuContext, showContextMenuForReport} from '@components/ShowContextMenuContext'; import Text from '@components/Text'; import UserDetailsTooltip from '@components/UserDetailsTooltip'; @@ -20,6 +20,7 @@ import Navigation from '@libs/Navigation/Navigation'; import * as PersonalDetailsUtils from '@libs/PersonalDetailsUtils'; import * as ReportUtils from '@libs/ReportUtils'; import CONST from '@src/CONST'; +import ONYXKEYS from '@src/ONYXKEYS'; import ROUTES from '@src/ROUTES'; import type {Route} from '@src/ROUTES'; import asMutable from '@src/types/utils/asMutable'; @@ -31,7 +32,7 @@ function MentionUserRenderer({style, tnode, TDefaultRenderer, currentUserPersona const styles = useThemeStyles(); const StyleUtils = useStyleUtils(); const htmlAttribAccountID = tnode.attributes.accountid; - const personalDetails = usePersonalDetails() || CONST.EMPTY_OBJECT; + const [personalDetails] = useOnyx(ONYXKEYS.PERSONAL_DETAILS_LIST); const htmlAttributeAccountID = tnode.attributes.accountid; let accountID: number; @@ -56,7 +57,7 @@ function MentionUserRenderer({style, tnode, TDefaultRenderer, currentUserPersona return displayText.split('@').at(0); }; - if (!isEmpty(htmlAttribAccountID)) { + if (!isEmpty(htmlAttribAccountID) && personalDetails?.[htmlAttribAccountID]) { const user = personalDetails[htmlAttribAccountID]; accountID = parseInt(htmlAttribAccountID, 10); mentionDisplayText = LocalePhoneNumber.formatPhoneNumber(user?.login ?? '') || PersonalDetailsUtils.getDisplayNameOrDefault(user); From 3204148992ce2e3927608e2c6359439a17bd313f Mon Sep 17 00:00:00 2001 From: Abdelrahman Khattab <59809993+abzokhattab@users.noreply.github.com> Date: Sat, 2 Nov 2024 12:39:52 +0100 Subject: [PATCH 03/40] Disable the pay button if the report has violaitons --- src/libs/actions/IOU.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/libs/actions/IOU.ts b/src/libs/actions/IOU.ts index 047d081fbacb..f0461d03dd26 100644 --- a/src/libs/actions/IOU.ts +++ b/src/libs/actions/IOU.ts @@ -7042,8 +7042,9 @@ function canIOUBePaid( const {reimbursableSpend} = ReportUtils.getMoneyRequestSpendBreakdown(iouReport); const isAutoReimbursable = policy?.reimbursementChoice === CONST.POLICY.REIMBURSEMENT_CHOICES.REIMBURSEMENT_YES ? false : ReportUtils.canBeAutoReimbursed(iouReport, policy); const shouldBeApproved = canApproveIOU(iouReport, policy); - + const hasViolations = ReportUtils.hasViolations(iouReport?.reportID ?? '-1', allTransactionViolations); const isPayAtEndExpenseReport = ReportUtils.isPayAtEndExpenseReport(iouReport?.reportID, transactions); + return ( isPayer && !isOpenExpenseReport && @@ -7053,8 +7054,10 @@ function canIOUBePaid( !isChatReportArchived && !isAutoReimbursable && !shouldBeApproved && + !hasViolations && !isPayAtEndExpenseReport ); + } function getIOUReportActionToApproveOrPay(chatReport: OnyxEntry, excludedIOUReportID: string): OnyxEntry { From f760f272cdfd83f421fe929553a84c699d23a40a Mon Sep 17 00:00:00 2001 From: Abdelrahman Khattab <59809993+abzokhattab@users.noreply.github.com> Date: Sat, 2 Nov 2024 12:40:51 +0100 Subject: [PATCH 04/40] minor edit --- src/libs/actions/IOU.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/src/libs/actions/IOU.ts b/src/libs/actions/IOU.ts index f0461d03dd26..575e14e36fd3 100644 --- a/src/libs/actions/IOU.ts +++ b/src/libs/actions/IOU.ts @@ -7057,7 +7057,6 @@ function canIOUBePaid( !hasViolations && !isPayAtEndExpenseReport ); - } function getIOUReportActionToApproveOrPay(chatReport: OnyxEntry, excludedIOUReportID: string): OnyxEntry { From 38a56502c4168ba86d0ae449b8aa1fe03b7d69fa Mon Sep 17 00:00:00 2001 From: Abdelrahman Khattab <59809993+abzokhattab@users.noreply.github.com> Date: Sat, 2 Nov 2024 12:45:12 +0100 Subject: [PATCH 05/40] minor edit --- src/libs/actions/IOU.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/src/libs/actions/IOU.ts b/src/libs/actions/IOU.ts index 575e14e36fd3..3a0ca373d3de 100644 --- a/src/libs/actions/IOU.ts +++ b/src/libs/actions/IOU.ts @@ -7043,6 +7043,7 @@ function canIOUBePaid( const isAutoReimbursable = policy?.reimbursementChoice === CONST.POLICY.REIMBURSEMENT_CHOICES.REIMBURSEMENT_YES ? false : ReportUtils.canBeAutoReimbursed(iouReport, policy); const shouldBeApproved = canApproveIOU(iouReport, policy); const hasViolations = ReportUtils.hasViolations(iouReport?.reportID ?? '-1', allTransactionViolations); + const hasViolations = ReportUtils.hasViolations(iouReport?.reportID ?? '-1', allTransactionViolations); const isPayAtEndExpenseReport = ReportUtils.isPayAtEndExpenseReport(iouReport?.reportID, transactions); return ( From 7af0e6a9cebac8ae5c2bc1c53ffd5b4833dc9193 Mon Sep 17 00:00:00 2001 From: Abdelrahman Khattab <59809993+abzokhattab@users.noreply.github.com> Date: Sat, 2 Nov 2024 12:46:17 +0100 Subject: [PATCH 06/40] minor edit --- src/libs/actions/IOU.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/src/libs/actions/IOU.ts b/src/libs/actions/IOU.ts index 3a0ca373d3de..575e14e36fd3 100644 --- a/src/libs/actions/IOU.ts +++ b/src/libs/actions/IOU.ts @@ -7043,7 +7043,6 @@ function canIOUBePaid( const isAutoReimbursable = policy?.reimbursementChoice === CONST.POLICY.REIMBURSEMENT_CHOICES.REIMBURSEMENT_YES ? false : ReportUtils.canBeAutoReimbursed(iouReport, policy); const shouldBeApproved = canApproveIOU(iouReport, policy); const hasViolations = ReportUtils.hasViolations(iouReport?.reportID ?? '-1', allTransactionViolations); - const hasViolations = ReportUtils.hasViolations(iouReport?.reportID ?? '-1', allTransactionViolations); const isPayAtEndExpenseReport = ReportUtils.isPayAtEndExpenseReport(iouReport?.reportID, transactions); return ( From 6c716bb01bf104c444a4597c4cb140d9b99097bd Mon Sep 17 00:00:00 2001 From: Abdelrahman Khattab <59809993+abzokhattab@users.noreply.github.com> Date: Sat, 2 Nov 2024 12:49:51 +0100 Subject: [PATCH 07/40] minor edit --- src/libs/actions/IOU.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libs/actions/IOU.ts b/src/libs/actions/IOU.ts index 575e14e36fd3..f911f17d0103 100644 --- a/src/libs/actions/IOU.ts +++ b/src/libs/actions/IOU.ts @@ -7044,7 +7044,7 @@ function canIOUBePaid( const shouldBeApproved = canApproveIOU(iouReport, policy); const hasViolations = ReportUtils.hasViolations(iouReport?.reportID ?? '-1', allTransactionViolations); const isPayAtEndExpenseReport = ReportUtils.isPayAtEndExpenseReport(iouReport?.reportID, transactions); - + return ( isPayer && !isOpenExpenseReport && From c9bcc679361382b9ba6c30fdd26a14828aa09ad7 Mon Sep 17 00:00:00 2001 From: Bernhard Owen Josephus Date: Tue, 12 Nov 2024 14:28:07 +0800 Subject: [PATCH 08/40] fix go back/forward gesture doesn't work --- web/index.html | 1 - 1 file changed, 1 deletion(-) diff --git a/web/index.html b/web/index.html index c15f79b428a7..aad82434c342 100644 --- a/web/index.html +++ b/web/index.html @@ -44,7 +44,6 @@ } body { overflow: hidden; - overscroll-behavior: none; touch-action: none; } [data-drag-area='true'] { From 6c69ab83991f4f4b610bf14670402ca358e61e59 Mon Sep 17 00:00:00 2001 From: Vit Horacek Date: Wed, 13 Nov 2024 01:08:47 +0100 Subject: [PATCH 09/40] Update the reviewer and author checklist to include unit tests and update the proposal template --- .github/PULL_REQUEST_TEMPLATE.md | 3 ++- contributingGuides/PROPOSAL_TEMPLATE.md | 3 +++ contributingGuides/REVIEWER_CHECKLIST.md | 3 ++- 3 files changed, 7 insertions(+), 2 deletions(-) diff --git a/.github/PULL_REQUEST_TEMPLATE.md b/.github/PULL_REQUEST_TEMPLATE.md index 459a780ca8b4..917ca7fff142 100644 --- a/.github/PULL_REQUEST_TEMPLATE.md +++ b/.github/PULL_REQUEST_TEMPLATE.md @@ -94,7 +94,7 @@ This is a checklist for PR authors. Please make sure to complete all tasks and c - [ ] I followed the guidelines as stated in the [Review Guidelines](https://github.com/Expensify/App/blob/main/contributingGuides/PR_REVIEW_GUIDELINES.md) - [ ] I tested other components that can be impacted by my changes (i.e. if the PR modifies a shared library or component like `Avatar`, I verified the components using `Avatar` are working as expected) - [ ] I verified all code is DRY (the PR doesn't include any logic written more than once, with the exception of tests) -- [ ] I verified any variables that can be defined as constants (ie. in CONST.js or at the top of the file that uses the constant) are defined as such +- [ ] I verified any variables that can be defined as constants (ie. in CONST.ts or at the top of the file that uses the constant) are defined as such - [ ] I verified that if a function's arguments changed that all usages have also been updated correctly - [ ] If any new file was added I verified that: - [ ] The file has a description of what it does and/or why is needed at the top of the file if the code is not self explanatory @@ -109,6 +109,7 @@ This is a checklist for PR authors. Please make sure to complete all tasks and c - [ ] I verified that all the inputs inside a form are aligned with each other. - [ ] I added `Design` label and/or tagged `@Expensify/design` so the design team can review the changes. - [ ] If a new page is added, I verified it's using the `ScrollView` component to make it scrollable when more elements are added to the page. +- [ ] I added unit tests for any new feature or bug fix in this PR to help automatically prevent regressions in this user flow. - [ ] If the `main` branch was merged into this PR after a review, I tested again and verified the outcome was still expected according to the `Test` steps. ### Screenshots/Videos diff --git a/contributingGuides/PROPOSAL_TEMPLATE.md b/contributingGuides/PROPOSAL_TEMPLATE.md index 8c9fa7968fe2..437355448269 100644 --- a/contributingGuides/PROPOSAL_TEMPLATE.md +++ b/contributingGuides/PROPOSAL_TEMPLATE.md @@ -7,6 +7,9 @@ ### What changes do you think we should make in order to solve the problem? +### What specific scenarios should we cover in unit tests to prevent reintroducing this issue in the future? + + ### What alternative solutions did you explore? (Optional) **Reminder:** Please use plain English, be brief and avoid jargon. Feel free to use images, charts or pseudo-code if necessary. Do not post large multi-line diffs or write walls of text. Do not create PRs unless you have been hired for this job. diff --git a/contributingGuides/REVIEWER_CHECKLIST.md b/contributingGuides/REVIEWER_CHECKLIST.md index 5fc14328f3b4..baee49e27156 100644 --- a/contributingGuides/REVIEWER_CHECKLIST.md +++ b/contributingGuides/REVIEWER_CHECKLIST.md @@ -30,7 +30,7 @@ - [ ] I verified that this PR follows the guidelines as stated in the [Review Guidelines](https://github.com/Expensify/App/blob/main/contributingGuides/PR_REVIEW_GUIDELINES.md) - [ ] I verified other components that can be impacted by these changes have been tested, and I retested again (i.e. if the PR modifies a shared library or component like `Avatar`, I verified the components using `Avatar` have been tested & I retested again) - [ ] I verified all code is DRY (the PR doesn't include any logic written more than once, with the exception of tests) -- [ ] I verified any variables that can be defined as constants (ie. in CONST.js or at the top of the file that uses the constant) are defined as such +- [ ] I verified any variables that can be defined as constants (ie. in CONST.ts or at the top of the file that uses the constant) are defined as such - [ ] If a new component is created I verified that: - [ ] A similar component doesn't exist in the codebase - [ ] All props are defined accurately and each prop has a `/** comment above it */` @@ -54,6 +54,7 @@ - [ ] I verified that all the inputs inside a form are aligned with each other. - [ ] I added `Design` label and/or tagged `@Expensify/design` so the design team can review the changes. - [ ] If a new page is added, I verified it's using the `ScrollView` component to make it scrollable when more elements are added to the page. +- [ ] For any bug fix or new feature in this PR, I verified that sufficient unit tests are included to prevent regressions in this flow. - [ ] If the `main` branch was merged into this PR after a review, I tested again and verified the outcome was still expected according to the `Test` steps. - [ ] I have checked off every checkbox in the PR reviewer checklist, including those that don't apply to this PR. From f1a930d0687272601b2cb0e7c74cde6923c096a4 Mon Sep 17 00:00:00 2001 From: Vit Horacek Date: Wed, 13 Nov 2024 01:13:26 +0100 Subject: [PATCH 10/40] Update the template --- contributingGuides/PROPOSAL_TEMPLATE.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/contributingGuides/PROPOSAL_TEMPLATE.md b/contributingGuides/PROPOSAL_TEMPLATE.md index 437355448269..aee4aa5d22e5 100644 --- a/contributingGuides/PROPOSAL_TEMPLATE.md +++ b/contributingGuides/PROPOSAL_TEMPLATE.md @@ -8,7 +8,7 @@ ### What specific scenarios should we cover in unit tests to prevent reintroducing this issue in the future? - + ### What alternative solutions did you explore? (Optional) From 939dd095ac383488fe1ae10af2befcd3acd83c73 Mon Sep 17 00:00:00 2001 From: Anusha Date: Wed, 13 Nov 2024 21:16:18 +0500 Subject: [PATCH 11/40] fix navigation to room description --- src/components/ReportWelcomeText.tsx | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/components/ReportWelcomeText.tsx b/src/components/ReportWelcomeText.tsx index 3c38c9f4c4a3..80046b3ee8dc 100644 --- a/src/components/ReportWelcomeText.tsx +++ b/src/components/ReportWelcomeText.tsx @@ -45,6 +45,7 @@ function ReportWelcomeText({report, policy}: ReportWelcomeTextProps) { const displayNamesWithTooltips = ReportUtils.getDisplayNamesWithTooltips(OptionsListUtils.getPersonalDetailsForAccountIDs(participantAccountIDs, personalDetails), isMultipleParticipant); const welcomeMessage = SidebarUtils.getWelcomeMessage(report, policy); const moneyRequestOptions = ReportUtils.temporary_getMoneyRequestOptions(report, policy, participantAccountIDs); + const canEdit = ReportUtils.canEditReportDescription(report, policy); const filteredOptions = moneyRequestOptions.filter( (item): item is Exclude => item !== CONST.IOU.TYPE.INVOICE, @@ -123,12 +124,13 @@ function ReportWelcomeText({report, policy}: ReportWelcomeTextProps) { (welcomeMessage?.messageHtml ? ( { - if (!canEditPolicyDescription) { + if (!canEdit) { return; } - Navigation.navigate(ROUTES.WORKSPACE_PROFILE_DESCRIPTION.getRoute(policy?.id ?? '-1')); + const activeRoute = Navigation.getReportRHPActiveRoute(); + Navigation.navigate(ROUTES.REPORT_DESCRIPTION.getRoute(report?.reportID ?? '-1', activeRoute)); }} - style={[styles.renderHTML, canEditPolicyDescription ? styles.cursorPointer : styles.cursorText]} + style={[styles.renderHTML, canEdit ? styles.cursorPointer : styles.cursorText]} accessibilityLabel={translate('reportDescriptionPage.roomDescription')} > From 24ca21589f3e305537c23f4d17a1f894f1437d7b Mon Sep 17 00:00:00 2001 From: Anusha Date: Wed, 13 Nov 2024 22:39:28 +0500 Subject: [PATCH 12/40] change variable name --- src/components/ReportWelcomeText.tsx | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/components/ReportWelcomeText.tsx b/src/components/ReportWelcomeText.tsx index 80046b3ee8dc..128b7b9e81a1 100644 --- a/src/components/ReportWelcomeText.tsx +++ b/src/components/ReportWelcomeText.tsx @@ -45,7 +45,7 @@ function ReportWelcomeText({report, policy}: ReportWelcomeTextProps) { const displayNamesWithTooltips = ReportUtils.getDisplayNamesWithTooltips(OptionsListUtils.getPersonalDetailsForAccountIDs(participantAccountIDs, personalDetails), isMultipleParticipant); const welcomeMessage = SidebarUtils.getWelcomeMessage(report, policy); const moneyRequestOptions = ReportUtils.temporary_getMoneyRequestOptions(report, policy, participantAccountIDs); - const canEdit = ReportUtils.canEditReportDescription(report, policy); + const canEditReportDescription = ReportUtils.canEditReportDescription(report, policy); const filteredOptions = moneyRequestOptions.filter( (item): item is Exclude => item !== CONST.IOU.TYPE.INVOICE, @@ -124,13 +124,13 @@ function ReportWelcomeText({report, policy}: ReportWelcomeTextProps) { (welcomeMessage?.messageHtml ? ( { - if (!canEdit) { + if (!canEditReportDescription) { return; } const activeRoute = Navigation.getReportRHPActiveRoute(); Navigation.navigate(ROUTES.REPORT_DESCRIPTION.getRoute(report?.reportID ?? '-1', activeRoute)); }} - style={[styles.renderHTML, canEdit ? styles.cursorPointer : styles.cursorText]} + style={[styles.renderHTML, canEditReportDescription ? styles.cursorPointer : styles.cursorText]} accessibilityLabel={translate('reportDescriptionPage.roomDescription')} > @@ -156,7 +156,7 @@ function ReportWelcomeText({report, policy}: ReportWelcomeTextProps) { { const activeRoute = Navigation.getReportRHPActiveRoute(); - if (ReportUtils.canEditReportDescription(report, policy)) { + if (canEditReportDescription) { Navigation.navigate(ROUTES.REPORT_DESCRIPTION.getRoute(report?.reportID ?? '-1', activeRoute)); return; } From e2fcccf8e6b60adb6ab2c58931bc5ec3c80b8c13 Mon Sep 17 00:00:00 2001 From: Tim Golen Date: Wed, 13 Nov 2024 14:12:57 -0700 Subject: [PATCH 13/40] Add more info on given/when/then format --- tests/README.md | 70 ++++++++++++++++++++----------------------------- 1 file changed, 28 insertions(+), 42 deletions(-) diff --git a/tests/README.md b/tests/README.md index 890697d80719..d119179114f3 100644 --- a/tests/README.md +++ b/tests/README.md @@ -59,54 +59,40 @@ expect(onyxData).toBe(expectedOnyxData); ## Documenting Tests -Tests aren't always clear about what exactly is being tested. To make this a bit easier we recommend adopting the following format for code comments: +Comments are just as criticle in tests as the tests themselves. They provide context behind why the test was written and what the expected behavior is supposed to be which will benefit the future generations of engineers looking at them. Think about it. When was the last time you saw a unit test and wondered why it was written that way and then you didn't want to touch it because you didn't know if changing the behavior of the test was appropriate or not? It was probably pretty recent :D -``` -// Given -... code that sets initial condition +In order to give future engineers the best context for a unit test, follow this guide: + +1. DO add three sections to every test: + - "Given" - This introduces the initial condition of the test + - "When" - Describes what action is being done and the thing that is being tested + - "Then" - Describes what is expected to happen -// When -... code that does something +2. DO explain **WHY** the test is doing what it is doing in every comment. +3. DO NOT explain **WHAT** the code is doing in comments. This information should be self-evident. + +The format looks like this: -// Then -... code that performs the assertion ``` +// BAD +// Given an account +{* code that sets initial condition *} -## Example Test +// When it is closed +{* code that does something *} -```javascript -HttpUtils.xhr = jest.fn(); - -describe('actions/Report', () => { - it('adds an optimistic comment', () => { - // Given an Onyx subscription to a report's `reportActions` - const ACTION_ID = 1; - const REPORT_ID = 1; - let reportActions; - Onyx.connect({ - key: `${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${REPORT_ID}`, - callback: val => reportActions = val, - }); - - // Mock Report_AddComment command so it can succeed - HttpUtils.xhr.mockImplementation(() => Promise.resolve({ - jsonCode: 200, - })); - - // When we add a new action to that report - Report.addComment(REPORT_ID, 'Hello!'); - return waitForBatchedUpdates() - .then(() => { - const action = reportActions[ACTION_ID]; - - // Then the action set in the Onyx callback should match - // the comment we left and it will be in a loading state because - // it's an "optimistic comment" - expect(action.message[0].text).toBe('Hello!'); - expect(action.isPending).toBe(true); - }); - }); -}); +// Then the user is logged out +{* code that performs the assertion *} + +// GOOD +// Given an account of a brand new user +{* code that sets initial condition *} + +// When the account is closed by clicking on the close account button +{* code that does something *} + +// Then the user should be logged out because their account is no longer active +{* code that performs the assertion *} ``` ## When to Write a Test From 258a87a3e74b33f5443d4877bc607c172c6699d3 Mon Sep 17 00:00:00 2001 From: Tim Golen Date: Wed, 13 Nov 2024 14:18:05 -0700 Subject: [PATCH 14/40] Improve some more of the docs --- tests/README.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/README.md b/tests/README.md index d119179114f3..8a97660f09b5 100644 --- a/tests/README.md +++ b/tests/README.md @@ -97,10 +97,11 @@ The format looks like this: ## When to Write a Test -Many of the UI features of our application should go through rigorous testing by you, your PR reviewer, and finally QA before deployment. It's also difficult to maintain UI tests when the UI changes often. Therefore, it's not valuable for us to place every single part of the application UI under test at this time. The manual testing steps should catch most major UI bugs. Therefore, if we are writing any test there should be a **good reason**. +Many of the UI features of our application should go through rigorous testing by you, your PR reviewer, and finally QA before deployment. However, the code is mature enough now that protecting code against regressions is the top priority. **What's a "good reason" to write a test?** +- Any PR that fixes a bug - Anything that is difficult or impossible to run a manual tests on - e.g. a test to verify an outcome after an authentication token expires (which normally takes two hours) - Areas of the code that are changing often, breaking often, and would benefit from the resiliency an automated test would provide From ab621ba09a9f8155202029d18324d7694e0c10df Mon Sep 17 00:00:00 2001 From: nkdengineer Date: Thu, 14 Nov 2024 11:58:40 +0700 Subject: [PATCH 15/40] feat: Require magic code validation for add missing detail --- ...sonalDetailsAndShipExpensifyCardsParams.ts | 1 + src/libs/actions/PersonalDetails.ts | 3 +- .../MissingPersonalDetailsContent.tsx | 42 +++++++++++++++++-- 3 files changed, 41 insertions(+), 5 deletions(-) diff --git a/src/libs/API/parameters/SetPersonalDetailsAndShipExpensifyCardsParams.ts b/src/libs/API/parameters/SetPersonalDetailsAndShipExpensifyCardsParams.ts index 0ab82ba6b755..7f57658f2016 100644 --- a/src/libs/API/parameters/SetPersonalDetailsAndShipExpensifyCardsParams.ts +++ b/src/libs/API/parameters/SetPersonalDetailsAndShipExpensifyCardsParams.ts @@ -9,6 +9,7 @@ type SetPersonalDetailsAndShipExpensifyCardsParams = { addressCountry: string; addressState: string; dob: string; + validateCode: string; }; export default SetPersonalDetailsAndShipExpensifyCardsParams; diff --git a/src/libs/actions/PersonalDetails.ts b/src/libs/actions/PersonalDetails.ts index f759decda812..94a9dc95e846 100644 --- a/src/libs/actions/PersonalDetails.ts +++ b/src/libs/actions/PersonalDetails.ts @@ -465,7 +465,7 @@ function clearAvatarErrors() { }); } -function updatePersonalDetailsAndShipExpensifyCards(values: FormOnyxValues) { +function updatePersonalDetailsAndShipExpensifyCards(values: FormOnyxValues, validateCode: string) { const parameters: SetPersonalDetailsAndShipExpensifyCardsParams = { legalFirstName: values.legalFirstName?.trim() ?? '', legalLastName: values.legalLastName?.trim() ?? '', @@ -477,6 +477,7 @@ function updatePersonalDetailsAndShipExpensifyCards(values: FormOnyxValues = useRef(null); const values = useMemo(() => getSubstepValues(privatePersonalDetails, draftValues), [privatePersonalDetails, draftValues]); @@ -44,9 +52,7 @@ function MissingPersonalDetailsContent({privatePersonalDetails, draftValues}: Mi if (!values) { return; } - PersonalDetails.updatePersonalDetailsAndShipExpensifyCards(values); - FormActions.clearDraftValues(ONYXKEYS.FORMS.PERSONAL_DETAILS_FORM); - Navigation.goBack(); + setIsValidateCodeActionModalVisible(true); }, [values]); const { @@ -75,6 +81,23 @@ function MissingPersonalDetailsContent({privatePersonalDetails, draftValues}: Mi prevScreen(); }; + const handleValidateCodeEntered = useCallback( + (validateCode: string) => { + PersonalDetails.updatePersonalDetailsAndShipExpensifyCards(values, validateCode); + FormActions.clearDraftValues(ONYXKEYS.FORMS.PERSONAL_DETAILS_FORM); + Navigation.goBack(); + }, + [values], + ); + + const sendValidateCode = () => { + if (validateCodeAction?.validateCodeSent) { + return; + } + + requestValidateCodeAction(); + }; + const handleNextScreen = useCallback(() => { if (isEditing) { goToTheLastStep(); @@ -108,6 +131,17 @@ function MissingPersonalDetailsContent({privatePersonalDetails, draftValues}: Mi screenIndex={screenIndex} personalDetailsValues={values} /> + + {}} + onClose={() => setIsValidateCodeActionModalVisible(false)} + isVisible={isValidateCodeActionModalVisible} + title={translate('cardPage.validateCardTitle')} + descriptionPrimary={translate('cardPage.enterMagicCode', {contactMethod: primaryLogin})} + hasMagicCodeBeenSent={!!validateCodeAction?.validateCodeSent} + /> ); } From a6fb2039de9dbb9125b651e594daca64399d84ce Mon Sep 17 00:00:00 2001 From: Vit Horacek Date: Thu, 14 Nov 2024 19:19:37 +0100 Subject: [PATCH 16/40] Address PR feedback --- .github/PULL_REQUEST_TEMPLATE.md | 2 +- contributingGuides/PROPOSAL_TEMPLATE.md | 2 +- contributingGuides/REVIEWER_CHECKLIST.md | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/.github/PULL_REQUEST_TEMPLATE.md b/.github/PULL_REQUEST_TEMPLATE.md index 917ca7fff142..228a605d2c86 100644 --- a/.github/PULL_REQUEST_TEMPLATE.md +++ b/.github/PULL_REQUEST_TEMPLATE.md @@ -109,7 +109,7 @@ This is a checklist for PR authors. Please make sure to complete all tasks and c - [ ] I verified that all the inputs inside a form are aligned with each other. - [ ] I added `Design` label and/or tagged `@Expensify/design` so the design team can review the changes. - [ ] If a new page is added, I verified it's using the `ScrollView` component to make it scrollable when more elements are added to the page. -- [ ] I added unit tests for any new feature or bug fix in this PR to help automatically prevent regressions in this user flow. +- [ ] I added [unit tests](https://github.com/Expensify/App/blob/main/tests/README.md) for any new feature or bug fix in this PR to help automatically prevent regressions in this user flow. - [ ] If the `main` branch was merged into this PR after a review, I tested again and verified the outcome was still expected according to the `Test` steps. ### Screenshots/Videos diff --git a/contributingGuides/PROPOSAL_TEMPLATE.md b/contributingGuides/PROPOSAL_TEMPLATE.md index aee4aa5d22e5..ea908b5b0666 100644 --- a/contributingGuides/PROPOSAL_TEMPLATE.md +++ b/contributingGuides/PROPOSAL_TEMPLATE.md @@ -8,7 +8,7 @@ ### What specific scenarios should we cover in unit tests to prevent reintroducing this issue in the future? - + ### What alternative solutions did you explore? (Optional) diff --git a/contributingGuides/REVIEWER_CHECKLIST.md b/contributingGuides/REVIEWER_CHECKLIST.md index baee49e27156..545c79a95af1 100644 --- a/contributingGuides/REVIEWER_CHECKLIST.md +++ b/contributingGuides/REVIEWER_CHECKLIST.md @@ -54,7 +54,7 @@ - [ ] I verified that all the inputs inside a form are aligned with each other. - [ ] I added `Design` label and/or tagged `@Expensify/design` so the design team can review the changes. - [ ] If a new page is added, I verified it's using the `ScrollView` component to make it scrollable when more elements are added to the page. -- [ ] For any bug fix or new feature in this PR, I verified that sufficient unit tests are included to prevent regressions in this flow. +- [ ] For any bug fix or new feature in this PR, I verified that sufficient [unit tests](https://github.com/Expensify/App/blob/main/tests/README.md) are included to prevent regressions in this flow. - [ ] If the `main` branch was merged into this PR after a review, I tested again and verified the outcome was still expected according to the `Test` steps. - [ ] I have checked off every checkbox in the PR reviewer checklist, including those that don't apply to this PR. From e5baec5f9f91821f464bf6c1ff5523482504194d Mon Sep 17 00:00:00 2001 From: Vit Horacek Date: Thu, 14 Nov 2024 19:27:17 +0100 Subject: [PATCH 17/40] Also mention new features --- tests/README.md | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/README.md b/tests/README.md index 8a97660f09b5..cd6cdefd0b40 100644 --- a/tests/README.md +++ b/tests/README.md @@ -102,6 +102,7 @@ Many of the UI features of our application should go through rigorous testing by **What's a "good reason" to write a test?** - Any PR that fixes a bug +- When introducing a new feature, cover as much logic as possible by unit tests - Anything that is difficult or impossible to run a manual tests on - e.g. a test to verify an outcome after an authentication token expires (which normally takes two hours) - Areas of the code that are changing often, breaking often, and would benefit from the resiliency an automated test would provide From 5ba7e362b448f00eba49e8dfda1d5e6e732e056a Mon Sep 17 00:00:00 2001 From: mkzie2 Date: Mon, 18 Nov 2024 13:32:43 +0700 Subject: [PATCH 18/40] update childStateNum and childStatusNum for parent report action --- src/libs/actions/IOU.ts | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/src/libs/actions/IOU.ts b/src/libs/actions/IOU.ts index 3b0b43accbc6..30ff1ccf7a78 100644 --- a/src/libs/actions/IOU.ts +++ b/src/libs/actions/IOU.ts @@ -7065,6 +7065,19 @@ function unapproveExpenseReport(expenseReport: OnyxEntry) { const optimisticData: OnyxUpdate[] = [optimisticIOUReportData, optimisticReportActionData, optimisticNextStepData]; + if (expenseReport.parentReportID && expenseReport.parentReportActionID) { + optimisticData.push({ + onyxMethod: Onyx.METHOD.MERGE, + key: `${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${expenseReport.parentReportID}`, + value: { + [expenseReport.parentReportActionID]: { + childStateNum: CONST.REPORT.STATE_NUM.SUBMITTED, + childStatusNum: CONST.REPORT.STATUS_NUM.SUBMITTED, + }, + }, + }); + } + const successData: OnyxUpdate[] = [ { onyxMethod: Onyx.METHOD.MERGE, @@ -7276,6 +7289,19 @@ function cancelPayment(expenseReport: OnyxEntry, chatReport: O }, ]; + if (expenseReport.parentReportID && expenseReport.parentReportActionID) { + optimisticData.push({ + onyxMethod: Onyx.METHOD.MERGE, + key: `${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${expenseReport.parentReportID}`, + value: { + [expenseReport.parentReportActionID]: { + childStateNum: stateNum, + childStatusNum: statusNum, + }, + }, + }); + } + optimisticData.push({ onyxMethod: Onyx.METHOD.MERGE, key: `${ONYXKEYS.COLLECTION.NEXT_STEP}${expenseReport.reportID}`, From 110403f296bc692e5f2c404269f611fa8d88a5dc Mon Sep 17 00:00:00 2001 From: Anusha Date: Tue, 19 Nov 2024 03:34:48 +0500 Subject: [PATCH 19/40] fix back navigation --- src/components/ReportWelcomeText.tsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/components/ReportWelcomeText.tsx b/src/components/ReportWelcomeText.tsx index 128b7b9e81a1..1e3ce6119315 100644 --- a/src/components/ReportWelcomeText.tsx +++ b/src/components/ReportWelcomeText.tsx @@ -127,7 +127,7 @@ function ReportWelcomeText({report, policy}: ReportWelcomeTextProps) { if (!canEditReportDescription) { return; } - const activeRoute = Navigation.getReportRHPActiveRoute(); + const activeRoute = Navigation.getActiveRoute(); Navigation.navigate(ROUTES.REPORT_DESCRIPTION.getRoute(report?.reportID ?? '-1', activeRoute)); }} style={[styles.renderHTML, canEditReportDescription ? styles.cursorPointer : styles.cursorText]} @@ -155,7 +155,7 @@ function ReportWelcomeText({report, policy}: ReportWelcomeTextProps) { (welcomeMessage?.messageHtml ? ( { - const activeRoute = Navigation.getReportRHPActiveRoute(); + const activeRoute = Navigation.getActiveRoute(); if (canEditReportDescription) { Navigation.navigate(ROUTES.REPORT_DESCRIPTION.getRoute(report?.reportID ?? '-1', activeRoute)); return; From 91a0d0d4a5e27dccbe5fccca835d11eba9ade529 Mon Sep 17 00:00:00 2001 From: abzokhattab Date: Tue, 19 Nov 2024 10:28:24 +0100 Subject: [PATCH 20/40] adding violations as an argument to the can approve and pay functions --- src/components/MoneyReportHeader.tsx | 12 ++++++++---- src/components/ReportActionItem/ReportPreview.tsx | 6 +++--- src/libs/actions/IOU.ts | 15 ++++++++++----- 3 files changed, 21 insertions(+), 12 deletions(-) diff --git a/src/components/MoneyReportHeader.tsx b/src/components/MoneyReportHeader.tsx index d01b69ed5649..88c947b5ee6f 100644 --- a/src/components/MoneyReportHeader.tsx +++ b/src/components/MoneyReportHeader.tsx @@ -108,7 +108,7 @@ function MoneyReportHeader({policy, report: moneyRequestReport, transactionThrea const allTransactions = useMemo(() => TransactionUtils.getAllReportTransactions(moneyRequestReport?.reportID, transactions), [moneyRequestReport?.reportID, transactions]); const canAllowSettlement = ReportUtils.hasUpdatedTotal(moneyRequestReport, policy); const policyType = policy?.type; - const isDraft = ReportUtils.isOpenExpenseReport(moneyRequestReport); + const isDraft = !ReportUtils.isOpenExpenseReport(moneyRequestReport); const connectedIntegration = PolicyUtils.getConnectedIntegration(policy); const navigateBackToAfterDelete = useRef(); const hasScanningReceipt = ReportUtils.getTransactionsWithReceipts(moneyRequestReport?.reportID).some((t) => TransactionUtils.isReceiptBeingScanned(t)); @@ -120,10 +120,11 @@ function MoneyReportHeader({policy, report: moneyRequestReport, transactionThrea const isPayAtEndExpense = TransactionUtils.isPayAtEndExpense(transaction); const isArchivedReport = ReportUtils.isArchivedRoomWithID(moneyRequestReport?.reportID); const [archiveReason] = useOnyx(`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${moneyRequestReport?.reportID ?? '-1'}`, {selector: ReportUtils.getArchiveReason}); + const [transactionViolations] = useOnyx(ONYXKEYS.COLLECTION.TRANSACTION_VIOLATIONS); const getCanIOUBePaid = useCallback( - (onlyShowPayElsewhere = false) => IOU.canIOUBePaid(moneyRequestReport, chatReport, policy, transaction ? [transaction] : undefined, onlyShowPayElsewhere), - [moneyRequestReport, chatReport, policy, transaction], + (onlyShowPayElsewhere = false) => IOU.canIOUBePaid(moneyRequestReport, chatReport, policy, transactionViolations, transaction ? [transaction] : undefined, onlyShowPayElsewhere), + [moneyRequestReport, chatReport, policy, transaction, transactionViolations], ); const canIOUBePaid = useMemo(() => getCanIOUBePaid(), [getCanIOUBePaid]); @@ -135,7 +136,10 @@ function MoneyReportHeader({policy, report: moneyRequestReport, transactionThrea const shouldShowPayButton = canIOUBePaid || onlyShowPayElsewhere; - const shouldShowApproveButton = useMemo(() => IOU.canApproveIOU(moneyRequestReport, policy) && !hasOnlyPendingTransactions, [moneyRequestReport, policy, hasOnlyPendingTransactions]); + const shouldShowApproveButton = useMemo( + () => IOU.canApproveIOU(moneyRequestReport, policy, transactionViolations) && !hasOnlyPendingTransactions, + [moneyRequestReport, policy, hasOnlyPendingTransactions, transactionViolations], + ); const shouldDisableApproveButton = shouldShowApproveButton && !ReportUtils.isAllowedToApproveExpenseReport(moneyRequestReport); diff --git a/src/components/ReportActionItem/ReportPreview.tsx b/src/components/ReportActionItem/ReportPreview.tsx index 5edeffd4dea4..bbceeb18bdcd 100644 --- a/src/components/ReportActionItem/ReportPreview.tsx +++ b/src/components/ReportActionItem/ReportPreview.tsx @@ -330,14 +330,14 @@ function ReportPreview({ const bankAccountRoute = ReportUtils.getBankAccountRoute(chatReport); const getCanIOUBePaid = useCallback( - (onlyShowPayElsewhere = false) => IOU.canIOUBePaid(iouReport, chatReport, policy, allTransactions, onlyShowPayElsewhere), - [iouReport, chatReport, policy, allTransactions], + (onlyShowPayElsewhere = false) => IOU.canIOUBePaid(iouReport, chatReport, policy, transactionViolations, allTransactions, onlyShowPayElsewhere), + [iouReport, chatReport, policy, allTransactions, transactionViolations], ); const canIOUBePaid = useMemo(() => getCanIOUBePaid(), [getCanIOUBePaid]); const onlyShowPayElsewhere = useMemo(() => !canIOUBePaid && getCanIOUBePaid(true), [canIOUBePaid, getCanIOUBePaid]); const shouldShowPayButton = isPaidAnimationRunning || canIOUBePaid || onlyShowPayElsewhere; - const shouldShowApproveButton = useMemo(() => IOU.canApproveIOU(iouReport, policy), [iouReport, policy]); + const shouldShowApproveButton = useMemo(() => IOU.canApproveIOU(iouReport, policy, transactionViolations), [iouReport, policy, transactionViolations]); const shouldDisableApproveButton = shouldShowApproveButton && !ReportUtils.isAllowedToApproveExpenseReport(iouReport); diff --git a/src/libs/actions/IOU.ts b/src/libs/actions/IOU.ts index ee529f28cc9b..eac4178660b7 100644 --- a/src/libs/actions/IOU.ts +++ b/src/libs/actions/IOU.ts @@ -6741,7 +6741,11 @@ function sendMoneyWithWallet(report: OnyxEntry, amount: number Report.notifyNewAction(params.chatReportID, managerID); } -function canApproveIOU(iouReport: OnyxTypes.OnyxInputOrEntry, policy: OnyxTypes.OnyxInputOrEntry) { +function canApproveIOU( + iouReport: OnyxTypes.OnyxInputOrEntry, + policy: OnyxTypes.OnyxInputOrEntry, + violations: OnyxCollection, +) { // Only expense reports can be approved const isPaidGroupPolicy = policy && PolicyUtils.isPaidGroupPolicy(policy); if (!isPaidGroupPolicy) { @@ -6760,7 +6764,7 @@ function canApproveIOU(iouReport: OnyxTypes.OnyxInputOrEntry, const iouSettled = ReportUtils.isSettled(iouReport?.reportID); const reportNameValuePairs = ReportUtils.getReportNameValuePairs(iouReport?.reportID); const isArchivedReport = ReportUtils.isArchivedRoom(iouReport, reportNameValuePairs); - const hasViolations = ReportUtils.hasViolations(iouReport?.reportID ?? '-1', allTransactionViolations); + const hasViolations = ReportUtils.hasViolations(iouReport?.reportID ?? '-1', violations); let isTransactionBeingScanned = false; const reportTransactions = TransactionUtils.getAllReportTransactions(iouReport?.reportID); for (const transaction of reportTransactions) { @@ -6780,6 +6784,7 @@ function canIOUBePaid( iouReport: OnyxTypes.OnyxInputOrEntry, chatReport: OnyxTypes.OnyxInputOrEntry, policy: OnyxTypes.OnyxInputOrEntry, + violations: OnyxCollection, transactions?: OnyxTypes.Transaction[], onlyShowPayElsewhere = false, ) { @@ -6824,10 +6829,10 @@ function canIOUBePaid( const {reimbursableSpend} = ReportUtils.getMoneyRequestSpendBreakdown(iouReport); const isAutoReimbursable = policy?.reimbursementChoice === CONST.POLICY.REIMBURSEMENT_CHOICES.REIMBURSEMENT_YES ? false : ReportUtils.canBeAutoReimbursed(iouReport, policy); - const shouldBeApproved = canApproveIOU(iouReport, policy); - const hasViolations = ReportUtils.hasViolations(iouReport?.reportID ?? '-1', allTransactionViolations); - const isPayAtEndExpenseReport = ReportUtils.isPayAtEndExpenseReport(iouReport?.reportID, transactions); + const shouldBeApproved = canApproveIOU(iouReport, policy, violations); + const hasViolations = ReportUtils.hasViolations(iouReport?.reportID ?? '-1', violations); + const isPayAtEndExpenseReport = ReportUtils.isPayAtEndExpenseReport(iouReport?.reportID, transactions); return ( isPayer && !isOpenExpenseReport && From 29e5c226a7620faed55bf43103b2df12c60fb3d9 Mon Sep 17 00:00:00 2001 From: abzokhattab Date: Tue, 19 Nov 2024 10:36:51 +0100 Subject: [PATCH 21/40] minor edit --- src/components/MoneyReportHeader.tsx | 2 +- src/libs/actions/IOU.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/components/MoneyReportHeader.tsx b/src/components/MoneyReportHeader.tsx index 88c947b5ee6f..3976538afe0a 100644 --- a/src/components/MoneyReportHeader.tsx +++ b/src/components/MoneyReportHeader.tsx @@ -108,7 +108,7 @@ function MoneyReportHeader({policy, report: moneyRequestReport, transactionThrea const allTransactions = useMemo(() => TransactionUtils.getAllReportTransactions(moneyRequestReport?.reportID, transactions), [moneyRequestReport?.reportID, transactions]); const canAllowSettlement = ReportUtils.hasUpdatedTotal(moneyRequestReport, policy); const policyType = policy?.type; - const isDraft = !ReportUtils.isOpenExpenseReport(moneyRequestReport); + const isDraft = ReportUtils.isOpenExpenseReport(moneyRequestReport); const connectedIntegration = PolicyUtils.getConnectedIntegration(policy); const navigateBackToAfterDelete = useRef(); const hasScanningReceipt = ReportUtils.getTransactionsWithReceipts(moneyRequestReport?.reportID).some((t) => TransactionUtils.isReceiptBeingScanned(t)); diff --git a/src/libs/actions/IOU.ts b/src/libs/actions/IOU.ts index eac4178660b7..7ac367e3b559 100644 --- a/src/libs/actions/IOU.ts +++ b/src/libs/actions/IOU.ts @@ -6853,7 +6853,7 @@ function getIOUReportActionToApproveOrPay(chatReport: OnyxEntry { const iouReport = ReportUtils.getReportOrDraftReport(action.childReportID ?? '-1'); const policy = PolicyUtils.getPolicy(iouReport?.policyID); - const shouldShowSettlementButton = canIOUBePaid(iouReport, chatReport, policy) || canApproveIOU(iouReport, policy); + const shouldShowSettlementButton = canIOUBePaid(iouReport, chatReport, policy, allTransactionViolations) || canApproveIOU(iouReport, policy, allTransactionViolations); return action.childReportID?.toString() !== excludedIOUReportID && action.actionName === CONST.REPORT.ACTIONS.TYPE.REPORT_PREVIEW && shouldShowSettlementButton; }); } From e30f72e98a1768ab7406b8e16c5cafc96c74e9fa Mon Sep 17 00:00:00 2001 From: mkzie2 Date: Tue, 19 Nov 2024 21:29:01 +0700 Subject: [PATCH 22/40] add failureData --- src/libs/actions/IOU.ts | 74 ++++++++++++++++++++++++++--------------- 1 file changed, 48 insertions(+), 26 deletions(-) diff --git a/src/libs/actions/IOU.ts b/src/libs/actions/IOU.ts index 4d487fa14aaa..64c2eb02269b 100644 --- a/src/libs/actions/IOU.ts +++ b/src/libs/actions/IOU.ts @@ -7083,19 +7083,6 @@ function unapproveExpenseReport(expenseReport: OnyxEntry) { const optimisticData: OnyxUpdate[] = [optimisticIOUReportData, optimisticReportActionData, optimisticNextStepData]; - if (expenseReport.parentReportID && expenseReport.parentReportActionID) { - optimisticData.push({ - onyxMethod: Onyx.METHOD.MERGE, - key: `${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${expenseReport.parentReportID}`, - value: { - [expenseReport.parentReportActionID]: { - childStateNum: CONST.REPORT.STATE_NUM.SUBMITTED, - childStatusNum: CONST.REPORT.STATUS_NUM.SUBMITTED, - }, - }, - }); - } - const successData: OnyxUpdate[] = [ { onyxMethod: Onyx.METHOD.MERGE, @@ -7134,6 +7121,30 @@ function unapproveExpenseReport(expenseReport: OnyxEntry) { }, ]; + if (expenseReport.parentReportID && expenseReport.parentReportActionID) { + optimisticData.push({ + onyxMethod: Onyx.METHOD.MERGE, + key: `${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${expenseReport.parentReportID}`, + value: { + [expenseReport.parentReportActionID]: { + childStateNum: CONST.REPORT.STATE_NUM.SUBMITTED, + childStatusNum: CONST.REPORT.STATUS_NUM.SUBMITTED, + }, + }, + }); + + failureData.push({ + onyxMethod: Onyx.METHOD.MERGE, + key: `${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${expenseReport.parentReportID}`, + value: { + [expenseReport.parentReportActionID]: { + childStateNum: expenseReport.stateNum, + childStatusNum: expenseReport.statusNum, + }, + }, + }); + } + const parameters: UnapproveExpenseReportParams = { reportID: expenseReport.reportID, reportActionID: optimisticUnapprovedReportAction.reportActionID, @@ -7307,19 +7318,6 @@ function cancelPayment(expenseReport: OnyxEntry, chatReport: O }, ]; - if (expenseReport.parentReportID && expenseReport.parentReportActionID) { - optimisticData.push({ - onyxMethod: Onyx.METHOD.MERGE, - key: `${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${expenseReport.parentReportID}`, - value: { - [expenseReport.parentReportActionID]: { - childStateNum: stateNum, - childStatusNum: statusNum, - }, - }, - }); - } - optimisticData.push({ onyxMethod: Onyx.METHOD.MERGE, key: `${ONYXKEYS.COLLECTION.NEXT_STEP}${expenseReport.reportID}`, @@ -7357,6 +7355,30 @@ function cancelPayment(expenseReport: OnyxEntry, chatReport: O }, ]; + if (expenseReport.parentReportID && expenseReport.parentReportActionID) { + optimisticData.push({ + onyxMethod: Onyx.METHOD.MERGE, + key: `${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${expenseReport.parentReportID}`, + value: { + [expenseReport.parentReportActionID]: { + childStateNum: stateNum, + childStatusNum: statusNum, + }, + }, + }); + + failureData.push({ + onyxMethod: Onyx.METHOD.MERGE, + key: `${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${expenseReport.parentReportID}`, + value: { + [expenseReport.parentReportActionID]: { + childStateNum: expenseReport.stateNum, + childStatusNum: expenseReport.statusNum, + }, + }, + }); + } + if (chatReport?.reportID) { failureData.push({ onyxMethod: Onyx.METHOD.MERGE, From 8bd9c6cabf7b471e4b67e34276bf8de41e90379c Mon Sep 17 00:00:00 2001 From: Tim Golen Date: Tue, 19 Nov 2024 09:24:19 -0700 Subject: [PATCH 23/40] Update tests/README.md --- tests/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/README.md b/tests/README.md index cd6cdefd0b40..87aa9068013b 100644 --- a/tests/README.md +++ b/tests/README.md @@ -59,7 +59,7 @@ expect(onyxData).toBe(expectedOnyxData); ## Documenting Tests -Comments are just as criticle in tests as the tests themselves. They provide context behind why the test was written and what the expected behavior is supposed to be which will benefit the future generations of engineers looking at them. Think about it. When was the last time you saw a unit test and wondered why it was written that way and then you didn't want to touch it because you didn't know if changing the behavior of the test was appropriate or not? It was probably pretty recent :D +Comments are just as critical in tests as the tests themselves. They provide context behind why the test was written and what the expected behavior is supposed to be which will benefit the future generations of engineers looking at them. Think about it. When was the last time you saw a unit test and wondered why it was written that way and then you didn't want to touch it because you didn't know if changing the behavior of the test was appropriate or not? It was probably pretty recent :D In order to give future engineers the best context for a unit test, follow this guide: From a73f1421afeba523a992457861c676bc70935afe Mon Sep 17 00:00:00 2001 From: Tim Golen Date: Tue, 19 Nov 2024 09:25:55 -0700 Subject: [PATCH 24/40] Be even more clear --- tests/README.md | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/tests/README.md b/tests/README.md index 87aa9068013b..08df0b89794b 100644 --- a/tests/README.md +++ b/tests/README.md @@ -68,8 +68,9 @@ In order to give future engineers the best context for a unit test, follow this - "When" - Describes what action is being done and the thing that is being tested - "Then" - Describes what is expected to happen -2. DO explain **WHY** the test is doing what it is doing in every comment. -3. DO NOT explain **WHAT** the code is doing in comments. This information should be self-evident. +2. DO begin each comment section with the literal words "Given", "When", and "Then", just like the examples below. +3. DO explain **WHY** the test is doing what it is doing in every comment. +4. DO NOT explain **WHAT** the code is doing in comments. This information should be self-evident. The format looks like this: From d53ec6516a08e1e4e3b36b107945750fa25cd9d5 Mon Sep 17 00:00:00 2001 From: abzokhattab Date: Wed, 20 Nov 2024 17:18:23 +0100 Subject: [PATCH 25/40] minor --- src/libs/actions/IOU.ts | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/libs/actions/IOU.ts b/src/libs/actions/IOU.ts index 8856c6b775ae..0a893ad7a2a8 100644 --- a/src/libs/actions/IOU.ts +++ b/src/libs/actions/IOU.ts @@ -1,7 +1,6 @@ -import { format } from 'date-fns'; -import { fastMerge, Str } from 'expensify-common'; -import { InteractionManager } from 'react-native'; - +import {format} from 'date-fns'; +import {fastMerge, Str} from 'expensify-common'; +import {InteractionManager} from 'react-native'; import type {NullishDeep, OnyxCollection, OnyxEntry, OnyxInputValue, OnyxUpdate} from 'react-native-onyx'; import Onyx from 'react-native-onyx'; import type {PartialDeep, SetRequired, ValueOf} from 'type-fest'; From aa2d3c520c9937172d83101a56e722c39eb2a98b Mon Sep 17 00:00:00 2001 From: Jules Rosser Date: Thu, 21 Nov 2024 15:16:55 +0000 Subject: [PATCH 26/40] stop releasing standalone builds on prod --- .github/workflows/deploy.yml | 11 ----------- 1 file changed, 11 deletions(-) diff --git a/.github/workflows/deploy.yml b/.github/workflows/deploy.yml index 1ceb12a30af5..70d5319368c7 100644 --- a/.github/workflows/deploy.yml +++ b/.github/workflows/deploy.yml @@ -136,11 +136,6 @@ jobs: run: gpg --batch --yes --decrypt --passphrase="${{ secrets.LARGE_SECRET_PASSPHRASE }}" --output android-fastlane-json-key.json android-fastlane-json-key.json.gpg working-directory: android/app - - name: Submit Android build for review - run: bundle exec fastlane android upload_google_play_production - env: - VERSION: ${{ steps.getAndroidVersion.outputs.VERSION_CODE }} - - name: Warn deployers if Android production deploy failed if: ${{ failure() }} uses: 8398a7/action-slack@v3 @@ -431,12 +426,6 @@ jobs: APPLE_DEMO_EMAIL: ${{ secrets.APPLE_DEMO_EMAIL }} APPLE_DEMO_PASSWORD: ${{ secrets.APPLE_DEMO_PASSWORD }} - - name: Submit build for App Store review - if: ${{ fromJSON(env.SHOULD_DEPLOY_PRODUCTION) }} - run: bundle exec fastlane ios submit_for_review - env: - VERSION: ${{ steps.getIOSVersion.outputs.IOS_VERSION }} - - name: Upload iOS build to Browser Stack if: ${{ !fromJSON(env.SHOULD_DEPLOY_PRODUCTION) }} run: curl -u "$BROWSERSTACK" -X POST "https://api-cloud.browserstack.com/app-live/upload" -F "file=@/Users/runner/work/App/App/New Expensify.ipa" From 53ff59332bbf2b961846c1b84d8616f151111326 Mon Sep 17 00:00:00 2001 From: Artem Makushov Date: Fri, 22 Nov 2024 10:19:46 +0100 Subject: [PATCH 27/40] update isLoading both for expensify and company cards --- src/pages/workspace/companyCards/WorkspaceCompanyCardsPage.tsx | 2 +- .../workspace/expensifyCard/WorkspaceExpensifyCardPage.tsx | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/pages/workspace/companyCards/WorkspaceCompanyCardsPage.tsx b/src/pages/workspace/companyCards/WorkspaceCompanyCardsPage.tsx index 453be1f58a32..565d10a54324 100644 --- a/src/pages/workspace/companyCards/WorkspaceCompanyCardsPage.tsx +++ b/src/pages/workspace/companyCards/WorkspaceCompanyCardsPage.tsx @@ -47,7 +47,7 @@ function WorkspaceCompanyCardPage({route}: WorkspaceCompanyCardPageProps) { }, [policyID, workspaceAccountID]); const {isOffline} = useNetwork({onReconnect: fetchCompanyCards}); - const isLoading = !isOffline && (!cardFeeds || cardFeeds.isLoading); + const isLoading = !isOffline && !cardFeeds; useFocusEffect(fetchCompanyCards); diff --git a/src/pages/workspace/expensifyCard/WorkspaceExpensifyCardPage.tsx b/src/pages/workspace/expensifyCard/WorkspaceExpensifyCardPage.tsx index 0dd050fba4e7..20cb74a6a2b1 100644 --- a/src/pages/workspace/expensifyCard/WorkspaceExpensifyCardPage.tsx +++ b/src/pages/workspace/expensifyCard/WorkspaceExpensifyCardPage.tsx @@ -36,7 +36,7 @@ function WorkspaceExpensifyCardPage({route}: WorkspaceExpensifyCardPageProps) { useFocusEffect(fetchExpensifyCards); const paymentBankAccountID = cardSettings?.paymentBankAccountID ?? 0; - const isLoading = !isOffline && (!cardSettings || (cardSettings.isLoading && !cardsList)); + const isLoading = !isOffline && !cardSettings; return ( Date: Fri, 22 Nov 2024 14:19:27 +0100 Subject: [PATCH 28/40] revert expensify card loading --- .../workspace/expensifyCard/WorkspaceExpensifyCardPage.tsx | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/pages/workspace/expensifyCard/WorkspaceExpensifyCardPage.tsx b/src/pages/workspace/expensifyCard/WorkspaceExpensifyCardPage.tsx index 20cb74a6a2b1..97cadec3ca1a 100644 --- a/src/pages/workspace/expensifyCard/WorkspaceExpensifyCardPage.tsx +++ b/src/pages/workspace/expensifyCard/WorkspaceExpensifyCardPage.tsx @@ -36,8 +36,7 @@ function WorkspaceExpensifyCardPage({route}: WorkspaceExpensifyCardPageProps) { useFocusEffect(fetchExpensifyCards); const paymentBankAccountID = cardSettings?.paymentBankAccountID ?? 0; - const isLoading = !isOffline && !cardSettings; - + const isLoading = !isOffline && (!cardSettings || (cardSettings.isLoading && !cardsList)); return ( Date: Fri, 22 Nov 2024 14:20:10 +0100 Subject: [PATCH 29/40] get back an empty line --- src/pages/workspace/expensifyCard/WorkspaceExpensifyCardPage.tsx | 1 + 1 file changed, 1 insertion(+) diff --git a/src/pages/workspace/expensifyCard/WorkspaceExpensifyCardPage.tsx b/src/pages/workspace/expensifyCard/WorkspaceExpensifyCardPage.tsx index 97cadec3ca1a..0dd050fba4e7 100644 --- a/src/pages/workspace/expensifyCard/WorkspaceExpensifyCardPage.tsx +++ b/src/pages/workspace/expensifyCard/WorkspaceExpensifyCardPage.tsx @@ -37,6 +37,7 @@ function WorkspaceExpensifyCardPage({route}: WorkspaceExpensifyCardPageProps) { const paymentBankAccountID = cardSettings?.paymentBankAccountID ?? 0; const isLoading = !isOffline && (!cardSettings || (cardSettings.isLoading && !cardsList)); + return ( Date: Fri, 22 Nov 2024 14:54:06 +0100 Subject: [PATCH 30/40] bump react-native-live-markdown to 0.1.187 --- ios/NewExpensify.xcodeproj/project.pbxproj | 4 ++++ ios/Podfile.lock | 12 ++++++------ package-lock.json | 14 +++++++------- package.json | 2 +- 4 files changed, 18 insertions(+), 14 deletions(-) diff --git a/ios/NewExpensify.xcodeproj/project.pbxproj b/ios/NewExpensify.xcodeproj/project.pbxproj index cd38fcaaaf6c..cd2598608a0f 100644 --- a/ios/NewExpensify.xcodeproj/project.pbxproj +++ b/ios/NewExpensify.xcodeproj/project.pbxproj @@ -638,6 +638,7 @@ "${PODS_CONFIGURATION_BUILD_DIR}/PromisesObjC/FBLPromises_Privacy.bundle", "${PODS_CONFIGURATION_BUILD_DIR}/RCT-Folly/RCT-Folly_privacy.bundle", "${PODS_ROOT}/../../node_modules/@expensify/react-native-live-markdown/parser/react-native-live-markdown-parser.js", + "${PODS_CONFIGURATION_BUILD_DIR}/RNSVG/RNSVGFilters.bundle", "${PODS_CONFIGURATION_BUILD_DIR}/React-Core/React-Core_privacy.bundle", "${PODS_CONFIGURATION_BUILD_DIR}/React-cxxreact/React-cxxreact_privacy.bundle", "${PODS_CONFIGURATION_BUILD_DIR}/SDWebImage/SDWebImage.bundle", @@ -658,6 +659,7 @@ "${TARGET_BUILD_DIR}/${UNLOCALIZED_RESOURCES_FOLDER_PATH}/FBLPromises_Privacy.bundle", "${TARGET_BUILD_DIR}/${UNLOCALIZED_RESOURCES_FOLDER_PATH}/RCT-Folly_privacy.bundle", "${TARGET_BUILD_DIR}/${UNLOCALIZED_RESOURCES_FOLDER_PATH}/react-native-live-markdown-parser.js", + "${TARGET_BUILD_DIR}/${UNLOCALIZED_RESOURCES_FOLDER_PATH}/RNSVGFilters.bundle", "${TARGET_BUILD_DIR}/${UNLOCALIZED_RESOURCES_FOLDER_PATH}/React-Core_privacy.bundle", "${TARGET_BUILD_DIR}/${UNLOCALIZED_RESOURCES_FOLDER_PATH}/React-cxxreact_privacy.bundle", "${TARGET_BUILD_DIR}/${UNLOCALIZED_RESOURCES_FOLDER_PATH}/SDWebImage.bundle", @@ -842,6 +844,7 @@ "${PODS_CONFIGURATION_BUILD_DIR}/PromisesObjC/FBLPromises_Privacy.bundle", "${PODS_CONFIGURATION_BUILD_DIR}/RCT-Folly/RCT-Folly_privacy.bundle", "${PODS_ROOT}/../../node_modules/@expensify/react-native-live-markdown/parser/react-native-live-markdown-parser.js", + "${PODS_CONFIGURATION_BUILD_DIR}/RNSVG/RNSVGFilters.bundle", "${PODS_CONFIGURATION_BUILD_DIR}/React-Core/React-Core_privacy.bundle", "${PODS_CONFIGURATION_BUILD_DIR}/React-cxxreact/React-cxxreact_privacy.bundle", "${PODS_CONFIGURATION_BUILD_DIR}/SDWebImage/SDWebImage.bundle", @@ -862,6 +865,7 @@ "${TARGET_BUILD_DIR}/${UNLOCALIZED_RESOURCES_FOLDER_PATH}/FBLPromises_Privacy.bundle", "${TARGET_BUILD_DIR}/${UNLOCALIZED_RESOURCES_FOLDER_PATH}/RCT-Folly_privacy.bundle", "${TARGET_BUILD_DIR}/${UNLOCALIZED_RESOURCES_FOLDER_PATH}/react-native-live-markdown-parser.js", + "${TARGET_BUILD_DIR}/${UNLOCALIZED_RESOURCES_FOLDER_PATH}/RNSVGFilters.bundle", "${TARGET_BUILD_DIR}/${UNLOCALIZED_RESOURCES_FOLDER_PATH}/React-Core_privacy.bundle", "${TARGET_BUILD_DIR}/${UNLOCALIZED_RESOURCES_FOLDER_PATH}/React-cxxreact_privacy.bundle", "${TARGET_BUILD_DIR}/${UNLOCALIZED_RESOURCES_FOLDER_PATH}/SDWebImage.bundle", diff --git a/ios/Podfile.lock b/ios/Podfile.lock index 5ea5b19896e4..21633b432c12 100644 --- a/ios/Podfile.lock +++ b/ios/Podfile.lock @@ -1722,7 +1722,7 @@ PODS: - ReactCommon/turbomodule/bridging - ReactCommon/turbomodule/core - Yoga - - react-native-keyboard-controller (1.14.1): + - react-native-keyboard-controller (1.14.4): - DoubleConversion - glog - hermes-engine @@ -2391,7 +2391,7 @@ PODS: - RNGoogleSignin (10.0.1): - GoogleSignIn (~> 7.0) - React-Core - - RNLiveMarkdown (0.1.183): + - RNLiveMarkdown (0.1.187): - DoubleConversion - glog - hermes-engine @@ -2411,9 +2411,9 @@ PODS: - ReactCodegen - ReactCommon/turbomodule/bridging - ReactCommon/turbomodule/core - - RNLiveMarkdown/newarch (= 0.1.183) + - RNLiveMarkdown/newarch (= 0.1.187) - Yoga - - RNLiveMarkdown/newarch (0.1.183): + - RNLiveMarkdown/newarch (0.1.187): - DoubleConversion - glog - hermes-engine @@ -3236,7 +3236,7 @@ SPEC CHECKSUMS: react-native-geolocation: b9bd12beaf0ebca61a01514517ca8455bd26fa06 react-native-image-picker: f8a13ff106bcc7eb00c71ce11fdc36aac2a44440 react-native-key-command: aae312752fcdfaa2240be9a015fc41ce54087546 - react-native-keyboard-controller: 902c07f41a415b632583b384427a71770a8b02a3 + react-native-keyboard-controller: 97bb7b48fa427c7455afdc8870c2978efd9bfa3a react-native-launch-arguments: 5f41e0abf88a15e3c5309b8875d6fd5ac43df49d react-native-netinfo: fb5112b1fa754975485884ae85a3fb6a684f49d5 react-native-pager-view: c64a744211a46202619a77509f802765d1659dba @@ -3286,7 +3286,7 @@ SPEC CHECKSUMS: RNFS: 4ac0f0ea233904cb798630b3c077808c06931688 RNGestureHandler: 8781e2529230a1bc3ea8d75e5c3cd071b6c6aed7 RNGoogleSignin: ccaa4a81582cf713eea562c5dd9dc1961a715fd0 - RNLiveMarkdown: fa9c6451960d09209bb5698745a0a66330ec53cc + RNLiveMarkdown: 8338447b39fcd86596c74b9e0e9509e365a2dd3b RNLocalize: d4b8af4e442d4bcca54e68fc687a2129b4d71a81 rnmapbox-maps: 460d6ff97ae49c7d5708c3212c6521697c36a0c4 RNPermissions: 0b1429b55af59d1d08b75a8be2459f65a8ac3f28 diff --git a/package-lock.json b/package-lock.json index 90868fbb3a55..bd9fd1e8060d 100644 --- a/package-lock.json +++ b/package-lock.json @@ -11,7 +11,7 @@ "license": "MIT", "dependencies": { "@dotlottie/react-player": "^1.6.3", - "@expensify/react-native-live-markdown": "0.1.183", + "@expensify/react-native-live-markdown": "0.1.187", "@expo/metro-runtime": "~3.2.3", "@firebase/app": "^0.10.10", "@firebase/performance": "^0.6.8", @@ -3632,14 +3632,14 @@ } }, "node_modules/@expensify/react-native-live-markdown": { - "version": "0.1.183", - "resolved": "https://registry.npmjs.org/@expensify/react-native-live-markdown/-/react-native-live-markdown-0.1.183.tgz", - "integrity": "sha512-egxknos7ghe4M5Z2rK7DvphcaxQBdxyppu5N2tdCVc/3oPO2ZtBNjDjtksqywC12wPtIYgHSgxrzvLEfbh5skw==", + "version": "0.1.187", + "resolved": "https://registry.npmjs.org/@expensify/react-native-live-markdown/-/react-native-live-markdown-0.1.187.tgz", + "integrity": "sha512-bw+dfhRN31u2xfG8LCI3e28g5EG/BfkyX1EqjPBRQlDZo4fZsdA61UFW6P8Y4rHlqspjYXJ0vk4ctECRWYl4Yg==", "license": "MIT", "workspaces": [ - "parser", - "example", - "WebExample" + "./parser", + "./example", + "./WebExample" ], "engines": { "node": ">= 18.0.0" diff --git a/package.json b/package.json index 5be8ade75636..f9050c1ba5c8 100644 --- a/package.json +++ b/package.json @@ -68,7 +68,7 @@ }, "dependencies": { "@dotlottie/react-player": "^1.6.3", - "@expensify/react-native-live-markdown": "0.1.183", + "@expensify/react-native-live-markdown": "0.1.187", "@expo/metro-runtime": "~3.2.3", "@firebase/app": "^0.10.10", "@firebase/performance": "^0.6.8", From 649d1326aa0d27d253efab69aaa5fdd1d36fc0f4 Mon Sep 17 00:00:00 2001 From: Tomasz Misiukiewicz Date: Fri, 22 Nov 2024 15:56:01 +0100 Subject: [PATCH 31/40] remove isHidden property from report --- src/libs/DebugUtils.ts | 3 --- src/libs/ReportUtils.ts | 4 ---- src/libs/actions/Task.ts | 18 ++++++++++++------ src/types/onyx/Report.ts | 3 --- src/types/utils/whitelistedReportKeys.ts | 1 - 5 files changed, 12 insertions(+), 17 deletions(-) diff --git a/src/libs/DebugUtils.ts b/src/libs/DebugUtils.ts index 671fb03f268b..740feab4569f 100644 --- a/src/libs/DebugUtils.ts +++ b/src/libs/DebugUtils.ts @@ -480,8 +480,6 @@ function validateReportDraftProperty(key: keyof Report, value: string) { case 'isOptimisticReport': case 'isWaitingOnBankAccount': case 'isCancelledIOU': - case 'isHidden': - return validateBoolean(value); case 'lastReadSequenceNumber': case 'managerID': case 'lastActorAccountID': @@ -621,7 +619,6 @@ function validateReportDraftProperty(key: keyof Report, value: string) { iouReportID: CONST.RED_BRICK_ROAD_PENDING_ACTION, preexistingReportID: CONST.RED_BRICK_ROAD_PENDING_ACTION, nonReimbursableTotal: CONST.RED_BRICK_ROAD_PENDING_ACTION, - isHidden: CONST.RED_BRICK_ROAD_PENDING_ACTION, pendingChatMembers: CONST.RED_BRICK_ROAD_PENDING_ACTION, fieldList: CONST.RED_BRICK_ROAD_PENDING_ACTION, permissions: CONST.RED_BRICK_ROAD_PENDING_ACTION, diff --git a/src/libs/ReportUtils.ts b/src/libs/ReportUtils.ts index 40749c525a38..db4e8c2faa6d 100644 --- a/src/libs/ReportUtils.ts +++ b/src/libs/ReportUtils.ts @@ -329,7 +329,6 @@ type OptimisticChatReport = Pick< | 'writeCapability' | 'avatarUrl' | 'invoiceReceiver' - | 'isHidden' > & { isOptimisticReport: true; }; @@ -6517,8 +6516,6 @@ function reasonForReportToBeInOptionList({ !report?.reportID || !report?.type || report?.reportName === undefined || - // eslint-disable-next-line @typescript-eslint/prefer-nullish-coalescing - report?.isHidden || (!report?.participants && // We omit sending back participants for chat rooms when searching for reports since they aren't needed to display the results and can get very large. // So we allow showing rooms with no participants–in any other circumstances we should never have these reports with no participants in Onyx. @@ -7399,7 +7396,6 @@ function getTaskAssigneeChatOnyxData( pendingFields: { createChat: CONST.RED_BRICK_ROAD_PENDING_ACTION.ADD, }, - isHidden: false, }, }, { diff --git a/src/libs/actions/Task.ts b/src/libs/actions/Task.ts index aec6c4bd9d30..ea87f5dd5cc6 100644 --- a/src/libs/actions/Task.ts +++ b/src/libs/actions/Task.ts @@ -776,13 +776,19 @@ function setAssigneeChatReport(chatReport: OnyxTypes.Report) { } function setNewOptimisticAssignee(assigneeLogin: string, assigneeAccountID: number) { - const report: ReportUtils.OptimisticChatReport = ReportUtils.buildOptimisticChatReport([assigneeAccountID, currentUserAccountID]); + const report: ReportUtils.OptimisticChatReport = ReportUtils.buildOptimisticChatReport( + [assigneeAccountID, currentUserAccountID], + '', + undefined, + CONST.POLICY.OWNER_EMAIL_FAKE, + CONST.POLICY.OWNER_ACCOUNT_ID_FAKE, + false, + '', + undefined, + undefined, + CONST.REPORT.NOTIFICATION_PREFERENCE.HIDDEN, + ); - // When assigning a task to a new user, by default we share the task in their DM - // However, the DM doesn't exist yet - and will be created optimistically once the task is created - // We don't want to show the new DM yet, because if you select an assignee and then change the assignee, the previous DM will still be shown - // So here, we create it optimistically to share it with the assignee, but we have to hide it until the task is created - report.isHidden = true; Onyx.set(`${ONYXKEYS.COLLECTION.REPORT}${report.reportID}`, report); const optimisticPersonalDetailsListAction: OnyxTypes.PersonalDetails = { diff --git a/src/types/onyx/Report.ts b/src/types/onyx/Report.ts index 0fec22cd9e34..42be2a8c3613 100644 --- a/src/types/onyx/Report.ts +++ b/src/types/onyx/Report.ts @@ -211,9 +211,6 @@ type Report = OnyxCommon.OnyxValueWithOfflineFeedback< /** If the report contains nonreimbursable expenses, send the nonreimbursable total */ nonReimbursableTotal?: number; - /** Whether the report is hidden from options list */ - isHidden?: boolean; - /** Collection of participant private notes, indexed by their accountID */ privateNotes?: Record; diff --git a/src/types/utils/whitelistedReportKeys.ts b/src/types/utils/whitelistedReportKeys.ts index 32aa0797d0f8..d556a51cd3de 100644 --- a/src/types/utils/whitelistedReportKeys.ts +++ b/src/types/utils/whitelistedReportKeys.ts @@ -54,7 +54,6 @@ type WhitelistedReport = OnyxCommon.OnyxValueWithOfflineFeedback< iouReportID: unknown; preexistingReportID: unknown; nonReimbursableTotal: unknown; - isHidden: unknown; privateNotes: unknown; pendingChatMembers: unknown; fieldList: unknown; From cf2f2eda543eabd32f6ec2c6bea2b841d70595ab Mon Sep 17 00:00:00 2001 From: Jules Rosser Date: Fri, 22 Nov 2024 16:20:00 +0000 Subject: [PATCH 32/40] remove submitAndroid step --- .github/workflows/deploy.yml | 40 ------------------------------------ 1 file changed, 40 deletions(-) diff --git a/.github/workflows/deploy.yml b/.github/workflows/deploy.yml index 70d5319368c7..bcc7eea6b940 100644 --- a/.github/workflows/deploy.yml +++ b/.github/workflows/deploy.yml @@ -114,46 +114,6 @@ jobs: env: BROWSERSTACK: ${{ secrets.BROWSERSTACK }} - submitAndroid: - name: Submit Android app for production review - needs: prep - if: ${{ github.ref == 'refs/heads/production' }} - runs-on: ubuntu-latest - steps: - - name: Checkout - uses: actions/checkout@v4 - - - name: Setup Ruby - uses: ruby/setup-ruby@v1.190.0 - with: - bundler-cache: true - - - name: Get Android native version - id: getAndroidVersion - run: echo "VERSION_CODE=$(grep -o 'versionCode\s\+[0-9]\+' android/app/build.gradle | awk '{ print $2 }')" >> "$GITHUB_OUTPUT" - - - name: Decrypt json w/ Google Play credentials - run: gpg --batch --yes --decrypt --passphrase="${{ secrets.LARGE_SECRET_PASSPHRASE }}" --output android-fastlane-json-key.json android-fastlane-json-key.json.gpg - working-directory: android/app - - - name: Warn deployers if Android production deploy failed - if: ${{ failure() }} - uses: 8398a7/action-slack@v3 - with: - status: custom - custom_payload: | - { - channel: '#deployer', - attachments: [{ - color: "#DB4545", - pretext: ``, - text: `💥 Android production deploy failed. Please manually submit ${{ needs.prep.outputs.APP_VERSION }} in the . 💥`, - }] - } - env: - GITHUB_TOKEN: ${{ github.token }} - SLACK_WEBHOOK_URL: ${{ secrets.SLACK_WEBHOOK }} - android_hybrid: name: Build and deploy Android HybridApp needs: prep From c7c9f5e706b59c6deb24a449c899a850a79af128 Mon Sep 17 00:00:00 2001 From: Shahidullah Muffakir Date: Fri, 22 Nov 2024 22:01:44 +0530 Subject: [PATCH 33/40] Don't show 'Make default payment method' button if it's the only payment method or if it's already the default --- .../settings/Wallet/WalletPage/WalletPage.tsx | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/src/pages/settings/Wallet/WalletPage/WalletPage.tsx b/src/pages/settings/Wallet/WalletPage/WalletPage.tsx index 7b9366370349..7da13a6d1de9 100644 --- a/src/pages/settings/Wallet/WalletPage/WalletPage.tsx +++ b/src/pages/settings/Wallet/WalletPage/WalletPage.tsx @@ -313,9 +313,22 @@ function WalletPage({shouldListenForResize = false}: WalletPageProps) { } } }, [hideDefaultDeleteMenu, paymentMethod.methodID, paymentMethod.selectedPaymentMethodType, bankAccountList, fundList, shouldShowDefaultDeleteMenu]); + // Don't show "Make default payment method" button if it's the only payment method or if it's already the default + const isCurrentPaymentMethodDefault = () => { + const hasMultiplePaymentMethods = PaymentUtils.formatPaymentMethods(bankAccountList ?? {}, fundList ?? {}, styles).length > 1; + if (hasMultiplePaymentMethods) { + if (paymentMethod.formattedSelectedPaymentMethod.type === CONST.PAYMENT_METHODS.PERSONAL_BANK_ACCOUNT) { + return paymentMethod.selectedPaymentMethod.bankAccountID === userWallet?.walletLinkedAccountID; + } + if (paymentMethod.formattedSelectedPaymentMethod.type === CONST.PAYMENT_METHODS.DEBIT_CARD) { + return paymentMethod.selectedPaymentMethod.fundID === userWallet?.walletLinkedAccountID; + } + } + return true; + }; const shouldShowMakeDefaultButton = - !paymentMethod.isSelectedPaymentMethodDefault && + !isCurrentPaymentMethodDefault() && !(paymentMethod.formattedSelectedPaymentMethod.type === CONST.PAYMENT_METHODS.PERSONAL_BANK_ACCOUNT && paymentMethod.selectedPaymentMethod.type === CONST.BANK_ACCOUNT.TYPE.BUSINESS); // Determines whether or not the modal popup is mounted from the bottom of the screen instead of the side mount on Web or Desktop screens From 33aae9351d9f924d2af2e0569c42d54667ccf6d1 Mon Sep 17 00:00:00 2001 From: FitseTLT Date: Fri, 22 Nov 2024 19:43:05 +0300 Subject: [PATCH 34/40] updated gb zip code regex --- src/CONST.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/CONST.ts b/src/CONST.ts index 496b7d8b28ec..9b7cd9290aa9 100755 --- a/src/CONST.ts +++ b/src/CONST.ts @@ -3809,8 +3809,8 @@ const CONST = { }, GA: {}, GB: { - regex: /^[A-Z]{1,2}[0-9R][0-9A-Z]?\s*[0-9][A-Z-CIKMOV]{2}$/, - samples: 'LA102UX, BL2F8FX, BD1S9LU, WR4G 6LH', + regex: /^[A-Z]{1,2}[0-9R][0-9A-Z]?\s*([0-9][ABD-HJLNP-UW-Z]{2})?$/, + samples: 'LA102UX, BL2F8FX, BD1S9LU, WR4G 6LH, W1U', }, GD: {}, GE: { From 140bbaaa116bd7881d0ce1408a235732d3b24320 Mon Sep 17 00:00:00 2001 From: abzokhattab Date: Sat, 23 Nov 2024 04:40:31 +0100 Subject: [PATCH 35/40] minor --- src/libs/actions/IOU.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libs/actions/IOU.ts b/src/libs/actions/IOU.ts index 35ef2de437e0..21b074092447 100644 --- a/src/libs/actions/IOU.ts +++ b/src/libs/actions/IOU.ts @@ -6874,8 +6874,8 @@ function canIOUBePaid( const {reimbursableSpend} = ReportUtils.getMoneyRequestSpendBreakdown(iouReport); const isAutoReimbursable = policy?.reimbursementChoice === CONST.POLICY.REIMBURSEMENT_CHOICES.REIMBURSEMENT_YES ? false : ReportUtils.canBeAutoReimbursed(iouReport, policy); - const shouldBeApproved = canApproveIOU(iouReport, policy, violations); const allViolations = violations ?? allTransactionViolations; + const shouldBeApproved = canApproveIOU(iouReport, policy, allViolations); const hasViolations = ReportUtils.hasViolations(iouReport?.reportID ?? '-1', allViolations); const isPayAtEndExpenseReport = ReportUtils.isPayAtEndExpenseReport(iouReport?.reportID, transactions); From 6a9a7697b0ebe763c5adf0eefe5192a5c5293ef0 Mon Sep 17 00:00:00 2001 From: Tomasz Misiukiewicz Date: Mon, 25 Nov 2024 08:29:53 +0100 Subject: [PATCH 36/40] fix tests --- src/libs/DebugUtils.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/src/libs/DebugUtils.ts b/src/libs/DebugUtils.ts index 740feab4569f..aa87f28bef4b 100644 --- a/src/libs/DebugUtils.ts +++ b/src/libs/DebugUtils.ts @@ -480,6 +480,7 @@ function validateReportDraftProperty(key: keyof Report, value: string) { case 'isOptimisticReport': case 'isWaitingOnBankAccount': case 'isCancelledIOU': + return validateBoolean(value); case 'lastReadSequenceNumber': case 'managerID': case 'lastActorAccountID': From efd1e4fd355643ee4d11ae80dbf58144ef28aff2 Mon Sep 17 00:00:00 2001 From: Jules Rosser Date: Mon, 25 Nov 2024 11:36:13 +0000 Subject: [PATCH 37/40] remove Android prod release as deployment success dependency --- .github/workflows/deploy.yml | 30 +++++++++--------------------- 1 file changed, 9 insertions(+), 21 deletions(-) diff --git a/.github/workflows/deploy.yml b/.github/workflows/deploy.yml index bcc7eea6b940..2cacdf557560 100644 --- a/.github/workflows/deploy.yml +++ b/.github/workflows/deploy.yml @@ -679,7 +679,7 @@ jobs: name: Post a Slack message when any platform fails to build or deploy runs-on: ubuntu-latest if: ${{ failure() }} - needs: [buildAndroid, uploadAndroid, submitAndroid, android_hybrid, desktop, iOS, iOS_hybrid, web] + needs: [buildAndroid, uploadAndroid, android_hybrid, desktop, iOS, iOS_hybrid, web] steps: - name: Checkout uses: actions/checkout@v4 @@ -694,21 +694,15 @@ jobs: outputs: IS_AT_LEAST_ONE_PLATFORM_DEPLOYED: ${{ steps.checkDeploymentSuccessOnAtLeastOnePlatform.outputs.IS_AT_LEAST_ONE_PLATFORM_DEPLOYED }} IS_ALL_PLATFORMS_DEPLOYED: ${{ steps.checkDeploymentSuccessOnAllPlatforms.outputs.IS_ALL_PLATFORMS_DEPLOYED }} - needs: [buildAndroid, uploadAndroid, submitAndroid, android_hybrid, desktop, iOS, iOS_hybrid, web] + needs: [buildAndroid, uploadAndroid, android_hybrid, desktop, iOS, iOS_hybrid, web] if: ${{ always() }} steps: - name: Check deployment success on at least one platform id: checkDeploymentSuccessOnAtLeastOnePlatform run: | isAtLeastOnePlatformDeployed="false" - if [ ${{ github.ref }} == 'refs/heads/production' ]; then - if [ "${{ needs.submitAndroid.result }}" == "success" ]; then - isAtLeastOnePlatformDeployed="true" - fi - else - if [ "${{ needs.uploadAndroid.result }}" == "success" ]; then - isAtLeastOnePlatformDeployed="true" - fi + if [ "${{ needs.uploadAndroid.result }}" == "success" ]; then + isAtLeastOnePlatformDeployed="true" fi if [ "${{ needs.iOS.result }}" == "success" ] || \ @@ -733,14 +727,8 @@ jobs: isAllPlatformsDeployed="true" fi - if [ ${{ github.ref }} == 'refs/heads/production' ]; then - if [ "${{ needs.submitAndroid.result }}" != "success" ]; then - isAllPlatformsDeployed="false" - fi - else - if [ "${{ needs.uploadAndroid.result }}" != "success" ]; then - isAllPlatformsDeployed="false" - fi + if [ "${{ needs.uploadAndroid.result }}" != "success" ]; then + isAllPlatformsDeployed="false" fi echo "IS_ALL_PLATFORMS_DEPLOYED=$isAllPlatformsDeployed" >> "$GITHUB_OUTPUT" @@ -888,7 +876,7 @@ jobs: name: Post a Slack message when all platforms deploy successfully runs-on: ubuntu-latest if: ${{ always() && fromJSON(needs.checkDeploymentSuccess.outputs.IS_ALL_PLATFORMS_DEPLOYED) }} - needs: [prep, buildAndroid, uploadAndroid, submitAndroid, android_hybrid, desktop, iOS, iOS_hybrid, web, checkDeploymentSuccess, createPrerelease, finalizeRelease] + needs: [prep, buildAndroid, uploadAndroid, android_hybrid, desktop, iOS, iOS_hybrid, web, checkDeploymentSuccess, createPrerelease, finalizeRelease] steps: - name: 'Announces the deploy in the #announce Slack room' uses: 8398a7/action-slack@v3 @@ -942,11 +930,11 @@ jobs: postGithubComments: uses: ./.github/workflows/postDeployComments.yml if: ${{ always() && fromJSON(needs.checkDeploymentSuccess.outputs.IS_AT_LEAST_ONE_PLATFORM_DEPLOYED) }} - needs: [prep, buildAndroid, uploadAndroid, submitAndroid, android_hybrid, desktop, iOS, iOS_hybrid, web, checkDeploymentSuccess, createPrerelease, finalizeRelease] + needs: [prep, buildAndroid, uploadAndroid, android_hybrid, desktop, iOS, iOS_hybrid, web, checkDeploymentSuccess, createPrerelease, finalizeRelease] with: version: ${{ needs.prep.outputs.APP_VERSION }} env: ${{ github.ref == 'refs/heads/production' && 'production' || 'staging' }} - android: ${{ github.ref == 'refs/heads/production' && needs.submitAndroid.result || needs.uploadAndroid.result }} + android: ${{ github.ref == 'refs/heads/production' && needs.uploadAndroid.result }} android_hybrid: ${{ needs.android_hybrid.result }} ios: ${{ needs.iOS.result }} ios_hybrid: ${{ needs.iOS_hybrid.result }} From ef96172fe3174834de23a6fc902187a50825d579 Mon Sep 17 00:00:00 2001 From: Artem Makushov Date: Tue, 26 Nov 2024 00:03:23 +0100 Subject: [PATCH 38/40] update loading --- src/pages/workspace/companyCards/WorkspaceCompanyCardsPage.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/pages/workspace/companyCards/WorkspaceCompanyCardsPage.tsx b/src/pages/workspace/companyCards/WorkspaceCompanyCardsPage.tsx index 565d10a54324..1b26e4950ef4 100644 --- a/src/pages/workspace/companyCards/WorkspaceCompanyCardsPage.tsx +++ b/src/pages/workspace/companyCards/WorkspaceCompanyCardsPage.tsx @@ -47,7 +47,7 @@ function WorkspaceCompanyCardPage({route}: WorkspaceCompanyCardPageProps) { }, [policyID, workspaceAccountID]); const {isOffline} = useNetwork({onReconnect: fetchCompanyCards}); - const isLoading = !isOffline && !cardFeeds; + const isLoading = !isOffline && (!cardFeeds || (cardFeeds.isLoading && !cardsList)); useFocusEffect(fetchCompanyCards); From ba08044df1de0bd0ef1ee50fb5ccfb484f1d74cc Mon Sep 17 00:00:00 2001 From: OSBotify Date: Tue, 26 Nov 2024 00:07:02 +0000 Subject: [PATCH 39/40] Update version to 9.0.66-7 --- android/app/build.gradle | 4 ++-- ios/NewExpensify/Info.plist | 2 +- ios/NewExpensifyTests/Info.plist | 2 +- ios/NotificationServiceExtension/Info.plist | 2 +- package-lock.json | 4 ++-- package.json | 2 +- 6 files changed, 8 insertions(+), 8 deletions(-) diff --git a/android/app/build.gradle b/android/app/build.gradle index 5d10e3d6f6f8..672776f6ecb4 100644 --- a/android/app/build.gradle +++ b/android/app/build.gradle @@ -110,8 +110,8 @@ android { minSdkVersion rootProject.ext.minSdkVersion targetSdkVersion rootProject.ext.targetSdkVersion multiDexEnabled rootProject.ext.multiDexEnabled - versionCode 1009006606 - versionName "9.0.66-6" + versionCode 1009006607 + versionName "9.0.66-7" // Supported language variants must be declared here to avoid from being removed during the compilation. // This also helps us to not include unnecessary language variants in the APK. resConfigs "en", "es" diff --git a/ios/NewExpensify/Info.plist b/ios/NewExpensify/Info.plist index 2cd9c81c19ca..45fe3eb36805 100644 --- a/ios/NewExpensify/Info.plist +++ b/ios/NewExpensify/Info.plist @@ -40,7 +40,7 @@ CFBundleVersion - 9.0.66.6 + 9.0.66.7 FullStory OrgId diff --git a/ios/NewExpensifyTests/Info.plist b/ios/NewExpensifyTests/Info.plist index 57ba616450b6..05f70824981c 100644 --- a/ios/NewExpensifyTests/Info.plist +++ b/ios/NewExpensifyTests/Info.plist @@ -19,6 +19,6 @@ CFBundleSignature ???? CFBundleVersion - 9.0.66.6 + 9.0.66.7 diff --git a/ios/NotificationServiceExtension/Info.plist b/ios/NotificationServiceExtension/Info.plist index 27a481ab98ef..eb799cfd6323 100644 --- a/ios/NotificationServiceExtension/Info.plist +++ b/ios/NotificationServiceExtension/Info.plist @@ -13,7 +13,7 @@ CFBundleShortVersionString 9.0.66 CFBundleVersion - 9.0.66.6 + 9.0.66.7 NSExtension NSExtensionPointIdentifier diff --git a/package-lock.json b/package-lock.json index a58e33023eef..ef5871b02093 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "new.expensify", - "version": "9.0.66-6", + "version": "9.0.66-7", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "new.expensify", - "version": "9.0.66-6", + "version": "9.0.66-7", "hasInstallScript": true, "license": "MIT", "dependencies": { diff --git a/package.json b/package.json index c82882a2c9cf..dd7ad1afc04a 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "new.expensify", - "version": "9.0.66-6", + "version": "9.0.66-7", "author": "Expensify, Inc.", "homepage": "https://new.expensify.com", "description": "New Expensify is the next generation of Expensify: a reimagination of payments based atop a foundation of chat.", From 6dff844e78c0f31ac0eb527ae794c60b17351fed Mon Sep 17 00:00:00 2001 From: Bernhard Owen Josephus Date: Tue, 26 Nov 2024 10:14:01 +0800 Subject: [PATCH 40/40] make the whole search menu scrollable --- src/pages/Search/SearchTypeMenu.tsx | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/src/pages/Search/SearchTypeMenu.tsx b/src/pages/Search/SearchTypeMenu.tsx index 6d74ccb46e21..6658e05c298d 100644 --- a/src/pages/Search/SearchTypeMenu.tsx +++ b/src/pages/Search/SearchTypeMenu.tsx @@ -244,7 +244,10 @@ function SearchTypeMenu({queryJSON, searchName}: SearchTypeMenuProps) { const shouldShowSavedSearchesMenuItemTitle = Object.values(savedSearches ?? {}).filter((s) => s.pendingAction !== CONST.RED_BRICK_ROAD_PENDING_ACTION.DELETE || isOffline).length > 0; return ( - <> + {typeMenuItems.map((item, index) => { const onPress = singleExecution(() => { @@ -272,16 +275,11 @@ function SearchTypeMenu({queryJSON, searchName}: SearchTypeMenuProps) { {shouldShowSavedSearchesMenuItemTitle && ( <> {translate('search.savedSearchesMenuItemTitle')} - - {renderSavedSearchesSection(savedSearchesMenuItems())} - + {renderSavedSearchesSection(savedSearchesMenuItems())} )} - + ); }