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.

Sign up for GitHub

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

Jump to bottom

Last synced just now does not update #41310

Merged
merged 13 commits into from
May 24, 2024
Merged

Last synced just now does not update #41310

Changes from 11 commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
6fb62d5
Last synced just now doesnt update
tienifr Apr 30, 2024
4ab21ee
Merge branch 'main' into fix/41045
tienifr May 1, 2024
c3e0e4b
lint fix
tienifr May 1, 2024
ff3397e
resolve conflicts
tienifr May 7, 2024
8ecfa6e
fix: formate date
tienifr May 7, 2024
12d407b
Merge branch 'main' into fix/41045
tienifr May 8, 2024
bcaca76
fix: typo word
tienifr May 8, 2024
fa426e3
Merge branch 'main' into fix/41045
tienifr May 10, 2024
f6e5060
convert date to string
tienifr May 10, 2024
c82b2d9
Merge branch 'main' into fix/41045
tienifr May 13, 2024
69a3652
fix timeone error
tienifr May 13, 2024
e343f5b
Merge branch 'main' into fix/41045
tienifr May 14, 2024
0c8b014
remove setinterval
tienifr May 14, 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
31 changes: 24 additions & 7 deletions src/pages/workspace/accounting/PolicyAccountingPage.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import React, {useMemo, useRef, useState} from 'react';
import {formatDistanceToNow} from 'date-fns';
import React, {useEffect, useMemo, useRef, useState} from 'react';
import {ActivityIndicator, View} from 'react-native';
import {withOnyx} from 'react-native-onyx';
import type {OnyxEntry} from 'react-native-onyx';
Expand Down Expand Up @@ -34,13 +35,14 @@
import CONST from '@src/CONST';
import ONYXKEYS from '@src/ONYXKEYS';
import ROUTES from '@src/ROUTES';
import type {Policy, PolicyConnectionSyncProgress} from '@src/types/onyx';
import type {Locale, Policy, PolicyConnectionSyncProgress} from '@src/types/onyx';
import type {PolicyConnectionName} from '@src/types/onyx/Policy';
import {isEmptyObject} from '@src/types/utils/EmptyObject';
import type IconAsset from '@src/types/utils/IconAsset';

type PolicyAccountingPageOnyxProps = {
connectionSyncProgress: OnyxEntry<PolicyConnectionSyncProgress>;
preferredLocale: OnyxEntry<Locale>;
};

type PolicyAccountingPageProps = WithPolicyProps &
Expand Down Expand Up @@ -100,7 +102,7 @@
}
}

function PolicyAccountingPage({policy, connectionSyncProgress}: PolicyAccountingPageProps) {
function PolicyAccountingPage({policy, connectionSyncProgress, preferredLocale}: PolicyAccountingPageProps) {

Check failure on line 105 in src/pages/workspace/accounting/PolicyAccountingPage.tsx

View workflow job for this annotation

GitHub Actions / Run ESLint 

'preferredLocale' is defined but never used
const theme = useTheme();
const styles = useThemeStyles();
const {translate} = useLocalize();
Expand All @@ -109,13 +111,16 @@
const {isSmallScreenWidth, windowWidth} = useWindowDimensions();
const [threeDotsMenuPosition, setThreeDotsMenuPosition] = useState<AnchorPosition>({horizontal: 0, vertical: 0});
const [isDisconnectModalOpen, setIsDisconnectModalOpen] = useState(false);
const [datetimeToRelative, setDateTimeToRelative] = useState('');
const threeDotsMenuContainerRef = useRef<View>(null);

const isSyncInProgress = !!connectionSyncProgress?.stageInProgress && connectionSyncProgress.stageInProgress !== CONST.POLICY.CONNECTIONS.SYNC_STAGE_NAME.JOB_DONE;

const accountingIntegrations = Object.values(CONST.POLICY.CONNECTIONS.NAME).filter((name) => !(name === CONST.POLICY.CONNECTIONS.NAME.XERO && !canUseXeroIntegration));
const connectedIntegration = accountingIntegrations.find((integration) => !!policy?.connections?.[integration]) ?? connectionSyncProgress?.connectionName;
const policyID = policy?.id ?? '';
const successfulDate = policy?.connections?.quickbooksOnline?.lastSync?.successfulDate;
mountiny marked this conversation as resolved.
Show resolved Hide resolved
const formattedDate = successfulDate ? new Date(successfulDate) : new Date();

Check warning on line 123 in src/pages/workspace/accounting/PolicyAccountingPage.tsx

View workflow job for this annotation

GitHub Actions / Run ESLint 

The 'formattedDate' conditional could make the dependencies of useEffect Hook (at line 157) change on every render. Move it inside the useEffect callback. Alternatively, wrap the initialization of 'formattedDate' in its own useMemo() Hook

const policyConnectedToXero = connectedIntegration === CONST.POLICY.CONNECTIONS.NAME.XERO;

Expand All @@ -140,6 +145,17 @@
[translate, policyID, isOffline],
);

useEffect(() => {
setDateTimeToRelative(formatDistanceToNow(formattedDate, {addSuffix: true}));
const interval = setInterval(() => {
Copy link
Contributor

Choose a reason for hiding this comment

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

Why do we need to use setInterval here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If we don't use setInterval, we have to refresh page to see the new relativeTime

Copy link
Contributor

Choose a reason for hiding this comment

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

Wouldn't any change in formattedDate trigger the useEffect though?

Copy link
Contributor

Choose a reason for hiding this comment

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

I agree with Olly, shouldn't this already be handled by the useEffect dependency?

Copy link
Contributor

Choose a reason for hiding this comment

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

@tienifr i dont think this is a requirement @trjExpensify i think its fine to not update the sync date until we get pusher update or user navigates away and back to the page.

I dont think there is much value in having the text auto update when user stays on the page

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah, agreed. Pretty sure with OldDot we do that too.

setDateTimeToRelative(formatDistanceToNow(formattedDate, {addSuffix: true}));
}, 60 * 1000);

return () => {
clearInterval(interval);
};
}, [formattedDate]);

const connectionsMenuItems: MenuItemProps[] = useMemo(() => {
if (isEmptyObject(policy?.connections) && !isSyncInProgress) {
return accountingIntegrations.map((integration) => {
Expand Down Expand Up @@ -168,10 +184,7 @@
wrapperStyle: [styles.sectionMenuItemTopDescription],
shouldShowRightComponent: true,
title: integrationData?.title,

description: isSyncInProgress
? translate('workspace.accounting.connections.syncStageName', connectionSyncProgress.stageInProgress)
: translate('workspace.accounting.lastSync'),
description: isSyncInProgress ? translate('workspace.accounting.connections.syncStageName', connectionSyncProgress.stageInProgress) : datetimeToRelative,
rightComponent: isSyncInProgress ? (
<ActivityIndicator
style={[styles.popoverMenuIcon]}
Expand Down Expand Up @@ -257,6 +270,7 @@
theme.spinner,
threeDotsMenuPosition,
translate,
datetimeToRelative,
accountingIntegrations,
]);

Expand Down Expand Up @@ -380,5 +394,8 @@
connectionSyncProgress: {
key: (props) => `${ONYXKEYS.COLLECTION.POLICY_CONNECTION_SYNC_PROGRESS}${props.route.params.policyID}`,
},
preferredLocale: {
key: ONYXKEYS.NVP_PREFERRED_LOCALE,
},
})(PolicyAccountingPage),
);
Loading