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

Remove sideloading of Onyx data for attachment modal #30866

Merged
merged 12 commits into from
Nov 13, 2023
29 changes: 20 additions & 9 deletions src/components/AttachmentModal.js
Original file line number Diff line number Diff line change
Expand Up @@ -117,14 +117,14 @@ function AttachmentModal(props) {
const [isAttachmentInvalid, setIsAttachmentInvalid] = useState(false);
const [isDeleteReceiptConfirmModalVisible, setIsDeleteReceiptConfirmModalVisible] = useState(false);
const [isAuthTokenRequired, setIsAuthTokenRequired] = useState(props.isAuthTokenRequired);
const [isAttachmentReceipt, setIsAttachmentReceipt] = useState(false);
const [isAttachmentReceipt, setIsAttachmentReceipt] = useState(null);
const [attachmentInvalidReasonTitle, setAttachmentInvalidReasonTitle] = useState('');
const [attachmentInvalidReason, setAttachmentInvalidReason] = useState(null);
const [source, setSource] = useState(props.source);
const [modalType, setModalType] = useState(CONST.MODAL.MODAL_TYPE.CENTERED_UNSWIPEABLE);
const [isConfirmButtonDisabled, setIsConfirmButtonDisabled] = useState(false);
const [confirmButtonFadeAnimation] = useState(() => new Animated.Value(1));
const [shouldShowDownloadButton, setShouldShowDownloadButton] = React.useState(true);
const [isDownloadButtonReadyToBeShown, setIsDownloadButtonReadyToBeShown] = React.useState(true);
const {windowWidth} = useWindowDimensions();

const [file, setFile] = useState(
Expand Down Expand Up @@ -173,13 +173,13 @@ function AttachmentModal(props) {
);

const setDownloadButtonVisibility = useCallback(
(shouldShowButton) => {
if (shouldShowDownloadButton === shouldShowButton) {
(isButtonVisible) => {
if (isDownloadButtonReadyToBeShown === isButtonVisible) {
return;
}
setShouldShowDownloadButton(shouldShowButton);
setIsDownloadButtonReadyToBeShown(isButtonVisible);
},
[shouldShowDownloadButton],
[isDownloadButtonReadyToBeShown],
);

/**
Expand Down Expand Up @@ -393,6 +393,17 @@ function AttachmentModal(props) {
// eslint-disable-next-line react-hooks/exhaustive-deps
}, [isAttachmentReceipt, props.parentReport, props.parentReportActions, props.policy, props.transaction, file]);

// There are a few things that shouldn't be set until we absolutely know if the file is a receipt or an attachment.
// isAttachmentReceipt will be null until its certain what the file is, in which case it will then be true|false.
let headerTitle = props.headerTitle;
let shouldShowDownloadButton = false;
let shouldShowThreeDotsButton = false;
if (!_.isNull(isAttachmentReceipt)) {
headerTitle = translate(isAttachmentReceipt ? 'common.receipt' : 'common.attachment');
shouldShowDownloadButton = props.allowDownload && isDownloadButtonReadyToBeShown && !isAttachmentReceipt && !isOffline;
shouldShowThreeDotsButton = isAttachmentReceipt && isModalOpen;
}

return (
<>
<Modal
Expand All @@ -417,15 +428,15 @@ function AttachmentModal(props) {
>
{props.isSmallScreenWidth && <HeaderGap />}
<HeaderWithBackButton
title={props.headerTitle || translate(isAttachmentReceipt ? 'common.receipt' : 'common.attachment')}
title={headerTitle}
shouldShowBorderBottom
shouldShowDownloadButton={props.allowDownload && shouldShowDownloadButton && !isAttachmentReceipt && !isOffline}
shouldShowDownloadButton={shouldShowDownloadButton}
onDownloadButtonPress={() => downloadAttachment(source)}
shouldShowCloseButton={!props.isSmallScreenWidth}
shouldShowBackButton={props.isSmallScreenWidth}
onBackButtonPress={closeModal}
onCloseButtonPress={closeModal}
shouldShowThreeDotsButton={isAttachmentReceipt && isModalOpen}
shouldShowThreeDotsButton={shouldShowThreeDotsButton}
threeDotsAnchorPosition={styles.threeDotsPopoverOffsetAttachmentModal(windowWidth)}
threeDotsMenuItems={threeDotsMenuItems}
shouldOverlay
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import PropTypes from 'prop-types';
import transactionPropTypes from '@components/transactionPropTypes';
import reportActionPropTypes from '@pages/home/report/reportActionPropTypes';
import reportPropTypes from '@pages/reportPropTypes';

Expand All @@ -16,15 +17,27 @@ const propTypes = {
setDownloadButtonVisibility: PropTypes.func,

/** Object of report actions for this report */
reportActions: PropTypes.shape(reportActionPropTypes),
reportActions: PropTypes.objectOf(PropTypes.shape(reportActionPropTypes)),

/** The report currently being looked at */
report: reportPropTypes.isRequired,

/** The parent of `report` */
parentReport: reportPropTypes,

/** The report actions of the parent report */
parentReportActions: PropTypes.objectOf(PropTypes.shape(reportActionPropTypes)),

/** The transaction attached to the parent report action */
transaction: transactionPropTypes,
};

const defaultProps = {
source: '',
reportActions: {},
parentReport: {},
parentReportActions: {},
transaction: {},
onNavigate: () => {},
onClose: () => {},
setDownloadButtonVisibility: () => {},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,12 +9,13 @@ import CONST from '@src/CONST';

/**
* Constructs the initial component state from report actions
* @param {Object} report
* @param {Array} reportActions
* @param {Object} parentReportAction
* @param {Object} reportActions
* @param {Object} transaction
* @returns {Array}
*/
function extractAttachmentsFromReport(report, reportActions) {
const actions = [ReportActionsUtils.getParentReportAction(report), ...ReportActionsUtils.getSortedReportActions(_.values(reportActions))];
function extractAttachmentsFromReport(parentReportAction, reportActions, transaction) {
const actions = [parentReportAction, ...ReportActionsUtils.getSortedReportActions(_.values(reportActions))];
const attachments = [];

const htmlParser = new HtmlParser({
Expand Down Expand Up @@ -51,7 +52,6 @@ function extractAttachmentsFromReport(report, reportActions) {
return;
}

const transaction = TransactionUtils.getTransaction(transactionID);
if (TransactionUtils.hasReceipt(transaction)) {
const {image} = ReceiptUtils.getThumbnailAndImageURIs(transaction);
const isLocalFile = typeof image === 'string' && (image.startsWith('blob:') || image.startsWith('file:'));
Expand Down
32 changes: 24 additions & 8 deletions src/components/Attachments/AttachmentCarousel/index.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import lodashGet from 'lodash/get';
import React, {useCallback, useEffect, useRef, useState} from 'react';
import {FlatList, Keyboard, PixelRatio, View} from 'react-native';
import {withOnyx} from 'react-native-onyx';
Expand All @@ -9,7 +10,6 @@ import withWindowDimensions from '@components/withWindowDimensions';
import compose from '@libs/compose';
import * as DeviceCapabilities from '@libs/DeviceCapabilities';
import Navigation from '@libs/Navigation/Navigation';
import * as ReportActionsUtils from '@libs/ReportActionsUtils';
import styles from '@styles/styles';
import variables from '@styles/variables';
import ONYXKEYS from '@src/ONYXKEYS';
Expand All @@ -27,7 +27,7 @@ const viewabilityConfig = {
itemVisiblePercentThreshold: 95,
};

function AttachmentCarousel({report, reportActions, source, onNavigate, setDownloadButtonVisibility, translate}) {
function AttachmentCarousel({report, reportActions, parentReportActions, source, onNavigate, setDownloadButtonVisibility, translate, transaction}) {
const scrollRef = useRef(null);

const canUseTouchScreen = DeviceCapabilities.canUseTouchScreen();
Expand All @@ -42,17 +42,16 @@ function AttachmentCarousel({report, reportActions, source, onNavigate, setDownl
const compareImage = useCallback(
(attachment) => {
if (attachment.isReceipt && isReceipt) {
const action = ReportActionsUtils.getParentReportAction(report);
const transactionID = _.get(action, ['originalMessage', 'IOUTransactionID']);
return attachment.transactionID === transactionID;
return attachment.transactionID === transaction.transactionID;
}
return attachment.source === source;
},
[source, report, isReceipt],
[source, isReceipt, transaction],
);

useEffect(() => {
const attachmentsFromReport = extractAttachmentsFromReport(report, reportActions);
const parentReportAction = parentReportActions[report.parentReportActionID];
const attachmentsFromReport = extractAttachmentsFromReport(parentReportAction, reportActions, transaction);

const initialPage = _.findIndex(attachmentsFromReport, compareImage);

Expand All @@ -72,7 +71,7 @@ function AttachmentCarousel({report, reportActions, source, onNavigate, setDownl
}
}
// eslint-disable-next-line react-hooks/exhaustive-deps
}, [reportActions, compareImage]);
}, [reportActions, parentReportActions, compareImage]);

/**
* Updates the page state when the user navigates between attachments
Expand Down Expand Up @@ -226,11 +225,28 @@ AttachmentCarousel.defaultProps = defaultProps;
AttachmentCarousel.displayName = 'AttachmentCarousel';

export default compose(
// eslint-disable-next-line rulesdir/no-multiple-onyx-in-file
dangrous marked this conversation as resolved.
Show resolved Hide resolved
withOnyx({
reportActions: {
key: ({report}) => `${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${report.reportID}`,
canEvict: false,
},
parentReport: {
key: ({report}) => `${ONYXKEYS.COLLECTION.REPORT}${report ? report.parentReportID : '0'}`,
},
parentReportActions: {
key: ({report}) => `${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${report ? report.parentReportID : '0'}`,
canEvict: false,
},
}),
// eslint-disable-next-line rulesdir/no-multiple-onyx-in-file
withOnyx({
dangrous marked this conversation as resolved.
Show resolved Hide resolved
transaction: {
key: ({report, parentReportActions}) => {
const parentReportAction = lodashGet(parentReportActions, [report.parentReportActionID]);
return `${ONYXKEYS.COLLECTION.TRANSACTION}${lodashGet(parentReportAction, 'originalMessage.IOUTransactionID', 0)}`;
},
},
}),
withLocalize,
withWindowDimensions,
Expand Down
30 changes: 23 additions & 7 deletions src/components/Attachments/AttachmentCarousel/index.native.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import lodashGet from 'lodash/get';
import React, {useCallback, useEffect, useRef, useState} from 'react';
import {Keyboard, PixelRatio, View} from 'react-native';
import {withOnyx} from 'react-native-onyx';
Expand All @@ -7,7 +8,6 @@ import * as Illustrations from '@components/Icon/Illustrations';
import withLocalize from '@components/withLocalize';
import compose from '@libs/compose';
import Navigation from '@libs/Navigation/Navigation';
import * as ReportActionsUtils from '@libs/ReportActionsUtils';
import styles from '@styles/styles';
import variables from '@styles/variables';
import ONYXKEYS from '@src/ONYXKEYS';
Expand All @@ -18,7 +18,7 @@ import extractAttachmentsFromReport from './extractAttachmentsFromReport';
import AttachmentCarouselPager from './Pager';
import useCarouselArrows from './useCarouselArrows';

function AttachmentCarousel({report, reportActions, source, onNavigate, onClose, setDownloadButtonVisibility, translate}) {
function AttachmentCarousel({report, reportActions, parentReportActions, source, onNavigate, setDownloadButtonVisibility, translate, transaction, onClose}) {
const pagerRef = useRef(null);

const [containerDimensions, setContainerDimensions] = useState({width: 0, height: 0});
Expand All @@ -32,17 +32,16 @@ function AttachmentCarousel({report, reportActions, source, onNavigate, onClose,
const compareImage = useCallback(
(attachment) => {
if (attachment.isReceipt && isReceipt) {
const action = ReportActionsUtils.getParentReportAction(report);
const transactionID = _.get(action, ['originalMessage', 'IOUTransactionID']);
return attachment.transactionID === transactionID;
return attachment.transactionID === transaction.transactionID;
}
return attachment.source === source;
},
[source, report, isReceipt],
[source, isReceipt, transaction],
);

useEffect(() => {
const attachmentsFromReport = extractAttachmentsFromReport(report, reportActions);
const parentReportAction = parentReportActions[report.parentReportActionID];
const attachmentsFromReport = extractAttachmentsFromReport(parentReportAction, reportActions, transaction);

const initialPage = _.findIndex(attachmentsFromReport, compareImage);

Expand Down Expand Up @@ -172,11 +171,28 @@ AttachmentCarousel.defaultProps = defaultProps;
AttachmentCarousel.displayName = 'AttachmentCarousel';

export default compose(
// eslint-disable-next-line rulesdir/no-multiple-onyx-in-file
withOnyx({
reportActions: {
key: ({report}) => `${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${report.reportID}`,
canEvict: false,
},
parentReport: {
key: ({report}) => `${ONYXKEYS.COLLECTION.REPORT}${report ? report.parentReportID : '0'}`,
},
parentReportActions: {
key: ({report}) => `${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${report ? report.parentReportID : '0'}`,
canEvict: false,
},
}),
// eslint-disable-next-line rulesdir/no-multiple-onyx-in-file
withOnyx({
transaction: {
key: ({report, parentReportActions}) => {
const parentReportAction = lodashGet(parentReportActions, [report.parentReportActionID]);
return `${ONYXKEYS.COLLECTION.TRANSACTION}${lodashGet(parentReportAction, 'originalMessage.IOUTransactionID', 0)}`;
},
},
}),
withLocalize,
)(AttachmentCarousel);
Loading