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 payment 2024-10-24] [HOLD for payment 2024-10-16] Add LHN debugging to Debug Mode #46992

Open
puneetlath opened this issue Aug 7, 2024 · 40 comments
Assignees
Labels
Awaiting Payment Auto-added when associated PR is deployed to production NewFeature Something to build that is a new item. Reviewing Has a PR in review Weekly KSv2

Comments

@puneetlath
Copy link
Contributor

puneetlath commented Aug 7, 2024

We are adding a debug mode to the App here: #45481

What I would really love from the Debug Mode is something that tells me why a given report is in the LHN. So if I'm seeing a report in the LHN, why is it there? And similarly if it has an RBR/GBR, what is causing that to be there?
This might be more useful in Focus Mode than in Most Recent, but I think it would help with a lot of the recent issues we've been troubleshooting.

cc @pac-guerreiro

Issue OwnerCurrent Issue Owner: @puneetlath
@puneetlath puneetlath added Weekly KSv2 NewFeature Something to build that is a new item. labels Aug 7, 2024
@puneetlath puneetlath self-assigned this Aug 7, 2024
Copy link

melvin-bot bot commented Aug 7, 2024

Current assignee @puneetlath is eligible for the NewFeature assigner, not assigning anyone new.

Copy link

melvin-bot bot commented Aug 7, 2024

⚠️ It looks like this issue is labelled as a New Feature but not tied to any GitHub Project. Keep in mind that all new features should be tied to GitHub Projects in order to properly track external CAP software time ⚠️

Copy link

melvin-bot bot commented Aug 7, 2024

Triggered auto assignment to Design team member for new feature review - @shawnborton (NewFeature)

@pac-guerreiro
Copy link
Contributor

Hi! I’m Pedro Guerreiro from Callstack - expert contributor group. I’d like to work on this task!

@puneetlath
Copy link
Contributor Author

Still waiting for the first PR to get merged.

@melvin-bot melvin-bot bot removed the Overdue label Aug 16, 2024
@DylanDylann
Copy link
Contributor

yeah, I will try to complete check list today

@melvin-bot melvin-bot bot added the Overdue label Aug 26, 2024
@puneetlath
Copy link
Contributor Author

Still waiting for the first PR to get merged.

@puneetlath
Copy link
Contributor Author

Still waiting for the first PR to get merged.

@melvin-bot melvin-bot bot removed the Overdue label Sep 4, 2024
@melvin-bot melvin-bot bot added the Overdue label Sep 13, 2024
@puneetlath
Copy link
Contributor Author

Still waiting for the first PR to get merged.

@melvin-bot melvin-bot bot removed the Overdue label Sep 17, 2024
@melvin-bot melvin-bot bot added the Overdue label Sep 25, 2024
@blimpich
Copy link
Contributor

@puneetlath can you respond to this comment on the PR?

@fabioh8010
Copy link
Contributor

Update: PR is being reviewed by C+, should be completed before next week.

@blimpich
Copy link
Contributor

blimpich commented Oct 1, 2024

Assigning @DylanDylann since they are reviewing the PR

@melvin-bot melvin-bot bot added Reviewing Has a PR in review Weekly KSv2 and removed Weekly KSv2 labels Oct 9, 2024
@pac-guerreiro
Copy link
Contributor

@puneetlath @DylanDylann

Here is the PR that adds the debug option to LHN context menu - #50519 😄

@DylanDylann
Copy link
Contributor

@pac-guerreiro Currently, we don't display the reason why RBR is displayed. But let's see logic to display RBR on LHN

