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

[$250] Implement tests for reports that should not be displayed in LHN #52034

Open
OlimpiaZurek opened this issue Nov 5, 2024 · 25 comments
Open
Assignees
Labels
Bug Something is broken. Auto assigns a BugZero manager. External Added to denote the issue can be worked on by a contributor Planning Changes still in the thought process Reviewing Has a PR in review Weekly KSv2

Comments

@OlimpiaZurek
Copy link
Contributor

OlimpiaZurek commented Nov 5, 2024

Tracking issue: #52031.

Develop and implement tests to verify that the following report types are not displayed in the Left-Hand Navigation (LHN):

  1. Hidden report: Ensure reports marked as hidden do not appear in the LHN.
  2. Report with no participants: Validate that chats without participants are excluded from the LHN.
  3. Access restricted report: Confirm that reports the user cannot access due to policy restrictions are not shown.
  4. Single transaction thread: Ensure that reports containing only one transaction are excluded from the LHN.
  5. Empty chat: Verify that empty chats, when the excludeEmptyChats setting is true, are not displayed.
  6. Domain email exclusion: Check that reports are excluded when the user’s email is domain-based and the setting is false.
  7. Parent message pending removal: Confirm that reports with parent messages pending removal do not appear in the LHN.
  8. Focus Mode without unread messages: Ensure that when the user is in focus mode without unread messages, no reports are displayed.
Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~021862798689700375032
  • Upwork Job ID: 1862798689700375032
  • Last Price Increase: 2024-11-30
  • Automatic offers:
    • c3024 | Reviewer | 105210430
    • mkzie2 | Contributor | 105210431
@OlimpiaZurek OlimpiaZurek added Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 labels Nov 5, 2024
Copy link

melvin-bot bot commented Nov 5, 2024

