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

ReportAction text for transactions edited in OldDot #30603

Merged
merged 52 commits into from
Dec 19, 2023
Merged
Changes from 3 commits
Commits
Show all changes
52 commits
Select commit Hold shift + click to select a range
94e2dee
ReportAction text for transactions edited in OldDot
cristipaval Oct 30, 2023
6e5d6b0
Make Lint happy.
cristipaval Oct 30, 2023
a7f3f38
Run prettier.
cristipaval Oct 30, 2023
a445ea6
Merge remote-tracking branch 'origin/main' into cristi_modified-expen…
cristipaval Nov 7, 2023
a0a856c
Improve message for MODIFIEDEXPENSE action
cristipaval Nov 7, 2023
960d00a
Add a safety check
cristipaval Nov 7, 2023
a4e82a8
Improve MODIFIEDEXPENSE action message
cristipaval Nov 7, 2023
b9b7ef4
Improve MODIFIEDEXPENSE action message
cristipaval Nov 7, 2023
5190539
Remove redundancy.
cristipaval Nov 7, 2023
b5518be
Update function doc
cristipaval Nov 8, 2023
be978f9
Create reusable function
cristipaval Nov 8, 2023
b31ba92
Run prettier
cristipaval Nov 8, 2023
376944c
Fix lint.
cristipaval Nov 8, 2023
b9f4243
Update src/libs/ReportUtils.js
cristipaval Nov 8, 2023
16df2d9
Merge remote-tracking branch 'origin/main' into cristi_modified-expen…
cristipaval Nov 20, 2023
bab1de4
Move logic in a separate unit
cristipaval Nov 20, 2023
4ea44ee
Move policy tags logic to PolicyUtils
cristipaval Nov 20, 2023
b9229a7
Better naming
cristipaval Nov 20, 2023
72714ad
Dry up code.
cristipaval Nov 20, 2023
18d1bf0
Run prettier
cristipaval Nov 20, 2023
5444abd
Fix lint errors
cristipaval Nov 20, 2023
371f138
Make it a ts file.
cristipaval Nov 21, 2023
ceaa915
Migrate to TS
cristipaval Nov 21, 2023
3bec4fd
Make Lint happy.
cristipaval Nov 21, 2023
b350c6b
Update src/libs/ModifiedExpenseMessage.ts
cristipaval Nov 28, 2023
03931a2
Update src/libs/ModifiedExpenseMessage.ts
cristipaval Nov 28, 2023
4e9a118
Update src/libs/ModifiedExpenseMessage.ts
cristipaval Nov 28, 2023
d855c90
Merge remote-tracking branch 'origin/main' into cristi_modified-expen…
cristipaval Nov 29, 2023
41f8a94
Resolve conflicts after merge.
cristipaval Nov 29, 2023
ceff5c1
Move policyTags to ModifiedExpenseMessage
cristipaval Nov 29, 2023
57f0837
Remove unused function.
cristipaval Nov 29, 2023
7dd7afb
Make lint happy.
cristipaval Nov 29, 2023
1cb7f47
Better checks.
cristipaval Nov 29, 2023
388962f
Update src/libs/ReportUtils.ts
cristipaval Nov 29, 2023
9cc7637
Update src/libs/ModifiedExpenseMessage.ts
cristipaval Nov 29, 2023
60a1493
Improve function doc.
cristipaval Nov 29, 2023
8dd3c99
Add return type declaration
cristipaval Nov 29, 2023
4606a46
Merge remote-tracking branch 'origin/main' into cristi_modified-expen…
cristipaval Nov 30, 2023
0d83c68
Merge remote-tracking branch 'origin/main' into cristi_modified-expen…
cristipaval Dec 13, 2023
a136b74
Fix conflicts after merge
cristipaval Dec 13, 2023
262cb52
RUn prettier
cristipaval Dec 13, 2023
cd28c61
Performance test for ModifiedExpenseMessage
cristipaval Dec 13, 2023
5219b6a
Fix typecheck failures
cristipaval Dec 13, 2023
df90850
Unit test for ModifiedExpenseMessage
cristipaval Dec 14, 2023
db1f935
More tests.
cristipaval Dec 14, 2023
8babd0e
One more test
cristipaval Dec 14, 2023
02c09f2
One more test
cristipaval Dec 14, 2023
a6a28a6
3 more tests
cristipaval Dec 14, 2023
27945e8
3 more tests
cristipaval Dec 14, 2023
8aad9bd
Make lint happy an run prettier
cristipaval Dec 14, 2023
d22c617
Merge remote-tracking branch 'origin/main' into cristi_modified-expen…
cristipaval Dec 18, 2023
13f7b9c
Add changes from main
cristipaval Dec 18, 2023
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
31 changes: 24 additions & 7 deletions src/libs/ReportUtils.js
Original file line number Diff line number Diff line change
Expand Up @@ -1844,6 +1844,8 @@ function getModifiedExpenseMessage(reportAction) {
return Localize.translateLocal('iou.changedTheRequest');
}

const messageFragments = [];

