Skip to content
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 new HOC to get policy.connections data #39132

Merged
merged 37 commits into from
Apr 17, 2024
Merged
Show file tree
Hide file tree
Changes from 9 commits
Commits
Show all changes
37 commits
Select commit Hold shift + click to select a range
06e5ea4
chore: add parameter type for OpenPolicyAccountingPage
Mar 27, 2024
c945585
chore: add new command
Mar 27, 2024
e0532d5
chore: add new action func to call web-e command
Mar 27, 2024
e6292e3
feat: add new HOC component
Mar 27, 2024
b509d0f
fix: conditional statement & change the parameter from object to sing…
Mar 28, 2024
8e32fde
fix: check if the connections feature is enabled & add comments
Mar 28, 2024
d8139a8
doc: add and improve comments
Mar 29, 2024
9a44aeb
doc: add comment on the purpose of the new state
Mar 29, 2024
cf2cce6
doc: remove unnecessary comments
Mar 29, 2024
2370278
chore: create a context
Apr 4, 2024
db3252e
fix: import react
Apr 4, 2024
0eca9cd
Merge branch 'main' into hayata-add-new-hoc
Apr 5, 2024
b7ab9e0
Merge branch 'main' into hayata-add-new-hoc
Apr 11, 2024
1b09ba0
chore: remove the context
Apr 12, 2024
32e979b
chore: use withPolicyConnectionsHOC
Apr 12, 2024
6f5d49b
feat: add field for loading status inside policy
Apr 15, 2024
14c8e92
feat: add new field to Policy object
Apr 15, 2024
357ce25
chore: update the fetching status field when taking the API request
Apr 15, 2024
d9340cf
chore: check the fetched and loading statuses before making the api r…
Apr 15, 2024
68db14d
refactor: remove unused statuses
Apr 15, 2024
4ba46b9
Merge branch 'main' into hayata-add-new-hoc
Apr 15, 2024
2254f3e
feat: add a new Onyx key
Apr 17, 2024
b1272d4
chore: remove fetch state from the policy object
Apr 17, 2024
d29646d
chore: remove unused field from the policy object
Apr 17, 2024
124f534
fix: delete non-existing variable
Apr 17, 2024
c1e0cac
fix: change the new key to the collections
Apr 17, 2024
515833e
feat: use the fetched state
Apr 17, 2024
f21307a
chore: update the fetched state
Apr 17, 2024
a44ab1c
chore: display loading indicator while data is being fetched
Apr 17, 2024
170cf3a
chore: ensure to display offline screen
Apr 17, 2024
f25a3e5
fix: getting the actual value and status from useOnyx
Apr 17, 2024
31cd1ec
Merge branch 'main' into hayata-add-new-hoc
Apr 17, 2024
9457554
fix: update the prop list
Apr 17, 2024
c4593e7
add missing underscore at the end of the Onyx collection key name
Apr 17, 2024
4804fd1
fix: default policy id to 0
Apr 17, 2024
e19060b
doc: specify the reason why the loading state can't be inside the pol…
Apr 17, 2024
a084e52
fix: remove as const statement
Apr 17, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions src/libs/API/parameters/OpenPolicyAccountingPageParams.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
type OpenPolicyAccountingPageParams = {
policyID: string;
};

export default OpenPolicyAccountingPageParams;
1 change: 1 addition & 0 deletions src/libs/API/parameters/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -198,3 +198,4 @@ export type {default as SetPolicyCustomTaxNameParams} from './SetPolicyCustomTax
export type {default as SetPolicyForeignCurrencyDefaultParams} from './SetPolicyForeignCurrencyDefaultParams';
export type {default as SetPolicyCurrencyDefaultParams} from './SetPolicyCurrencyDefaultParams';
export type {default as RenamePolicyTaxParams} from './RenamePolicyTaxParams';
export type {default as OpenPolicyAccountingPageParams} from './OpenPolicyAccountingPageParams';
2 changes: 2 additions & 0 deletions src/libs/API/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -426,6 +426,7 @@ const READ_COMMANDS = {
OPEN_POLICY_WORKFLOWS_PAGE: 'OpenPolicyWorkflowsPage',
OPEN_POLICY_DISTANCE_RATES_PAGE: 'OpenPolicyDistanceRatesPage',
OPEN_POLICY_MORE_FEATURES_PAGE: 'OpenPolicyMoreFeaturesPage',
OPEN_POLICY_ACCOUNTING_PAGE: 'OpenPolicyAccountingPage',
} as const;

