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

[hold for #51296] Write unit tests for updated ReportActionItem text depending on given originalMessage data #50351

Open
Beamanator opened this issue Oct 7, 2024 · 40 comments
Assignees
Labels
Bug Something is broken. Auto assigns a BugZero manager. Reviewing Has a PR in review Weekly KSv2

Comments

@Beamanator
Copy link
Contributor

Beamanator commented Oct 7, 2024

Background:

strongly encouraged suggestion: Write some automated tests to cover this change. It's a very good candidate for unit tests, and they'd go a long way to make this code more robust and avoid it breaking in the future.

That ^ suggestion was given here and I agree we should write unit tests for the changes in that PR 👍 I'm going to make this external b/c it would be great to get external help on this 🙏

Expected Result:

Please add unit tests for:

  1. UI tests rendering <ReportActionItem> using @testing-library/react-native.
  2. Additional getReportName tests covering changes from Update "automatically approved" report action copy #49909
@Beamanator Beamanator added Daily KSv2 Bug Something is broken. Auto assigns a BugZero manager. labels Oct 7, 2024
@Beamanator Beamanator self-assigned this Oct 7, 2024
Copy link

melvin-bot bot commented Oct 7, 2024

Triggered auto assignment to @Christinadobrzyn (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details. Please add this bug to a GH project, as outlined in the SO.

@Beamanator
Copy link
Contributor Author

Assigning @allgandalf from https://expensify.slack.com/archives/C02NK2DQWUX/p1728329535786379 🙏

@Beamanator Beamanator added Weekly KSv2 and removed Daily KSv2 labels Oct 7, 2024
@allgandalf
Copy link
Contributor

@Beamanator should i wait for #49909 to get merged ?

@Beamanator
Copy link
Contributor Author

Oh good call 😅 You caaaan start writing tests if you want, but obviously they shouldn't pass till #49909 is merged 😬

@Beamanator
Copy link
Contributor Author

Boom, PR merged!

@allgandalf
Copy link
Contributor

PR will be ready over the weekend 👍

@Christinadobrzyn
Copy link
Contributor

Ooh - looks like PR is in production - #49909

Adding a payment date to this GH.

@Christinadobrzyn Christinadobrzyn changed the title Write unit tests for updated ReportActionItem text depending on given originalMessage data [Payment due Oct 18] Write unit tests for updated ReportActionItem text depending on given originalMessage data Oct 15, 2024
@Christinadobrzyn

This comment was marked as off-topic.

@Christinadobrzyn Christinadobrzyn added Daily KSv2 and removed Weekly KSv2 labels Oct 18, 2024
@Beamanator
Copy link
Contributor Author

Hmm I don't actually think the PR was created yet, 😅 @allgandalf have you started this yet?

@allgandalf
Copy link
Contributor

Hmm I don't actually think the PR was created yet, 😅 @allgandalf have you started this yet?

This was weekly and didn't show up on K2 for some reason, I will raise one today/tomorrow 👍

@Beamanator
Copy link
Contributor Author

aah funky! glad it's daily now :D and no worries for the delay, was just curious haha

@Beamanator Beamanator changed the title [Payment due Oct 18] Write unit tests for updated ReportActionItem text depending on given originalMessage data Write unit tests for updated ReportActionItem text depending on given originalMessage data Oct 18, 2024
@melvin-bot melvin-bot bot added the Overdue label Oct 21, 2024
@allgandalf
Copy link
Contributor

PR would be up today 🙊

@melvin-bot melvin-bot bot removed the Overdue label Oct 21, 2024
Copy link

melvin-bot bot commented Oct 21, 2024

@Beamanator @Christinadobrzyn @allgandalf this issue was created 2 weeks ago. Are we close to a solution? Let's make sure we're treating this as a top priority. Don't hesitate to create a thread in #expensify-open-source to align faster in real time. Thanks!

@Christinadobrzyn
Copy link
Contributor

Update for Melvin- we're working on the PR!

@allgandalf
Copy link
Contributor

Wrong issue was linked here, @Christinadobrzyn can you please remove the hold for payment label here ? PR is still ongoing

@garrettmknight garrettmknight moved this to Bugs and Follow Up Issues in [#whatsnext] #expense Nov 14, 2024
@garrettmknight garrettmknight moved this from Bugs and Follow Up Issues to Hold for Payment in [#whatsnext] #expense Nov 14, 2024
@Christinadobrzyn Christinadobrzyn changed the title [HOLD for payment 2024-11-20] Write unit tests for updated ReportActionItem text depending on given originalMessage data Write unit tests for updated ReportActionItem text depending on given originalMessage data Nov 19, 2024
@Christinadobrzyn
Copy link
Contributor

yep! removed the payment information from the title. Let me know if you need me to do anything else!

@melvin-bot melvin-bot bot added Daily KSv2 and removed Weekly KSv2 labels Nov 19, 2024
@Christinadobrzyn
Copy link
Contributor

Christinadobrzyn commented Nov 20, 2024

@allgandalf can you link the PR we should be monitoring when you have a moment? TY!

@allgandalf
Copy link
Contributor

Yeah yeah, this one :

@Christinadobrzyn
Copy link
Contributor

watching PR #51330

Going to move this to weekly since the PR is still under works.

@Christinadobrzyn Christinadobrzyn added Weekly KSv2 Reviewing Has a PR in review and removed Daily KSv2 Awaiting Payment Auto-added when associated PR is deployed to production labels Nov 21, 2024
@allgandalf
Copy link
Contributor

@Christinadobrzyn @Beamanator lets put this one on hold for #51296, in that issue we are making reportactionItem to be a pure component. I am the C+ on that issue, so i can start working on this one as soon as we merge that one

@Beamanator
Copy link
Contributor Author

Sounds good to me!

@Christinadobrzyn Christinadobrzyn changed the title Write unit tests for updated ReportActionItem text depending on given originalMessage data [hold for #51296] Write unit tests for updated ReportActionItem text depending on given originalMessage data Nov 26, 2024
@Christinadobrzyn
Copy link
Contributor

Hold for #51296

6 similar comments
@Christinadobrzyn
Copy link
Contributor

Hold for #51296

@Christinadobrzyn
Copy link
Contributor

Hold for #51296

@Christinadobrzyn
Copy link
Contributor

Hold for #51296

@allgandalf
Copy link
Contributor

Hold for #51296

@Christinadobrzyn
Copy link
Contributor

Hold for #51296

@Christinadobrzyn
Copy link
Contributor

Hold for #51296

@allgandalf
Copy link
Contributor

still held...

@allgandalf
Copy link
Contributor

still held....

@Christinadobrzyn
Copy link
Contributor

Hold for #51296

1 similar comment
@Christinadobrzyn
Copy link
Contributor

Hold for #51296

@allgandalf
Copy link
Contributor

@Christinadobrzyn please drop this to monthly, the PR we are held on is still a long way from merged

@dylanexpensify dylanexpensify moved this from Hold for Payment to Bugs and Follow Up Issues in [#whatsnext] #expense Feb 4, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something is broken. Auto assigns a BugZero manager. Reviewing Has a PR in review Weekly KSv2
Projects
Status: Bugs and Follow Up Issues
Development

No branches or pull requests

3 participants