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.

Sign up for GitHub

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

Jump to bottom

[$250] [Bug] Clean up confusing logic for when a report is exempt from being filtered out of the LHN #11481

Closed
tgolen opened this issue Sep 30, 2022 · 10 comments
Closed

[$250] [Bug] Clean up confusing logic for when a report is exempt from being filtered out of the LHN #11481

tgolen opened this issue Sep 30, 2022 · 10 comments
Assignees
@JmillsExpensify @tgolen @parasharrajat
Labels
Daily KSv2 External Added to denote the issue can be worked on by a contributor Improvement Item broken or needs improvement. Internal Requires API changes or must be handled by Expensify staff Reviewing Has a PR in review

Comments

@tgolen
Copy link
Contributor

tgolen commented Sep 30, 2022

Problem

Coming from #10863 (comment)

The logic there is confusing and difficult to reason about.

Why is this important?

This makes the code hard to maintain.

Solution

Simplify and extract the logic to a separate method or break down the logic into several easier-to-read booleans.
@tgolen tgolen added the Internal Requires API changes or must be handled by Expensify staff label Sep 30, 2022
@tgolen tgolen self-assigned this Sep 30, 2022
@tgolen tgolen added Weekly KSv2 Improvement Item broken or needs improvement. labels Sep 30, 2022
@tgolen tgolen mentioned this issue Sep 30, 2022
Merged
93 tasks
@melvin-bot
Copy link

melvin-bot bot commented Sep 30, 2022

Triggered auto assignment to @JmillsExpensify for Upwork job creation (chore: ContributorPlusReview), see https://stackoverflow.com/c/expensify/questions/13732 for more details.

@melvin-bot melvin-bot bot added Daily KSv2 Overdue and removed Weekly KSv2 labels Sep 30, 2022
@tgolen tgolen added the Reviewing Has a PR in review label Oct 3, 2022
@melvin-bot melvin-bot bot removed the Overdue label Oct 3, 2022
@tgolen
Copy link
Contributor Author

tgolen commented Oct 3, 2022

@JmillsExpensify can we get a C+ review for this, please?

@JmillsExpensify
Copy link

Sure thing, jumping in now!

@JmillsExpensify JmillsExpensify added the External Added to denote the issue can be worked on by a contributor label Oct 3, 2022
@melvin-bot
Copy link

melvin-bot bot commented Oct 3, 2022

Current assignee @JmillsExpensify is eligible for the External assigner, not assigning anyone new.

@melvin-bot
Copy link

melvin-bot bot commented Oct 3, 2022

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

@melvin-bot
Copy link

melvin-bot bot commented Oct 3, 2022

Current assignee @tgolen is eligible for the External assigner, not assigning anyone new.

@melvin-bot melvin-bot bot changed the title [Bug] Clean up confusing logic for when a report is exempt from being filtered out of the LHN [$250] [Bug] Clean up confusing logic for when a report is exempt from being filtered out of the LHN Oct 3, 2022
@JmillsExpensify
Copy link

Created the Upwork here: https://www.upwork.com/jobs/~01f59be4a13ee28103

@melvin-bot melvin-bot bot closed this as completed Oct 4, 2022
@JmillsExpensify
Copy link

Wait, @tgolen did we have a C+ review on this PR? I created an Upwork job but I'm not seeing any review in the linked PR.

@parasharrajat
Copy link
Member

No it was merged before that.

@JmillsExpensify
Copy link

Cool. I'll close out the Upwork then, thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@JmillsExpensify JmillsExpensify

@tgolen tgolen

@parasharrajat parasharrajat

Labels
Daily KSv2 External Added to denote the issue can be worked on by a contributor Improvement Item broken or needs improvement. Internal Requires API changes or must be handled by Expensify staff Reviewing Has a PR in review
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@JmillsExpensify @tgolen @parasharrajat