Skip to content

Commit

Permalink
Merge pull request Expensify#33500 from samh-nl/fix/issue-25488
Browse files Browse the repository at this point in the history
Allow empty draft messages
  • Loading branch information
jasperhuangg authored Jan 2, 2024
2 parents cb4fc64 + 97a1be4 commit b3fe02a
Show file tree
Hide file tree
Showing 14 changed files with 132 additions and 37 deletions.
2 changes: 1 addition & 1 deletion src/components/LHNOptionsList/OptionRowLHN.js
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,7 @@ function OptionRowLHN(props) {
props.reportID,
'0',
props.reportID,
'',
undefined,
() => {},
() => setIsContextMenuActive(false),
false,
Expand Down
2 changes: 1 addition & 1 deletion src/components/ShowContextMenuContext.js
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ function showContextMenuForReport(event, anchor, reportID, action, checkIfContex
reportID,
action.reportActionID,
ReportUtils.getOriginalReportID(reportID, action),
'',
undefined,
checkIfContextMenuActive,
checkIfContextMenuActive,
isArchivedRoom,
Expand Down
9 changes: 8 additions & 1 deletion src/libs/actions/Report.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1355,10 +1355,16 @@ function editReportComment(reportID: string, originalReportAction: OnyxEntry<Rep
API.write('UpdateComment', parameters, {optimisticData, successData, failureData});
}

/** Deletes the draft for a comment report action. */
function deleteReportActionDraft(reportID: string, reportAction: ReportAction) {
const originalReportID = ReportUtils.getOriginalReportID(reportID, reportAction);
Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT_ACTIONS_DRAFTS}${originalReportID}`, {[reportAction.reportActionID]: null});
}

/** Saves the draft for a comment report action. This will put the comment into "edit mode" */
function saveReportActionDraft(reportID: string, reportAction: ReportAction, draftMessage: string) {
const originalReportID = ReportUtils.getOriginalReportID(reportID, reportAction);
Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT_ACTIONS_DRAFTS}${originalReportID}`, {[reportAction.reportActionID]: draftMessage});
Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT_ACTIONS_DRAFTS}${originalReportID}`, {[reportAction.reportActionID]: {message: draftMessage}});
}

/** Saves the number of lines for the report action draft */
Expand Down Expand Up @@ -2557,6 +2563,7 @@ export {
togglePinnedState,
editReportComment,
handleUserDeletedLinksInHtml,
deleteReportActionDraft,
saveReportActionDraft,
saveReportActionDraftNumberOfLines,
deleteReportComment,
Expand Down
3 changes: 2 additions & 1 deletion src/libs/migrateOnyx.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import _ from 'underscore';
import Log from './Log';
import KeyReportActionsDraftByReportActionID from './migrations/KeyReportActionsDraftByReportActionID';
import PersonalDetailsByAccountID from './migrations/PersonalDetailsByAccountID';
import RemoveEmptyReportActionsDrafts from './migrations/RemoveEmptyReportActionsDrafts';
import RenameReceiptFilename from './migrations/RenameReceiptFilename';
import TransactionBackupsToCollection from './migrations/TransactionBackupsToCollection';

Expand All @@ -11,7 +12,7 @@ export default function () {

return new Promise((resolve) => {
// Add all migrations to an array so they are executed in order
const migrationPromises = [PersonalDetailsByAccountID, RenameReceiptFilename, KeyReportActionsDraftByReportActionID, TransactionBackupsToCollection];
const migrationPromises = [PersonalDetailsByAccountID, RenameReceiptFilename, KeyReportActionsDraftByReportActionID, TransactionBackupsToCollection, RemoveEmptyReportActionsDrafts];

// Reduce all promises down to a single promise. All promises run in a linear fashion, waiting for the
// previous promise to finish before moving onto the next one.
Expand Down
76 changes: 76 additions & 0 deletions src/libs/migrations/RemoveEmptyReportActionsDrafts.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,76 @@
import _ from 'lodash';
import Onyx, {OnyxEntry} from 'react-native-onyx';
import Log from '@libs/Log';
import ONYXKEYS from '@src/ONYXKEYS';
import {ReportActionsDraft, ReportActionsDrafts} from '@src/types/onyx';
import {isEmptyObject} from '@src/types/utils/EmptyObject';

type ReportActionsDraftsKey = `${typeof ONYXKEYS.COLLECTION.REPORT_ACTIONS_DRAFTS}${string}`;

/**
* This migration removes empty drafts from reportActionsDrafts, which was previously used to mark a draft as being non-existent (e.g. upon cancel).
*/
export default function (): Promise<void> {
return new Promise<void>((resolve) => {
const connectionID = Onyx.connect({
key: ONYXKEYS.COLLECTION.REPORT_ACTIONS_DRAFTS,
waitForCollectionCallback: true,
callback: (allReportActionsDrafts) => {
Onyx.disconnect(connectionID);

if (!allReportActionsDrafts) {
Log.info('[Migrate Onyx] Skipped migration RemoveEmptyReportActionsDrafts because there were no reportActionsDrafts');
return resolve();
}

const newReportActionsDrafts: Record<ReportActionsDraftsKey, OnyxEntry<ReportActionsDrafts>> = {};
Object.entries(allReportActionsDrafts).forEach(([onyxKey, reportActionDrafts]) => {
const newReportActionsDraftsForReport: Record<string, ReportActionsDraft> = {};

// Whether there is at least one draft in this report that has to be migrated
let hasUnmigratedDraft = false;

if (reportActionDrafts) {
Object.entries(reportActionDrafts).forEach(([reportActionID, reportActionDraft]) => {
// If the draft is a string, it means it hasn't been migrated yet
if (typeof reportActionDraft === 'string') {
hasUnmigratedDraft = true;
Log.info(`[Migrate Onyx] Migrating draft for report action ${reportActionID}`);

if (_.isEmpty(reportActionDraft)) {
Log.info(`[Migrate Onyx] Removing draft for report action ${reportActionID}`);
return;
}

newReportActionsDraftsForReport[reportActionID] = {message: reportActionDraft};
} else {
// We've already migrated this draft, so keep the existing value
newReportActionsDraftsForReport[reportActionID] = reportActionDraft;
}
});
}

if (isEmptyObject(newReportActionsDraftsForReport)) {
Log.info('[Migrate Onyx] NO REMAINING');
// Clear if there are no drafts remaining
newReportActionsDrafts[onyxKey as ReportActionsDraftsKey] = null;
} else if (hasUnmigratedDraft) {
// Only migrate if there are unmigrated drafts, there's no need to overwrite this onyx key with the same data
newReportActionsDrafts[onyxKey as ReportActionsDraftsKey] = newReportActionsDraftsForReport;
}
});

if (isEmptyObject(newReportActionsDrafts)) {
Log.info('[Migrate Onyx] Skipped migration RemoveEmptyReportActionsDrafts because there are no actions drafts to migrate');
return resolve();
}

Log.info(`[Migrate Onyx] Updating drafts for ${Object.keys(newReportActionsDrafts).length} reports`);
Onyx.multiSet(newReportActionsDrafts).then(() => {
Log.info('[Migrate Onyx] Ran migration RemoveEmptyReportActionsDrafts successfully');
resolve();
});
},
});
});
}
8 changes: 7 additions & 1 deletion src/pages/home/report/ContextMenu/ContextMenuActions.js
Original file line number Diff line number Diff line change
Expand Up @@ -380,7 +380,13 @@ export default [
Navigation.navigate(ROUTES.REPORT_WITH_ID.getRoute(childReportID));
return;
}
const editAction = () => Report.saveReportActionDraft(reportID, reportAction, _.isEmpty(draftMessage) ? getActionText(reportAction) : '');
const editAction = () => {
if (_.isUndefined(draftMessage)) {
Report.saveReportActionDraft(reportID, reportAction, getActionText(reportAction));
} else {
Report.deleteReportActionDraft(reportID, reportAction);
}
};

if (closePopover) {
// Hide popover, then call editAction
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ function PopoverReportActionContextMenu(_props, ref) {
const reportActionIDRef = useRef('0');
const originalReportIDRef = useRef('0');
const selectionRef = useRef('');
const reportActionDraftMessageRef = useRef('');
const reportActionDraftMessageRef = useRef(undefined);

const cursorRelativePosition = useRef({
horizontal: 0,
Expand Down Expand Up @@ -226,7 +226,7 @@ function PopoverReportActionContextMenu(_props, ref) {
}

selectionRef.current = '';
reportActionDraftMessageRef.current = '';
reportActionDraftMessageRef.current = undefined;
setIsPopoverVisible(false);
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ function showContextMenu(
reportID = '0',
reportActionID = '0',
originalReportID = '0',
draftMessage = '',
draftMessage = undefined,
onShow = () => {},
onHide = () => {},
isArchivedRoom = false,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ const defaultProps = {
isMini: false,
isVisible: false,
selection: '',
draftMessage: '',
draftMessage: undefined,
};

export {propTypes, defaultProps};
34 changes: 17 additions & 17 deletions src/pages/home/report/ReportActionItem.js
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ const propTypes = {
};

const defaultProps = {
draftMessage: '',
draftMessage: undefined,
preferredSkinTone: CONST.EMOJI_DEFAULT_SKIN_TONE,
emojiReactions: {},
shouldShowSubscriptAvatar: false,
Expand Down Expand Up @@ -197,7 +197,7 @@ function ReportActionItem(props) {
}, [isDeletedParentAction, props.action.reportActionID]);

useEffect(() => {
if (prevDraftMessage || !props.draftMessage) {
if (!_.isUndefined(prevDraftMessage) || _.isUndefined(props.draftMessage)) {
return;
}

Expand All @@ -219,10 +219,10 @@ function ReportActionItem(props) {
}, [props.action, props.report.reportID]);

useEffect(() => {
if (!props.draftMessage || !ReportActionsUtils.isDeletedAction(props.action)) {
if (_.isUndefined(props.draftMessage) || !ReportActionsUtils.isDeletedAction(props.action)) {
return;
}
Report.saveReportActionDraft(props.report.reportID, props.action, '');
Report.deleteReportActionDraft(props.report.reportID, props.action);
}, [props.draftMessage, props.action, props.report.reportID]);

// Hide the message if it is being moderated for a higher offense, or is hidden by a moderator
Expand Down Expand Up @@ -260,7 +260,7 @@ function ReportActionItem(props) {
const showPopover = useCallback(
(event) => {
// Block menu on the message being Edited or if the report action item has errors
if (props.draftMessage || !_.isEmpty(props.action.errors)) {
if (!_.isUndefined(props.draftMessage) || !_.isEmpty(props.action.errors)) {
return;
}

Expand Down Expand Up @@ -426,7 +426,7 @@ function ReportActionItem(props) {
const hasBeenFlagged = !_.contains([CONST.MODERATION.MODERATOR_DECISION_APPROVED, CONST.MODERATION.MODERATOR_DECISION_PENDING], moderationDecision);
children = (
<ShowContextMenuContext.Provider value={contextValue}>
{!props.draftMessage ? (
{_.isUndefined(props.draftMessage) ? (
<View style={props.displayAsGroup && hasBeenFlagged ? styles.blockquote : {}}>
<ReportActionItemMessage
reportID={props.report.reportID}
Expand Down Expand Up @@ -485,13 +485,13 @@ function ReportActionItem(props) {

const shouldDisplayThreadReplies = hasReplies && props.action.childCommenterCount && !ReportUtils.isThreadFirstChat(props.action, props.report.reportID);
const oldestFourAccountIDs = _.map(lodashGet(props.action, 'childOldestFourAccountIDs', '').split(','), (accountID) => Number(accountID));
const draftMessageRightAlign = props.draftMessage ? styles.chatItemReactionsDraftRight : {};
const draftMessageRightAlign = !_.isUndefined(props.draftMessage) ? styles.chatItemReactionsDraftRight : {};

return (
<>
{children}
{Permissions.canUseLinkPreviews() && !isHidden && !_.isEmpty(props.action.linkMetadata) && (
<View style={props.draftMessage ? styles.chatItemReactionsDraftRight : {}}>
<View style={!_.isUndefined(props.draftMessage) ? styles.chatItemReactionsDraftRight : {}}>
<LinkPreviewer linkMetadata={_.filter(props.action.linkMetadata, (item) => !_.isEmpty(item))} />
</View>
)}
Expand Down Expand Up @@ -542,15 +542,15 @@ function ReportActionItem(props) {
const renderReportActionItem = (hovered, isWhisper, hasErrors) => {
const content = renderItemContent(hovered || isContextMenuActive, isWhisper, hasErrors);

if (props.draftMessage) {
if (!_.isUndefined(props.draftMessage)) {
return <ReportActionItemDraft>{content}</ReportActionItemDraft>;
}

if (!props.displayAsGroup) {
return (
<ReportActionItemSingle
action={props.action}
showHeader={!props.draftMessage}
showHeader={_.isUndefined(props.draftMessage)}
wrapperStyle={isWhisper ? styles.pt1 : {}}
shouldShowSubscriptAvatar={props.shouldShowSubscriptAvatar}
report={props.report}
Expand Down Expand Up @@ -586,7 +586,7 @@ function ReportActionItem(props) {
<View style={[StyleUtils.getReportWelcomeTopMarginStyle(props.isSmallScreenWidth)]}>
<ReportActionItemSingle
action={parentReportAction}
showHeader={!props.draftMessage}
showHeader={_.isUndefined(props.draftMessage)}
report={props.report}
>
<RenderHTML html={`<comment>${props.translate('parentReportAction.deletedTask')}</comment>`} />
Expand Down Expand Up @@ -664,13 +664,13 @@ function ReportActionItem(props) {
onPressIn={() => props.isSmallScreenWidth && DeviceCapabilities.canUseTouchScreen() && ControlSelection.block()}
onPressOut={() => ControlSelection.unblock()}
onSecondaryInteraction={showPopover}
preventDefaultContextMenu={!props.draftMessage && !hasErrors}
preventDefaultContextMenu={_.isUndefined(props.draftMessage) && !hasErrors}
withoutFocusOnSecondaryInteraction
accessibilityLabel={props.translate('accessibilityHints.chatMessage')}
>
<Hoverable
shouldHandleScroll
disabled={Boolean(props.draftMessage)}
disabled={!_.isUndefined(props.draftMessage)}
>
{(hovered) => (
<View style={highlightedBackgroundColorIfNeeded}>
Expand All @@ -681,14 +681,14 @@ function ReportActionItem(props) {
originalReportID={originalReportID}
isArchivedRoom={ReportUtils.isArchivedRoom(props.report)}
displayAsGroup={props.displayAsGroup}
isVisible={hovered && !props.draftMessage && !hasErrors}
isVisible={hovered && _.isUndefined(props.draftMessage) && !hasErrors}
draftMessage={props.draftMessage}
isChronosReport={ReportUtils.chatIncludesChronos(originalReport)}
/>
<View style={StyleUtils.getReportActionItemStyle(hovered || isWhisper || isContextMenuActive || props.draftMessage)}>
<View style={StyleUtils.getReportActionItemStyle(hovered || isWhisper || isContextMenuActive || !_.isUndefined(props.draftMessage))}>
<OfflineWithFeedback
onClose={() => ReportActions.clearReportActionErrors(props.report.reportID, props.action)}
pendingAction={props.draftMessage ? null : props.action.pendingAction}
pendingAction={!_.isUndefined(props.draftMessage) ? null : props.action.pendingAction}
shouldHideOnDelete={!ReportActionsUtils.isThreadParentMessage(props.action, props.report.reportID)}
errors={props.action.errors}
errorRowStyles={[styles.ml10, styles.mr2]}
Expand Down Expand Up @@ -744,7 +744,7 @@ export default compose(
transformValue: (drafts, props) => {
const originalReportID = ReportUtils.getOriginalReportID(props.report.reportID, props.action);
const draftKey = `${ONYXKEYS.COLLECTION.REPORT_ACTIONS_DRAFTS}${originalReportID}`;
return lodashGet(drafts, [draftKey, props.action.reportActionID], '');
return lodashGet(drafts, [draftKey, props.action.reportActionID, 'message']);
},
}),
withOnyx({
Expand Down
14 changes: 4 additions & 10 deletions src/pages/home/report/ReportActionItemMessageEdit.js
Original file line number Diff line number Diff line change
Expand Up @@ -270,16 +270,10 @@ function ReportActionItemMessageEdit(props) {

draftRef.current = newDraft;

// This component is rendered only when draft is set to a non-empty string. In order to prevent component
// unmount when user deletes content of textarea, we set previous message instead of empty string.
if (newDraft.trim().length > 0) {
// We want to escape the draft message to differentiate the HTML from the report action and the HTML the user drafted.
debouncedSaveDraft(_.escape(newDraft));
} else {
debouncedSaveDraft(props.action.message[0].html);
}
// We want to escape the draft message to differentiate the HTML from the report action and the HTML the user drafted.
debouncedSaveDraft(_.escape(newDraft));
},
[props.action.message, debouncedSaveDraft, debouncedUpdateFrequentlyUsedEmojis, props.preferredSkinTone, preferredLocale, selection.end],
[debouncedSaveDraft, debouncedUpdateFrequentlyUsedEmojis, props.preferredSkinTone, preferredLocale, selection.end],
);

useEffect(() => {
Expand All @@ -292,7 +286,7 @@ function ReportActionItemMessageEdit(props) {
*/
const deleteDraft = useCallback(() => {
debouncedSaveDraft.cancel();
Report.saveReportActionDraft(props.reportID, props.action, '');
Report.deleteReportActionDraft(props.reportID, props.action);

if (isActive()) {
ReportActionComposeFocusManager.clear();
Expand Down
7 changes: 7 additions & 0 deletions src/types/onyx/ReportActionsDraft.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
type ReportActionsDraft =
| {
message: string;
}
| string;

export default ReportActionsDraft;
4 changes: 3 additions & 1 deletion src/types/onyx/ReportActionsDrafts.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
type ReportActionsDrafts = Record<string, string>;
import ReportActionsDraft from './ReportActionsDraft';

type ReportActionsDrafts = Record<string, ReportActionsDraft>;

export default ReportActionsDrafts;
2 changes: 2 additions & 0 deletions src/types/onyx/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ import ReimbursementAccountDraft from './ReimbursementAccountDraft';
import Report from './Report';
import ReportAction, {ReportActions} from './ReportAction';
import ReportActionReactions from './ReportActionReactions';
import ReportActionsDraft from './ReportActionsDraft';
import ReportActionsDrafts from './ReportActionsDrafts';
import ReportMetadata from './ReportMetadata';
import ReportNextStep from './ReportNextStep';
Expand Down Expand Up @@ -105,6 +106,7 @@ export type {
ReportAction,
ReportActionReactions,
ReportActions,
ReportActionsDraft,
ReportActionsDrafts,
ReportMetadata,
ReportNextStep,
Expand Down

0 comments on commit b3fe02a

Please sign in to comment.