type ReadCommand = ValueOf<typeof READ_COMMANDS>;
Expand Down Expand Up @@ -466,6 +467,7 @@ type ReadCommandParameters = {
[READ_COMMANDS.OPEN_POLICY_WORKFLOWS_PAGE]: Parameters.OpenPolicyWorkflowsPageParams;
[READ_COMMANDS.OPEN_POLICY_DISTANCE_RATES_PAGE]: Parameters.OpenPolicyDistanceRatesPageParams;
[READ_COMMANDS.OPEN_POLICY_MORE_FEATURES_PAGE]: Parameters.OpenPolicyMoreFeaturesPageParams;
[READ_COMMANDS.OPEN_POLICY_ACCOUNTING_PAGE]: Parameters.OpenPolicyAccountingPageParams;
};

const SIDE_EFFECT_REQUEST_COMMANDS = {
Expand Down
15 changes: 15 additions & 0 deletions src/libs/actions/PolicyConnections.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
import * as API from '@libs/API';
import type {OpenPolicyAccountingPageParams} from '@libs/API/parameters';
import {READ_COMMANDS} from '@libs/API/types';

function openPolicyAccountingPage(policyID: string) {
const parameters: OpenPolicyAccountingPageParams = {
policyID,
};

API.read(READ_COMMANDS.OPEN_POLICY_ACCOUNTING_PAGE, parameters);
}

// More action functions will be added later
// eslint-disable-next-line import/prefer-default-export
export {openPolicyAccountingPage};
73 changes: 73 additions & 0 deletions src/pages/workspace/withPolicyConnections.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,73 @@
import {useEffect, useState} from 'react';
hayata-suenaga marked this conversation as resolved.
Show resolved Hide resolved
import type {ComponentType} from 'react';
import FullPageOfflineBlockingView from '@components/BlockingViews/FullPageOfflineBlockingView';
import useNetwork from '@hooks/useNetwork';
import {openPolicyAccountingPage} from '@libs/actions/PolicyConnections';
import withPolicy from './withPolicy';
import type {WithPolicyProps} from './withPolicy';

type WithPolicyConnectionsProps = WithPolicyProps;

/**
* Higher-order component that fetches the connections data and populates
* the corresponding field of the policy object if the field is empty. It then passes the policy object
* to the wrapped component.
*
* Use this HOC when you need the policy object with its connections field populated.
*
* Only the active policy gets the complete policy data upon app start that includes the connections data.
* For other policies, the connections data needs to be fetched when it's needed.
*/
function withPolicyConnections(WrappedComponent: ComponentType<WithPolicyConnectionsProps>) {
function WithPolicyConnections({policy, policyMembers, policyDraft, policyMembersDraft, route}: WithPolicyConnectionsProps) {
const {isOffline} = useNetwork();

// When the accounting feature is enabled, but the user hasn't connected to any accounting software,
// the connections data doesn't exist. We don't want to continually attempt to fetch non-existent data.
// This state helps us track whether a data fetch attempt has been made.
const [wasConnectionsDataFetched, setWasConnectionsDataFetched] = useState(false);

useEffect(() => {
// When the accounting feature is not enabled, or if the connections data already exists,
// there is no need to fetch the connections data.
if (wasConnectionsDataFetched || !policy?.areConnectionsEnabled || !!policy?.connections || !policy?.id) {
return;
}

openPolicyAccountingPage(policy.id);
setWasConnectionsDataFetched(true);
}, [policy, wasConnectionsDataFetched]);

if (!policy?.connections) {
if (isOffline) {
return (
<FullPageOfflineBlockingView>
<WrappedComponent
policy={policy}
policyMembers={policyMembers}
policyDraft={policyDraft}
policyMembersDraft={policyMembersDraft}
route={route}
/>
</FullPageOfflineBlockingView>
);
}

return null;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't get why we return nothing if the connections are false, I try to use it on the workspace profile page but it shows a blank page. Also, I'm curious about what case the HOC will be used.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added some comments in the new commit. It explains the usage of the HOC and the context that I also added in that commit.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't get why we return nothing if the connections are false

@mollfpr, @aldo-expensify, should we return LoadingPage instead of null?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We might be able to leverage this later. The only case I think we should return null is where we want to navigate the page if there are no connections to avoid rendering a component while animating the transition.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should should a spinner if policy.isLoadingConnections is true, and if policy.isLoadingConnections is false, hmmm 404? 🤷

}

return (
<WrappedComponent
policy={policy}
policyMembers={policyMembers}
policyDraft={policyDraft}
policyMembersDraft={policyMembersDraft}
route={route}
/>
);
}

return withPolicy(WithPolicyConnections);
}

export default withPolicyConnections;
Loading