const hasModifiedAmount =
_.has(reportActionOriginalMessage, 'oldAmount') &&
_.has(reportActionOriginalMessage, 'oldCurrency') &&
Expand All @@ -1864,40 +1866,55 @@ function getModifiedExpenseMessage(reportAction) {
return getProperSchemaForModifiedDistanceMessage(reportActionOriginalMessage.merchant, reportActionOriginalMessage.oldMerchant, amount, oldAmount);
}

return getProperSchemaForModifiedExpenseMessage(amount, oldAmount, Localize.translateLocal('iou.amount'), false);
messageFragments.push(getProperSchemaForModifiedExpenseMessage(amount, oldAmount, Localize.translateLocal('iou.amount'), false));
}

const hasModifiedComment = _.has(reportActionOriginalMessage, 'oldComment') && _.has(reportActionOriginalMessage, 'newComment');
if (hasModifiedComment) {
return getProperSchemaForModifiedExpenseMessage(reportActionOriginalMessage.newComment, reportActionOriginalMessage.oldComment, Localize.translateLocal('common.description'), true);
messageFragments.push(
getProperSchemaForModifiedExpenseMessage(reportActionOriginalMessage.newComment, reportActionOriginalMessage.oldComment, Localize.translateLocal('common.description'), true),
);
}

const hasModifiedCreated = _.has(reportActionOriginalMessage, 'oldCreated') && _.has(reportActionOriginalMessage, 'created');
if (hasModifiedCreated) {
// Take only the YYYY-MM-DD value as the original date includes timestamp
let formattedOldCreated = new Date(reportActionOriginalMessage.oldCreated);
formattedOldCreated = format(formattedOldCreated, CONST.DATE.FNS_FORMAT_STRING);
return getProperSchemaForModifiedExpenseMessage(reportActionOriginalMessage.created, formattedOldCreated, Localize.translateLocal('common.date'), false);
messageFragments.push(getProperSchemaForModifiedExpenseMessage(reportActionOriginalMessage.created, formattedOldCreated, Localize.translateLocal('common.date'), false));
}

if (hasModifiedMerchant) {
return getProperSchemaForModifiedExpenseMessage(reportActionOriginalMessage.merchant, reportActionOriginalMessage.oldMerchant, Localize.translateLocal('common.merchant'), true);
messageFragments.push(
getProperSchemaForModifiedExpenseMessage(reportActionOriginalMessage.merchant, reportActionOriginalMessage.oldMerchant, Localize.translateLocal('common.merchant'), true),
);
}

const hasModifiedCategory = _.has(reportActionOriginalMessage, 'oldCategory') && _.has(reportActionOriginalMessage, 'category');
if (hasModifiedCategory) {
return getProperSchemaForModifiedExpenseMessage(reportActionOriginalMessage.category, reportActionOriginalMessage.oldCategory, Localize.translateLocal('common.category'), true);
messageFragments.push(
getProperSchemaForModifiedExpenseMessage(reportActionOriginalMessage.category, reportActionOriginalMessage.oldCategory, Localize.translateLocal('common.category'), true),
);
}

const hasModifiedTag = _.has(reportActionOriginalMessage, 'oldTag') && _.has(reportActionOriginalMessage, 'tag');
if (hasModifiedTag) {
return getProperSchemaForModifiedExpenseMessage(reportActionOriginalMessage.tag, reportActionOriginalMessage.oldTag, Localize.translateLocal('common.tag'), true);
messageFragments.push(getProperSchemaForModifiedExpenseMessage(reportActionOriginalMessage.tag, reportActionOriginalMessage.oldTag, Localize.translateLocal('common.tag'), true));
}

const hasModifiedBillable = _.has(reportActionOriginalMessage, 'oldBillable') && _.has(reportActionOriginalMessage, 'billable');
if (hasModifiedBillable) {
return getProperSchemaForModifiedExpenseMessage(reportActionOriginalMessage.billable, reportActionOriginalMessage.oldBillable, Localize.translateLocal('iou.request'), true);
messageFragments.push(
getProperSchemaForModifiedExpenseMessage(reportActionOriginalMessage.billable, reportActionOriginalMessage.oldBillable, Localize.translateLocal('iou.request'), true),
);
}

return messageFragments.reduce((acc, value, index) => {
if (index === 0) {
return acc + value;
}
return `${acc}. ${value.charAt(0).toUpperCase()}${value.slice(1)}`;
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this looks a bit strange to have a mixture of upper/lower case and also a mix of punctuation:

image

The reason they don't start with a capital is because you should read it like "Cristi changed...". With the addition of these multiple fragments though, it looks off. Here is what I suggest:

  1. Each message should start with a lower case
  2. Each message should end with a period
  3. If there are multiple fragments, then they should be like changed the date to blah and the amount to blah. (for two) and changed the date to blah, the amount to blah, and the description to blah. (for more than two)

Copy link
Contributor Author

@cristipaval cristipaval Oct 31, 2023

Choose a reason for hiding this comment

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

I implemented it similar to how it currently is in OldDot.
I really like your suggestion, I'll also ask in the wave4 channel if we want to change it in OldDot as well because it sounds more natural.

}, '');
}

/**
Expand Down
Loading