Merge pull request #30049 from bernhardoj/fix/26887-actions-draft

Expensify · Oct 25, 2023 · 9a85f80 · 9a85f80
2 parents 1e22431 + 9edcd3b
commit 9a85f80
Show file tree

Hide file tree

Showing 9 changed files with 158 additions and 5 deletions.
diff --git a/src/ONYXKEYS.ts b/src/ONYXKEYS.ts
@@ -392,7 +392,7 @@ type OnyxValues = {
     [ONYXKEYS.COLLECTION.REPORT]: OnyxTypes.Report;
     [ONYXKEYS.COLLECTION.REPORT_METADATA]: OnyxTypes.ReportMetadata;
     [ONYXKEYS.COLLECTION.REPORT_ACTIONS]: OnyxTypes.ReportActions;
-    [ONYXKEYS.COLLECTION.REPORT_ACTIONS_DRAFTS]: string;
+    [ONYXKEYS.COLLECTION.REPORT_ACTIONS_DRAFTS]: OnyxTypes.ReportActionsDrafts;
     [ONYXKEYS.COLLECTION.REPORT_ACTIONS_REACTIONS]: OnyxTypes.ReportActionReactions;
     [ONYXKEYS.COLLECTION.REPORT_DRAFT_COMMENT]: string;
     [ONYXKEYS.COLLECTION.REPORT_DRAFT_COMMENT_NUMBER_OF_LINES]: number;

diff --git a/src/libs/actions/Policy.js b/src/libs/actions/Policy.js
@@ -33,6 +33,7 @@ Onyx.connect({
             _.each(policyReports, ({reportID}) => {
                 cleanUpMergeQueries[`${ONYXKEYS.COLLECTION.REPORT}${reportID}`] = {hasDraft: false};
                 cleanUpSetQueries[`${ONYXKEYS.COLLECTION.REPORT_DRAFT_COMMENT}${reportID}`] = null;
+                cleanUpSetQueries[`${ONYXKEYS.COLLECTION.REPORT_ACTIONS_DRAFTS}${reportID}`] = null;
             });
             Onyx.mergeCollection(ONYXKEYS.COLLECTION.REPORT, cleanUpMergeQueries);
             Onyx.multiSet(cleanUpSetQueries);
@@ -116,6 +117,12 @@ function deleteWorkspace(policyID, reports, policyName) {
             },
         })),
 
