-
Notifications
You must be signed in to change notification settings - Fork 3k
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
Update "automatically approved" report action copy #49909
Changes from 16 commits
73a3a47
5567b20
d4f8bf1
e6a177d
176d9cb
92e739d
6ade3df
5c82888
842cd2f
beb72d7
22fad90
c2a3b4c
82ef5bb
be26889
a0e65b3
30be12b
559b9d8
05e2d8c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -270,6 +270,30 @@ | |
}); | ||
}); | ||
}); | ||
|
||
describe('ParentReportAction is', () => { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @roryabraham here's an example There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I was thinking more along the lines of UI tests rendering There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hmm in that case since there's not much to go on right now, i'd prefer taking this as a follow-up for a contributor or C+ to write up b/c it's going to take me a bit to figure out how that works & that'll keep delaying this PR from getting merged There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Up to you. I'd encourage you to take the time to learn and write the tests, because I don't see this PR as so urgent that we can't wait for tests. Just my opinion though (informed by my observed experience that 90% of the time when we "follow up to add tests", that part just never happens) 🙂 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I feel confident that I can get unit tests written externally, so I created this follow-up #50351 I believe this will be more time efficient because I have more important issues to focus on this week |
||
test('Manually Submitted Report Action', () => { | ||
const threadOfSubmittedReportAction = { | ||
...LHNTestUtils.getFakeReport(), | ||
type: CONST.REPORT.TYPE.EXPENSE, | ||
stateNum: CONST.REPORT.STATE_NUM.SUBMITTED, | ||
statusNum: CONST.REPORT.STATUS_NUM.SUBMITTED, | ||
parentReportID: '101', | ||
policyID: policy.id, | ||
}; | ||
const submittedParentReportAction = { | ||
actionName: CONST.REPORT.ACTIONS.TYPE.SUBMITTED, | ||
originalMessage: { | ||
amount: 169, | ||
currency: 'USD', | ||
}, | ||
} as ReportAction; | ||
|
||
expect( | ||
Check failure on line 292 in tests/unit/ReportUtilsTest.ts GitHub Actions / Changed files ESLint check
Check failure on line 292 in tests/unit/ReportUtilsTest.ts GitHub Actions / ESLint check
|
||
ReportUtils.getReportName(threadOfSubmittedReportAction, policy, submittedParentReportAction), | ||
).toBe('submitted $1.69'); | ||
}); | ||
}); | ||
}); | ||
|
||
describe('requiresAttentionFromCurrentUser', () => { | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Suggestion: log when we execute this fallback such that, some time from now, we can search up the log and remove this code if it's no longer needed. Create a monthly issue to check back in.