Triggered auto assignment to @alexpensify (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.

@mountiny mountiny changed the title Implement tests for reports that should not be displayed in LHN [HOLD App#52036]Implement tests for reports that should not be displayed in LHN Nov 5, 2024
@mountiny mountiny changed the title [HOLD App#52036]Implement tests for reports that should not be displayed in LHN [HOLD App#52036] Implement tests for reports that should not be displayed in LHN Nov 5, 2024
@alexpensify alexpensify added Weekly KSv2 Planning Changes still in the thought process and removed Daily KSv2 labels Nov 7, 2024
@alexpensify
Copy link
Contributor

Moving to Weekly since we are on hold for App#52036


Heads up, I will be offline until Tuesday, November 12, 2024, and will not actively watch over this GitHub during that period.

@melvin-bot melvin-bot bot added the Overdue label Nov 18, 2024
@alexpensify
Copy link
Contributor

Still on hold for App#52036

@melvin-bot melvin-bot bot removed the Overdue label Nov 19, 2024
@alexpensify
Copy link
Contributor

On hold

@CyberAndrii
Copy link
Contributor

Proposal

Please re-state the problem that we are trying to solve in this issue.

Implement tests for reports that should not be displayed in LHN.

What is the root cause of that problem?

This is the first time we're adding these tests.

What changes do you think we should make in order to solve the problem?

Example of 1 test for reference:

it('should not display hidden report', async () => {
    // When the SidebarLinks are rendered.
    await LHNTestUtils.getDefaultRenderedSidebarLinks();
    const report = createReport({isHidden: false});

    // And a non-hidden report is initialized in Onyx.
    await initializeState(false, {
        [`${ONYXKEYS.COLLECTION.REPORT}${report.reportID}`]: report,
    });

    // Then the report should appear in the sidebar.
    expect(getOptionRows()).toHaveLength(1);

    // When the report is hidden.
    await act(async () => {
        await Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT}${report.reportID}`, {isHidden: true});
    });

    // Then the report should not appear in the sidebar.
    expect(getOptionRows()).toHaveLength(0);
});

A few more can be found on my test branch. It also includes fixes for console errors that appeared while running these tests, e.g. missing key prop and missing act() calls.

Todo: check not only for the count of reports but also validate the name, etc.

@melvin-bot melvin-bot bot added Daily KSv2 Overdue and removed Weekly KSv2 labels Nov 25, 2024
Copy link

melvin-bot bot commented Nov 29, 2024

@alexpensify Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

@alexpensify
Copy link
Contributor

@mountiny - #52036 is closed now, can we move forward here? Also, should this one be external or internal? Thanks for the update!

@melvin-bot melvin-bot bot removed the Overdue label Nov 29, 2024
@mountiny mountiny changed the title [HOLD App#52036] Implement tests for reports that should not be displayed in LHN Implement tests for reports that should not be displayed in LHN Nov 30, 2024
@mountiny mountiny added the External Added to denote the issue can be worked on by a contributor label Nov 30, 2024
@melvin-bot melvin-bot bot changed the title Implement tests for reports that should not be displayed in LHN [$250] Implement tests for reports that should not be displayed in LHN Nov 30, 2024
Copy link

melvin-bot bot commented Nov 30, 2024

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

@melvin-bot melvin-bot bot added the Help Wanted Apply this label when an issue is open to proposals by contributors label Nov 30, 2024
Copy link

melvin-bot bot commented Nov 30, 2024

Triggered auto assignment to Contributor-plus team member for initial proposal review - @c3024 (External)

@mountiny
Copy link
Contributor

Yes external 🙌

Asking for the proposals in similar way as on this one #52032

@alexpensify
Copy link
Contributor

Thanks! We are still looking for proposals here.

@melvin-bot melvin-bot bot added the Overdue label Dec 3, 2024
Copy link

melvin-bot bot commented Dec 3, 2024

@alexpensify @OlimpiaZurek @mountiny @c3024 this issue is now 4 weeks old, please consider:

  • Finding a contributor to fix the bug
  • Closing the issue if BZ has been unable to add the issue to a VIP or Wave project
  • If you have any questions, don't hesitate to start a discussion in #expensify-open-source

Thanks!

@mkzie2
Copy link
Contributor

mkzie2 commented Dec 3, 2024

Edited by proposal-police: This proposal was edited at 2024-12-03 15:59:03 UTC.

Proposal

Please re-state the problem that we are trying to solve in this issue.

Implement tests for reports that should not be displayed in LHN

What is the root cause of that problem?

This is a new feature

What changes do you think we should make in order to solve the problem?

  1. To verify each type of report in the requirement will not display in LHN, we need to create a test case with shouldReportBeInOptionList function. Create a mock data for this report type and the expected shouldReportBeInOptionList function will return false for each type of report above.

function shouldReportBeInOptionList(params: ShouldReportBeInOptionListParams) {

  1. For the UI test, we need to create a test case for each report type in the requirement in tests/ui/LHNItemsPresence.tsx. And verify that this report isn't displayed in LHN by counting the row via getOptionRows

What specific scenarios should we cover in automated tests to prevent reintroducing this issue in the future?

The implementation of this issue is adding the unit test

What alternative solutions did you explore? (Optional)

@alexpensify
Copy link
Contributor

@c3024, when you get a chance, can you review if this new proposal will fix the issue? Thanks!

Copy link

melvin-bot bot commented Dec 4, 2024

@alexpensify, @OlimpiaZurek, @mountiny, @c3024 Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

@c3024
Copy link
Contributor

c3024 commented Dec 5, 2024

@mkzie2 's proposal for writing tests for both shouldReportBeInOptionList and UI looks better to me.

🎀 👀 🎀 C+ reviewed

Copy link

melvin-bot bot commented Dec 5, 2024

Triggered auto assignment to @luacmartins, see https://stackoverflow.com/c/expensify/questions/7972 for more details.

@melvin-bot melvin-bot bot removed the Help Wanted Apply this label when an issue is open to proposals by contributors label Dec 5, 2024
Copy link

melvin-bot bot commented Dec 5, 2024

📣 @c3024 🎉 An offer has been automatically sent to your Upwork account for the Reviewer role 🎉 Thanks for contributing to the Expensify app!

Offer link
Upwork job

Copy link

melvin-bot bot commented Dec 5, 2024

📣 @mkzie2 🎉 An offer has been automatically sent to your Upwork account for the Contributor role 🎉 Thanks for contributing to the Expensify app!

Offer link
Upwork job
Please accept the offer and leave a comment on the Github issue letting us know when we can expect a PR to be ready for review 🧑‍💻
Keep in mind: Code of Conduct | Contributing 📖

@mountiny
Copy link
Contributor

mountiny commented Dec 9, 2024

@luacmartins happy to keep this one, I think you got assigned before I was out for christmas lunch

@mountiny
Copy link
Contributor

mountiny commented Dec 9, 2024

@mkzie2 what is youer ETA for the PR?

@mkzie2
Copy link
Contributor

mkzie2 commented Dec 9, 2024

Will complete it today.

@melvin-bot melvin-bot bot added Reviewing Has a PR in review Weekly KSv2 and removed Daily KSv2 labels Dec 9, 2024
@mkzie2
Copy link
Contributor

mkzie2 commented Dec 9, 2024

@c3024 The PR is ready.

@alexpensify
Copy link
Contributor

Heads up, I will be offline until Wednesday, December 18, 2024, and will not actively watch over this GitHub during that period.

If this GitHub requires an urgent update, please ask for help in the #expensify-open-source Slack Room. If the inquiry can wait, I'll review it when I return online.

@alexpensify
Copy link
Contributor

Weekly Update: PR is moving along through the review process

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. External Added to denote the issue can be worked on by a contributor Planning Changes still in the thought process Reviewing Has a PR in review Weekly KSv2
Projects
Development

No branches or pull requests

7 participants