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

Fix the current issue and regression #32647

Merged
merged 6 commits into from
Dec 11, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
36 changes: 17 additions & 19 deletions src/components/AttachmentModal.js
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
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
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({
Copy link
Contributor

Choose a reason for hiding this comment

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

Please remove this line from above too

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@shubham1206agra Updated.

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
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
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
Loading