[$500] [Wave 6: Workspace Chats] Refactor ReportUtils.requiresAttentionFromCurrentUser

[puneetlath]

Right now, ReportUtils.requiresAttentionFromCurrentUser can take either an option item or a report. This leads to some confusing to follow logic which is dependent on what type of argument its taking. Ideally we can clean this up to either:

1. take a specific type, not both
2. use clearer logic depending on the type
3. something else

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~019b604f4627d28c77
• Upwork Job ID: 1724144016716673024
• Last Price Increase: 2023-11-13

[waterim]

Hello, Im Artem from Callstack and would like to help with this issue!

[melvin-bot]

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

[melvin-bot]

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

[melvin-bot]

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

[melvin-bot]

📣 @sobitneupane Please request via NewDot manual requests for the Reviewer role ($500)

[puneetlath]

@waterim can you please post a proposal with your general planned approach for us to agree on before starting the PR? Thanks!

[waterim]

Sure, thank you, one question
2. use clearer logic depending on the type
3. something else

Do we have some specific requirements for these steps?

[puneetlath]

Nothing thought through. Just that right now the function is confusing because it is in ReportUtils, but is technically called with both reports and optionItems. And reports and optionItems don't have all the same properties and that makes the code both more confusing and more likely to have a bug introduced. And we seem to already have run into at least one bug as a result.

[sobitneupane]

@waterim Which logics are shared between optionItem and report? What are the distinct logics for each?

const isOption = !!option.parentReportAction

I believe relying on a field such as parentReportAction to ascertain isOption is not ideal. Instead, I would prefer passing isOption as a function argument if its value can be determined in advance.

To get rid of confusing logic, the best way to go will be splitting the function.

[puneetlath]

The downside of two separate functions is that they need to use the same logic and have the potential to get out of sync. So I think I like the typescript types approach and we do it once https://github.com/Expensify/App/pull/29012/files is done. What do y'all think about that?

[sobitneupane]

@puneetlath I agree. We can just wait for #29012 to get merged. It is being actively worked on.

[puneetlath]

Still holding on #29012

[puneetlath]

ReportUtils to TS was merged, so you're unblocked from going forward here @waterim

[puneetlath]

How's this going @waterim?

[waterim]

@puneetlath Updated today, waiting for C+ answers

[puneetlath]

Ok great.

[melvin-bot]

This issue has not been updated in over 15 days. @puneetlath, @sobitneupane, @waterim eroding to Monthly issue.

P.S. Is everyone reading this sure this is really a near-term priority? Be brave: if you disagree, go ahead and close it out. If someone disagrees, they'll reopen it, and if they don't: one less thing to do!

[puneetlath]

@sobitneupane @waterim everything is done here right? If so, all that is left is to pay @sobitneupane for C+?

[sobitneupane]

Yep @puneetlath.

[puneetlath]

Great. Please request on NewDot.

Payment summary:

• sobitneupane C+ - $500

[JmillsExpensify]

$500 approved for @sobitneupane based on comment above.