Skip to content

Commit

Permalink
Merge pull request #32647 from dukenv0307/fix/31105
Browse files Browse the repository at this point in the history 
Fix the current issue and regression
  • Loading branch information
Hayata Suenaga authored Dec 11, 2023
2 parents 458f121 + e963f05 commit 3358b03
Show file tree
Hide file tree
Showing 5 changed files with 44 additions and 107 deletions.
36 changes: 17 additions & 19 deletions src/components/AttachmentModal.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import Str from 'expensify-common/lib/str';
import lodashExtend from 'lodash/extend';
import lodashGet from 'lodash/get';
import PropTypes from 'prop-types';
import React, {useCallback, useEffect, useMemo, useRef, useState} from 'react';
import React, {useCallback, useEffect, useMemo, useState} from 'react';
import {Animated, Keyboard, View} from 'react-native';
import {withOnyx} from 'react-native-onyx';
import _ from 'underscore';
Expand Down Expand Up @@ -90,6 +90,9 @@ const propTypes = {

/** Denotes whether it is a workspace avatar or not */
isWorkspaceAvatar: PropTypes.bool,

/** Whether it is a receipt attachment or not */
isReceiptAttachment: PropTypes.bool,
};

const defaultProps = {
Expand All @@ -107,19 +110,18 @@ const defaultProps = {
onModalHide: () => {},
onCarouselAttachmentChange: () => {},
isWorkspaceAvatar: false,
isReceiptAttachment: false,
};

function AttachmentModal(props) {
const theme = useTheme();
const styles = useThemeStyles();
const StyleUtils = useStyleUtils();
const onModalHideCallbackRef = useRef(null);
const [isModalOpen, setIsModalOpen] = useState(props.defaultOpen);
const [shouldLoadAttachment, setShouldLoadAttachment] = useState(false);
const [isAttachmentInvalid, setIsAttachmentInvalid] = useState(false);
const [isDeleteReceiptConfirmModalVisible, setIsDeleteReceiptConfirmModalVisible] = useState(false);
const [isAuthTokenRequired, setIsAuthTokenRequired] = useState(props.isAuthTokenRequired);
const [isAttachmentReceipt, setIsAttachmentReceipt] = useState(null);
const [attachmentInvalidReasonTitle, setAttachmentInvalidReasonTitle] = useState('');
const [attachmentInvalidReason, setAttachmentInvalidReason] = useState(null);
const [source, setSource] = useState(props.source);
Expand Down Expand Up @@ -155,7 +157,6 @@ function AttachmentModal(props) {
(attachment) => {
setSource(attachment.source);
setFile(attachment.file);
setIsAttachmentReceipt(attachment.isReceipt);
setIsAuthTokenRequired(attachment.isAuthTokenRequired);
onCarouselAttachmentChange(attachment);
},
Expand Down Expand Up @@ -358,7 +359,7 @@ function AttachmentModal(props) {
const sourceForAttachmentView = props.source || source;

const threeDotsMenuItems = useMemo(() => {
if (!isAttachmentReceipt || !props.parentReport || !props.parentReportActions) {
if (!props.isReceiptAttachment || !props.parentReport || !props.parentReportActions) {
return [];
}
const menuItems = [];
Expand All @@ -372,8 +373,8 @@ function AttachmentModal(props) {
icon: Expensicons.Camera,
text: props.translate('common.replace'),
onSelected: () => {
onModalHideCallbackRef.current = () => Navigation.navigate(ROUTES.EDIT_REQUEST.getRoute(props.report.reportID, CONST.EDIT_REQUEST_FIELD.RECEIPT));
closeModal();
Navigation.navigate(ROUTES.EDIT_REQUEST.getRoute(props.report.reportID, CONST.EDIT_REQUEST_FIELD.RECEIPT));
},
});
}
Expand All @@ -393,17 +394,17 @@ function AttachmentModal(props) {
}
return menuItems;
// eslint-disable-next-line react-hooks/exhaustive-deps
}, [isAttachmentReceipt, props.parentReport, props.parentReportActions, props.policy, props.transaction, file]);
}, [props.isReceiptAttachment, 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.
// props.isReceiptAttachment 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;
if (!_.isEmpty(props.report)) {
headerTitle = translate(props.isReceiptAttachment ? 'common.receipt' : 'common.attachment');
shouldShowDownloadButton = props.allowDownload && isDownloadButtonReadyToBeShown && !props.isReceiptAttachment && !isOffline;
shouldShowThreeDotsButton = props.isReceiptAttachment && isModalOpen;
}

return (
Expand All @@ -420,10 +421,6 @@ function AttachmentModal(props) {
}}
onModalHide={(e) => {
props.onModalHide(e);
if (onModalHideCallbackRef.current) {
onModalHideCallbackRef.current();
}

setShouldLoadAttachment(false);
}}
propagateSwipe
Expand All @@ -444,7 +441,7 @@ function AttachmentModal(props) {
shouldOverlay
/>
<View style={styles.imageModalImageCenterContainer}>
{!_.isEmpty(props.report) ? (
{!_.isEmpty(props.report) && !props.isReceiptAttachment ? (
<AttachmentCarousel
report={props.report}
onNavigate={onNavigate}
Expand All @@ -465,6 +462,7 @@ function AttachmentModal(props) {
isWorkspaceAvatar={props.isWorkspaceAvatar}
fallbackSource={props.fallbackSource}
isUsedInAttachmentModal
transactionID={props.transaction.transactionID}
/>
)
)}
Expand All @@ -487,7 +485,7 @@ function AttachmentModal(props) {
)}
</SafeAreaConsumer>
)}
{isAttachmentReceipt && (
{props.isReceiptAttachment && (
<ConfirmModal
title={translate('receipt.deleteReceipt')}
isVisible={isDeleteReceiptConfirmModalVisible}
Expand All @@ -500,7 +498,7 @@ function AttachmentModal(props) {
/>
)}
</Modal>
{!isAttachmentReceipt && (
{!props.isReceiptAttachment && (
<ConfirmModal
title={attachmentInvalidReasonTitle ? translate(attachmentInvalidReasonTitle) : ''}
onConfirm={closeConfirmModal}
Expand Down
29 changes: 2 additions & 27 deletions src/components/Attachments/AttachmentCarousel/extractAttachmentsFromReport.js
View file
Original file line number Diff line number Diff line change
@@ -1,10 +1,7 @@
import {Parser as HtmlParser} from 'htmlparser2';
import lodashGet from 'lodash/get';
import _ from 'underscore';
import * as FileUtils from '@libs/fileDownload/FileUtils';
import * as ReceiptUtils from '@libs/ReceiptUtils';
import * as ReportActionsUtils from '@libs/ReportActionsUtils';
import * as TransactionUtils from '@libs/TransactionUtils';
import tryResolveUrlFromApiRoot from '@libs/tryResolveUrlFromApiRoot';
import CONST from '@src/CONST';

Expand All @@ -15,7 +12,7 @@ import CONST from '@src/CONST';
* @param {Object} transaction
* @returns {Array}
*/
function extractAttachmentsFromReport(parentReportAction, reportActions, transaction) {
function extractAttachmentsFromReport(parentReportAction, reportActions) {
const actions = [parentReportAction, ...ReportActionsUtils.getSortedReportActions(_.values(reportActions))];
const attachments = [];

Expand Down Expand Up @@ -43,32 +40,10 @@ function extractAttachmentsFromReport(parentReportAction, reportActions, transac
});

_.forEach(actions, (action, key) => {
if (!ReportActionsUtils.shouldReportActionBeVisible(action, key)) {
if (!ReportActionsUtils.shouldReportActionBeVisible(action, key) || ReportActionsUtils.isMoneyRequestAction(action)) {
return;
}

// We're handling receipts differently here because receipt images are not
// part of the report action message, the images are constructed client-side
if (ReportActionsUtils.isMoneyRequestAction(action)) {
const transactionID = lodashGet(action, ['originalMessage', 'IOUTransactionID']);
if (!transactionID) {
return;
}

if (TransactionUtils.hasReceipt(transaction)) {
const {image} = ReceiptUtils.getThumbnailAndImageURIs(transaction);
const isLocalFile = typeof image === 'string' && _.some(CONST.ATTACHMENT_LOCAL_URL_PREFIX, (prefix) => image.startsWith(prefix));
attachments.unshift({
source: tryResolveUrlFromApiRoot(image),
isAuthTokenRequired: !isLocalFile,
file: {name: transaction.filename},
isReceipt: true,
transactionID,
});
return;
}
}

const decision = _.get(action, ['message', 0, 'moderationDecision', 'decision'], '');
const hasBeenFlagged = decision === CONST.MODERATION.MODERATOR_DECISION_PENDING_HIDE || decision === CONST.MODERATION.MODERATOR_DECISION_HIDDEN;
const html = _.get(action, ['message', 0, 'html'], '').replace('/>', `data-flagged="${hasBeenFlagged}" data-id="${action.reportActionID}"/>`);
Expand Down
28 changes: 3 additions & 25 deletions src/components/Attachments/AttachmentCarousel/index.js
View file
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
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 Down Expand Up @@ -28,7 +27,7 @@ const viewabilityConfig = {
itemVisiblePercentThreshold: 95,
};

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

Expand All @@ -39,21 +38,12 @@ function AttachmentCarousel({report, reportActions, parentReportActions, source,
const [attachments, setAttachments] = useState([]);
const [activeSource, setActiveSource] = useState(source);
const [shouldShowArrows, setShouldShowArrows, autoHideArrows, cancelAutoHideArrows] = useCarouselArrows();
const [isReceipt, setIsReceipt] = useState(false);

const compareImage = useCallback(
(attachment) => {
if (attachment.isReceipt && isReceipt) {
return attachment.transactionID === transaction.transactionID;
}
return attachment.source === source;
},
[source, isReceipt, transaction],
);
const compareImage = useCallback((attachment) => attachment.source === source, [source]);

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

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

Expand Down Expand Up @@ -88,12 +78,10 @@ function AttachmentCarousel({report, reportActions, parentReportActions, source,
// to get the index of the current page
const entry = _.first(viewableItems);
if (!entry) {
setIsReceipt(false);
setActiveSource(null);
return;
}

setIsReceipt(entry.item.isReceipt);
setPage(entry.index);
setActiveSource(entry.item.source);

Expand Down Expand Up @@ -227,7 +215,6 @@ 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}`,
Expand All @@ -241,15 +228,6 @@ export default compose(
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,
withWindowDimensions,
)(AttachmentCarousel);
27 changes: 3 additions & 24 deletions src/components/Attachments/AttachmentCarousel/index.native.js
View file
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
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 @@ -18,7 +17,7 @@ import extractAttachmentsFromReport from './extractAttachmentsFromReport';
import AttachmentCarouselPager from './Pager';
import useCarouselArrows from './useCarouselArrows';

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

Expand All @@ -28,21 +27,12 @@ function AttachmentCarousel({report, reportActions, parentReportActions, source,
const [activeSource, setActiveSource] = useState(source);
const [isPinchGestureRunning, setIsPinchGestureRunning] = useState(true);
const [shouldShowArrows, setShouldShowArrows, autoHideArrows, cancelAutoHideArrows] = useCarouselArrows();
const [isReceipt, setIsReceipt] = useState(false);

const compareImage = useCallback(
(attachment) => {
if (attachment.isReceipt && isReceipt) {
return attachment.transactionID === transaction.transactionID;
}
return attachment.source === source;
},
[source, isReceipt, transaction],
);
const compareImage = useCallback((attachment) => attachment.source === source, [source]);

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

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

Expand Down Expand Up @@ -77,7 +67,6 @@ function AttachmentCarousel({report, reportActions, parentReportActions, source,
const item = attachments[newPageIndex];

setPage(newPageIndex);
setIsReceipt(item.isReceipt);
setActiveSource(item.source);

onNavigate(item);
Expand Down Expand Up @@ -172,7 +161,6 @@ 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}`,
Expand All @@ -186,14 +174,5 @@ export default compose(
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);
31 changes: 19 additions & 12 deletions src/components/ReportActionItem/ReportActionItemImage.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,19 +2,18 @@ import PropTypes from 'prop-types';
import React from 'react';
import {View} from 'react-native';
import _ from 'underscore';
import AttachmentModal from '@components/AttachmentModal';
import EReceiptThumbnail from '@components/EReceiptThumbnail';
import Image from '@components/Image';
import PressableWithoutFocus from '@components/Pressable/PressableWithoutFocus';
import {ShowContextMenuContext} from '@components/ShowContextMenuContext';
import ThumbnailImage from '@components/ThumbnailImage';
import transactionPropTypes from '@components/transactionPropTypes';
import useLocalize from '@hooks/useLocalize';
import Navigation from '@libs/Navigation/Navigation';
import * as TransactionUtils from '@libs/TransactionUtils';
import tryResolveUrlFromApiRoot from '@libs/tryResolveUrlFromApiRoot';
import useThemeStyles from '@styles/useThemeStyles';
import CONST from '@src/CONST';
import ROUTES from '@src/ROUTES';

const propTypes = {
/** thumbnail URI for the image */
Expand Down Expand Up @@ -83,17 +82,25 @@ function ReportActionItemImage({thumbnail, image, enablePreviewModal, transactio
return (
<ShowContextMenuContext.Consumer>
{({report}) => (
<PressableWithoutFocus
style={[styles.noOutline, styles.w100, styles.h100]}
onPress={() => {
const route = ROUTES.REPORT_ATTACHMENTS.getRoute(report.reportID, imageSource);
Navigation.navigate(route);
}}
role={CONST.ACCESSIBILITY_ROLE.IMAGEBUTTON}
accessibilityLabel={translate('accessibilityHints.viewAttachment')}
<AttachmentModal
source={imageSource}
isAuthTokenRequired={!isLocalFile}
report={report}
isReceiptAttachment
allowToDownload
originalFileName={transaction.filename}
>
{receiptImageComponent}
</PressableWithoutFocus>
{({show}) => (
<PressableWithoutFocus
style={[styles.noOutline, styles.w100, styles.h100]}
onPress={show}
role={CONST.ACCESSIBILITY_ROLE.IMAGEBUTTON}
accessibilityLabel={translate('accessibilityHints.viewAttachment')}
>
{receiptImageComponent}
</PressableWithoutFocus>
)}
</AttachmentModal>
)}
</ShowContextMenuContext.Consumer>
);
Expand Down

0 comments on commit 3358b03

Please sign in to comment.