function shouldShowRedBrickRoad(report: Report, reportActions: OnyxEntry<ReportActions>, hasViolations: boolean, transactionViolations?: OnyxCollection<TransactionViolation[]>) {
const hasErrors = Object.keys(OptionsListUtils.getAllReportErrors(report, reportActions)).length !== 0;
const oneTransactionThreadReportID = ReportActionsUtils.getOneTransactionThreadReportID(report.reportID, ReportActionsUtils.getAllReportActions(report.reportID));
if (oneTransactionThreadReportID) {
const oneTransactionThreadReport = ReportUtils.getReport(oneTransactionThreadReportID);
if (
ReportUtils.shouldDisplayTransactionThreadViolations(
oneTransactionThreadReport,
transactionViolations,
ReportActionsUtils.getAllReportActions(report.reportID)[oneTransactionThreadReport?.parentReportActionID ?? '-1'],
)
) {
return true;
}
}
return hasErrors || hasViolations;

We have three cases where the RBR is displayed on LHN

  • shouldDisplayTransactionThreadViolations return true
  • hasErrors (errors come from report or reportActions. Usually, this will be returned from BE)
  • hasViolations

We need to design three reason messages for each case

@puneetlath
Copy link
Contributor Author

Yes, let's add that. I think it would be very helpful for diagnosing why RBRs are showing.

@pac-guerreiro
Copy link
Contributor

@puneetlath can you create a new issue for me to implement this? 😄

@puneetlath
Copy link
Contributor Author

Yep. Here you go: #50665

@pac-guerreiro
Copy link
Contributor

Thanks @puneetlath, that was fast 😄

@melvin-bot melvin-bot bot added Daily KSv2 and removed Weekly KSv2 labels Oct 16, 2024
Copy link

melvin-bot bot commented Oct 16, 2024

Payment Summary

Upwork Job

  • Contributor: @pac-guerreiro is from an agency-contributor and not due payment
  • ROLE: @DylanDylann paid $(AMOUNT) via Upwork (LINK)

BugZero Checklist (@puneetlath)

  • I have verified the correct assignees and roles are listed above and updated the neccesary manual offers
  • I have verified that there are no duplicate or incorrect contracts on Upwork for this job (https://www.upwork.com/ab/applicants//hired)
  • I have paid out the Upwork contracts or cancelled the ones that are incorrect
  • I have verified the payment summary above is correct

@puneetlath
Copy link
Contributor Author

@DylanDylann are we all done here?

@melvin-bot melvin-bot bot added Weekly KSv2 and removed Daily KSv2 labels Oct 17, 2024
@melvin-bot melvin-bot bot changed the title [HOLD for payment 2024-10-16] Add LHN debugging to Debug Mode [HOLD for payment 2024-10-24] [HOLD for payment 2024-10-16] Add LHN debugging to Debug Mode Oct 17, 2024
Copy link

melvin-bot bot commented Oct 17, 2024

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

@melvin-bot melvin-bot bot removed the Reviewing Has a PR in review label Oct 17, 2024
Copy link

melvin-bot bot commented Oct 17, 2024

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

If no regressions arise, payment will be issued on 2024-10-24. 🎊

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

Copy link

melvin-bot bot commented Oct 17, 2024

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

  • [@DylanDylann] Please propose regression test steps to ensure the new feature will work correctly on production in further releases.
  • [@puneetlath] Link the GH issue for creating/updating the regression test once above steps have been agreed upon.

@DylanDylann
Copy link
Contributor

@puneetlath Do we require the regression test on debug feature issues?

@melvin-bot melvin-bot bot added Reviewing Has a PR in review Weekly KSv2 and removed Weekly KSv2 labels Oct 17, 2024
@puneetlath
Copy link
Contributor Author

Yes please. We rely on these features quite a bit, so it'd be good to have some regression testing to make sure we don't break them accidentally. Otherwise we won't realize they are broken until we need them to help us debug something.

@DylanDylann
Copy link
Contributor

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:

[@DylanDylann] The PR that introduced the bug has been identified. Link to the PR: NA
[@DylanDylann] 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: NA
[@DylanDylann] 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: NA
[@DylanDylann] Determine if we should create a regression test for this bug. Yes
[@DylanDylann] 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.

Regression Test Proposal

  1. Enable the Debug feature
  2. On the home page, open a chat report from LHN
  3. Click on the header of the chat you just opened, then click on Debug
  4. On the Details tab, confirm that "Visible in LHN" provides the correct reason for the report to be shown on LHN
  5. Confirm that "GBR" shows the correct reason, if the report has GBR in LHN
  6. Confirm that "RBR" shows the correct reason, if the report has RBR in LHN

Do we agree 👍 or 👎

Copy link

melvin-bot bot commented Oct 24, 2024

Payment Summary

Upwork Job

  • Contributor: @pac-guerreiro is from an agency-contributor and not due payment
  • ROLE: @DylanDylann paid $(AMOUNT) via Upwork (LINK)

BugZero Checklist (@puneetlath)

  • I have verified the correct assignees and roles are listed above and updated the neccesary manual offers
  • I have verified that there are no duplicate or incorrect contracts on Upwork for this job (https://www.upwork.com/ab/applicants//hired)
  • I have paid out the Upwork contracts or cancelled the ones that are incorrect
  • I have verified the payment summary above is correct

@puneetlath
Copy link
Contributor Author

@DylanDylann were there multiple PRs for this one?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Awaiting Payment Auto-added when associated PR is deployed to production NewFeature Something to build that is a new item. Reviewing Has a PR in review Weekly KSv2
Projects
Development

No branches or pull requests

7 participants