[$500] [HOLD for payment 2023-09-29] Follow ups to the Edit Money Request

[mountiny]

Follow ups to the API PR https://github.com/Expensify/Web-Expensify/pull/38252 and App PR #23703 related to https://github.com/Expensify/Expensify/issues/295091.

• Update the optimistic data to re-evaluate the iou report total correctly if the currency if all the requests is same
• Enable the currency edit option cc @koko57
• Discuss the messages for the various types of modifiedexpense, should we remove the previously part if comment was empty
• Auto focus the description field Enable editing of money requests #23703 (comment)
• Noting from internal issue - lets also translate the modified expense messages [HOLD for payment 2023-09-20] [$250] Translate the MODIFIEDEXPENSE messages #26369
• Update the header styles for when editing in offline mode to give correct feedback
• Update styles of the non-editable header to not look as disabled
• Add the verification checks from the EditRequestPage which make sure that something actually changed to some helper method in the IOU.editMoneyRequest
• Handle the red brick road better
• Do not allow setting future dates
• Make sure admins can edit the transactions logs
• Add tests to the jest suite for IOU when editing IOU requests and expense reports too
  • you can only edit your own transactions in case of IOU
  • admins can also edit the transactions made by employees
  • correct expense/iou report totals are computed
  • [HOLD for payment 2023-11-01] Improve the IOUTest with Edit, delete and payment with ACH #26366 will be handled here
• [HOLD for payment 2023-08-28] IOU - Cannot edit the amount field in a negative IOU request #24623
• User which is not able to edit the request can access the form when directly visiting the edit url. On EditMoneyRequest page, lets evaluate on first render if the user can edit and if not, lets show Hmmm its not here page or close the modal, whatever is better
• [$500] [HOLD for payment 2023-09-07] Update styles for money request header user cannot edit #25931

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~01778206bdcc3370d6
• Upwork Job ID: 1709801140497133568
• Last Price Increase: 2023-10-05

[mountiny]

@koko57 Could you please comment here, I will assign you to help out with this.

[koko57]

@mountiny I'm ready to help 🙌🏻

[mountiny]

Thank you! Would you be able to base on top of this PR and work on enabling the currency edit? #23703

[koko57]

@mountiny PR opened: #24565

[mountiny]

Will work on the other follow ups here with Agata, adding the C+ from the original PR here #23703

[melvin-bot]

⚠️ Looks like this issue was linked to a Deploy Blocker here

If you are the assigned CME please investigate whether the linked PR caused a regression and leave a comment with the results.

If a regression has occurred and you are the assigned CM follow the instructions here.

If this regression could have been avoided please consider also proposing a recommendation to the PR checklist so that we can avoid it in the future.

[mountiny]

Created a PR for allowing admins to edit the money requests

[mountiny]

@allroundexperts you are already here, there will be bunch more PRs here

[allroundexperts]

Sounds good!!!

[mountiny]

For the messages update, if the old amount is an empty value, we should use this language:

• Set an amount: %amount%
• Set an date: %created%

And the current message is if the old value is non empty. @koko57 would you be able to incorporate it to the next PR? We can have multiple tasks in one PR

[srikarparsi]

Linking this here to add it to your radar: Cannot edit a negative IOU request, It's also formatted different since the negative sign goes before the symbol in other places but not sure how that would look in the edit preview.

[melvin-bot]

Reviewing label has been removed, please complete the "BugZero Checklist".

[melvin-bot]

The solution for this issue has been 🚀 deployed to production 🚀 in version 1.3.72-11 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

• fix: move amount diff to check in edit money request #27451

If no regressions arise, payment will be issued on 2023-09-29. 🎊

After the hold period is over and BZ checklist items are completed, please complete any of the applicable payments for this issue, and check them off once done.

• External issue reporter
• Contributor that fixed the issue
• Contributor+ that helped on the issue and/or PR

For reference, here are some details about the assignees on this issue:

• @koko57 does not require payment (Contractor)
• @rushatgabhane does not require payment (Eligible for Manual Requests)
• @allroundexperts does not require payment (Eligible for Manual Requests)

As a reminder, here are the bonuses/penalties that should be applied for any External issue:

• Merged PR within 3 business days of assignment - 50% bonus
• Merged PR more than 9 business days after assignment - 50% penalty

[melvin-bot]

BugZero Checklist: The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed:

• [@allroundexperts] The PR that introduced the bug has been identified. Link to the PR:
• [@allroundexperts] The offending PR has been commented on, pointing out the bug it caused and why, so the author and reviewers can learn from the mistake. Link to comment:
• [@allroundexperts] A discussion in #expensify-bugs has been started about whether any other steps should be taken (e.g. updating the PR review checklist) in order to catch this type of bug sooner. Link to discussion:
• [@allroundexperts] Determine if we should create a regression test for this bug.
• [@allroundexperts] If we decide to create a regression test for the bug, please propose the regression test steps to ensure the same bug will not reach production again.
• [@kadiealexander] Link the GH issue for creating/updating the regression test once above steps have been agreed upon:

[kadiealexander]

BUmped @allroundexperts in Slack here.

[allroundexperts]

Payment Summary

1. Review of Enable editing of money requests #23703 by @allroundexperts - $1000

2. Review of Followup/23961 edit request headers #25399 by @allroundexperts - $1000

3. Review of Followup #23961: update IOU report total in optimistic data on edit money request #25498 by @allroundexperts - $500 (This one had a possible regression so half the amount)

@mountiny It was mentioned that this had a regression but as mentioned in the followup PR here, the issue was not visible in the UI anywhere. I'm not sure how this could have been caught while in the review stage so I would request for a penalty waiver.

5. Review of fix: move amount diff to check in edit money request #27451 by @allroundexperts - $500 (Since this was after Aug 30) (This is only payable IF the regression penalty of above PR is waived off)

6. I created and fixed another issue with the flow here. @mountiny and I mutually decided on a flat $500 for this - $500

Total Payment for @allroundexperts: $3000

If the above waiver request is accepted, then the payment becomes: $4000

@mountiny @kadiealexander Please let me know if I missed anything.

[mountiny]

LGTM

[kadiealexander]

@mountiny any thoughts on the waiver @allroundexperts has requested?

[kadiealexander]

Also, are @rushatgabhane and @koko57 due any payments?

[allroundexperts]

I think @rushatgabhane did review one PR. But I was not able to find it. @rushatgabhane Let us know if you remember.

@koko57 is from Agency.

[mountiny]

@kadiealexander waiver sounds reasonable to me in this case

[melvin-bot]

Job added to Upwork: https://www.upwork.com/jobs/~01778206bdcc3370d6

[kadiealexander]

Payouts due:

• Contributor: $4000 @allroundexperts

Eligible for 50% #urgency bonus? No

Upwork job is here

[allroundexperts]

Hi @kadiealexander, I get paid through the app. I'll request payment there now!

[JmillsExpensify]

$4,000 payment approved for @allroundexperts based on BZ summary.

[mountiny]

I believe we can close this one then, regression tests for edit moeny request are handled separately