+        ..._.map(reports, ({reportID}) => ({
+            onyxMethod: Onyx.METHOD.SET,
+            key: `${ONYXKEYS.COLLECTION.REPORT_ACTIONS_DRAFTS}${reportID}`,
+            value: null,
+        })),
+
         // Add closed actions to all chat reports linked to this policy
         ..._.map(reports, ({reportID, ownerAccountID}) => {
             // Announce & admin chats have FAKE owners, but workspace chats w/ users do have owners.

diff --git a/src/libs/actions/Report.js b/src/libs/actions/Report.js
@@ -1339,7 +1339,7 @@ function editReportComment(reportID, originalReportAction, textForNewComment) {
  */
 function saveReportActionDraft(reportID, reportAction, draftMessage) {
     const originalReportID = ReportUtils.getOriginalReportID(reportID, reportAction);
-    Onyx.set(`${ONYXKEYS.COLLECTION.REPORT_ACTIONS_DRAFTS}${originalReportID}_${reportAction.reportActionID}`, draftMessage);
+    Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT_ACTIONS_DRAFTS}${originalReportID}`, {[reportAction.reportActionID]: draftMessage});
 }
 
 /**

diff --git a/src/libs/migrateOnyx.js b/src/libs/migrateOnyx.js
@@ -2,14 +2,15 @@ import _ from 'underscore';
 import Log from './Log';
 import PersonalDetailsByAccountID from './migrations/PersonalDetailsByAccountID';
 import RenameReceiptFilename from './migrations/RenameReceiptFilename';
+import KeyReportActionsDraftByReportActionID from './migrations/KeyReportActionsDraftByReportActionID';
 
 export default function () {
     const startTime = Date.now();
     Log.info('[Migrate Onyx] start');
 
     return new Promise((resolve) => {
         // Add all migrations to an array so they are executed in order
-        const migrationPromises = [PersonalDetailsByAccountID, RenameReceiptFilename];
+        const migrationPromises = [PersonalDetailsByAccountID, RenameReceiptFilename, KeyReportActionsDraftByReportActionID];
 
         // 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.

diff --git a/src/libs/migrations/KeyReportActionsDraftByReportActionID.js b/src/libs/migrations/KeyReportActionsDraftByReportActionID.js
@@ -0,0 +1,60 @@
+import _ from 'underscore';
+import Onyx from 'react-native-onyx';
+import Log from '../Log';
+import ONYXKEYS from '../../ONYXKEYS';
+
+/**
+ * This migration updates reportActionsDrafts data to be keyed by reportActionID.
+ *
+ * Before: reportActionsDrafts_reportID_reportActionID: value
+ * After: reportActionsDrafts_reportID: {[reportActionID]: value}
+ *
+ * @returns {Promise}
+ */
+export default function () {
+    return new Promise((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 KeyReportActionsDraftByReportActionID because there were no reportActionsDrafts');
+                    return resolve();
+                }
+
+                const newReportActionsDrafts = {};
+                _.each(allReportActionsDrafts, (reportActionDraft, onyxKey) => {
+                    if (!_.isString(reportActionDraft)) {
+                        return;
+                    }
+                    newReportActionsDrafts[onyxKey] = null;
+
+                    if (_.isEmpty(reportActionDraft)) {
+                        return;
+                    }
+
+                    const reportActionID = onyxKey.split('_').pop();
+                    const newOnyxKey = onyxKey.replace(`_${reportActionID}`, '');
+
+                    // If newReportActionsDrafts[newOnyxKey] isn't set, fall back on the migrated draft if there is one
+                    const currentActionsDrafts = newReportActionsDrafts[newOnyxKey] || allReportActionsDrafts[newOnyxKey];
+                    newReportActionsDrafts[newOnyxKey] = {
+                        ...currentActionsDrafts,
+                        [reportActionID]: reportActionDraft,
+                    };
+                });
+
+                if (_.isEmpty(newReportActionsDrafts)) {
+                    Log.info('[Migrate Onyx] Skipped migration KeyReportActionsDraftByReportActionID because there are no actions drafts to migrate');
+                    return resolve();
+                }
+
+                Log.info(`[Migrate Onyx] Re-keying reportActionsDrafts by reportActionID for ${_.keys(newReportActionsDrafts).length} actions drafts`);
+                // eslint-disable-next-line rulesdir/prefer-actions-set-data
+                return Onyx.multiSet(newReportActionsDrafts).then(resolve);
+            },
+        });
+    });
+}
diff --git a/src/pages/home/report/ReportActionItem.js b/src/pages/home/report/ReportActionItem.js
@@ -725,8 +725,8 @@ export default compose(
         propName: 'draftMessage',
         transformValue: (drafts, props) => {
             const originalReportID = ReportUtils.getOriginalReportID(props.report.reportID, props.action);
-            const draftKey = `${ONYXKEYS.COLLECTION.REPORT_ACTIONS_DRAFTS}${originalReportID}_${props.action.reportActionID}`;
-            return lodashGet(drafts, draftKey, '');
+            const draftKey = `${ONYXKEYS.COLLECTION.REPORT_ACTIONS_DRAFTS}${originalReportID}`;
+            return lodashGet(drafts, [draftKey, props.action.reportActionID], '');
         },
     }),
     withOnyx({

diff --git a/src/types/onyx/ReportActionsDrafts.ts b/src/types/onyx/ReportActionsDrafts.ts
@@ -0,0 +1,3 @@
+type ReportActionsDrafts = Record<string, string>;
+
+export default ReportActionsDrafts;
diff --git a/src/types/onyx/index.ts b/src/types/onyx/index.ts
@@ -38,6 +38,7 @@ import PolicyCategory from './PolicyCategory';
 import Report from './Report';
 import ReportMetadata from './ReportMetadata';
 import ReportAction, {ReportActions} from './ReportAction';
+import ReportActionsDrafts from './ReportActionsDrafts';
 import ReportActionReactions from './ReportActionReactions';
 import SecurityGroup from './SecurityGroup';
 import Transaction from './Transaction';
@@ -89,6 +90,7 @@ export type {
     ReportMetadata,
     ReportAction,
     ReportActions,
+    ReportActionsDrafts,
     ReportActionReactions,
     SecurityGroup,
     Transaction,

diff --git a/tests/unit/MigrationTest.js b/tests/unit/MigrationTest.js
@@ -4,6 +4,7 @@ import waitForBatchedUpdates from '../utils/waitForBatchedUpdates';
 import Log from '../../src/libs/Log';
 import PersonalDetailsByAccountID from '../../src/libs/migrations/PersonalDetailsByAccountID';
 import CheckForPreviousReportActionID from '../../src/libs/migrations/CheckForPreviousReportActionID';
+import KeyReportActionsDraftByReportActionID from '../../src/libs/migrations/KeyReportActionsDraftByReportActionID';
 import ONYXKEYS from '../../src/ONYXKEYS';
 
 jest.mock('../../src/libs/getPlatform');
@@ -622,4 +623,83 @@ describe('Migrations', () => {
                     });
                 }));
     });
+
+    describe('KeyReportActionsDraftByReportActionID', () => {
+        it("Should work even if there's no reportActionsDrafts data in Onyx", () =>
+            KeyReportActionsDraftByReportActionID().then(() =>
+                expect(LogSpy).toHaveBeenCalledWith('[Migrate Onyx] Skipped migration KeyReportActionsDraftByReportActionID because there were no reportActionsDrafts'),
+            ));
+
+        it('Should move individual draft to a draft collection of report', () =>
+            Onyx.multiSet({
+                [`${ONYXKEYS.COLLECTION.REPORT_ACTIONS_DRAFTS}1_1`]: 'a',
+                [`${ONYXKEYS.COLLECTION.REPORT_ACTIONS_DRAFTS}1_2`]: 'b',
+                [`${ONYXKEYS.COLLECTION.REPORT_ACTIONS_DRAFTS}2`]: {3: 'c'},
+                [`${ONYXKEYS.COLLECTION.REPORT_ACTIONS_DRAFTS}2_4`]: 'd',
+            })
+                .then(KeyReportActionsDraftByReportActionID)
+                .then(() => {
+                    const connectionID = Onyx.connect({
+                        key: ONYXKEYS.COLLECTION.REPORT_ACTIONS_DRAFTS,
+                        waitForCollectionCallback: true,
+                        callback: (allReportActionsDrafts) => {
+                            Onyx.disconnect(connectionID);
+                            const expectedReportActionDraft1 = {
+                                1: 'a',
+                                2: 'b',
+                            };
+                            const expectedReportActionDraft2 = {
+                                3: 'c',
+                                4: 'd',
+                            };
+                            expect(allReportActionsDrafts[`${ONYXKEYS.COLLECTION.REPORT_ACTIONS_DRAFTS}1_1`]).toBeUndefined();
+                            expect(allReportActionsDrafts[`${ONYXKEYS.COLLECTION.REPORT_ACTIONS_DRAFTS}1_2`]).toBeUndefined();
+                            expect(allReportActionsDrafts[`${ONYXKEYS.COLLECTION.REPORT_ACTIONS_DRAFTS}2_4`]).toBeUndefined();
+                            expect(allReportActionsDrafts[`${ONYXKEYS.COLLECTION.REPORT_ACTIONS_DRAFTS}1`]).toMatchObject(expectedReportActionDraft1);
+                            expect(allReportActionsDrafts[`${ONYXKEYS.COLLECTION.REPORT_ACTIONS_DRAFTS}2`]).toMatchObject(expectedReportActionDraft2);
+                        },
+                    });
+                }));
+
+        it('Should skip if nothing to migrate', () =>
+            Onyx.multiSet({
+                [`${ONYXKEYS.COLLECTION.REPORT_ACTIONS_DRAFTS}1_1`]: null,
+                [`${ONYXKEYS.COLLECTION.REPORT_ACTIONS_DRAFTS}1_2`]: null,
+                [`${ONYXKEYS.COLLECTION.REPORT_ACTIONS_DRAFTS}2`]: {},
+                [`${ONYXKEYS.COLLECTION.REPORT_ACTIONS_DRAFTS}2_4`]: null,
+            })
+                .then(KeyReportActionsDraftByReportActionID)
+                .then(() => {
+                    expect(LogSpy).toHaveBeenCalledWith('[Migrate Onyx] Skipped migration KeyReportActionsDraftByReportActionID because there are no actions drafts to migrate');
+                    const connectionID = Onyx.connect({
+                        key: ONYXKEYS.COLLECTION.REPORT_ACTIONS_DRAFTS,
+                        waitForCollectionCallback: true,
+                        callback: (allReportActions) => {
+                            Onyx.disconnect(connectionID);
+                            const expectedReportActionDraft = {};
+                            expect(allReportActions[`${ONYXKEYS.COLLECTION.REPORT_ACTIONS_DRAFTS}1_1`]).toBeUndefined();
+                            expect(allReportActions[`${ONYXKEYS.COLLECTION.REPORT_ACTIONS_DRAFTS}1_2`]).toBeUndefined();
+                            expect(allReportActions[`${ONYXKEYS.COLLECTION.REPORT_ACTIONS_DRAFTS}2_4`]).toBeUndefined();
+                            expect(allReportActions[`${ONYXKEYS.COLLECTION.REPORT_ACTIONS_DRAFTS}2`]).toMatchObject(expectedReportActionDraft);
+                        },
+                    });
+                }));
+
+        it("Shouldn't move empty individual draft to a draft collection of report", () =>
+            Onyx.multiSet({
+                [`${ONYXKEYS.COLLECTION.REPORT_ACTIONS_DRAFTS}1_1`]: '',
+                [`${ONYXKEYS.COLLECTION.REPORT_ACTIONS_DRAFTS}1`]: {},
+            })
+                .then(KeyReportActionsDraftByReportActionID)
+                .then(() => {
+                    const connectionID = Onyx.connect({
+                        key: ONYXKEYS.COLLECTION.REPORT_ACTIONS_DRAFTS,
+                        waitForCollectionCallback: true,
+                        callback: (allReportActionsDrafts) => {
+                            Onyx.disconnect(connectionID);
+                            expect(allReportActionsDrafts[`${ONYXKEYS.COLLECTION.REPORT_ACTIONS_DRAFTS}1_1`]).toBeUndefined();
+                        },
+                    });
+                }));
+    });
 });