App: [HOLD for payment 2023-03-13] [$4000] The details view is broken for the room that is inaccessible to the user

My assignment was wrong. Bot marked it internal and assign me while proposals were being reviewed by another C+.

Thanks for clarifying, I added bonuses. Everyone has been paid out now, thanks!

@s77rt Updated

@luacmartins no, as commented above, that was the External label automation which assigned her because Andrew was ooo at the time.

Ok, Then Note for @sakluger the price should be dropped back to 4K which was the price during proposal acceptance.

The main reason why I persist using HOC is to avoid duplicated content showing logic of whether to show report detail or not found. Of course, we can define common function in utils to be used in 3 pages but I think it’s better to use already existing HOC which was made for report detail pages.

I totally agree that whether to use HOC or not is not a main factor to accept the proposal and it’s just a code dry thing. Also, the expected behaviour was not clear first time. So whether to close page or show not found is also not a factor to accept proposal. So I’d like to give priority to timeline - @s77rt who first proposed correct condition (reportID existence - this condition is already being used in ReportScreen) and general solution (applied to 3 pages).

Do you all think it’s fair now?

@0xmiroslav Is this not enough for this?

bump @0xmiroslav

This is still not a solid condition. We agreed to do nothing for loading check. And your proposal is limited to report detail page. We should apply this to all 3 pages.

@s77rt you could be assigned but PR should be in hold for testing because we’re not able to reproduce as I explained here. And this will affect timeline. Let’s wait a bit.

@0xmiroslav Your case is same as mine. What I meant is that the not found page is shown on the side bar but on the report screen the concierge report is shown (in your case)

So you meant report detail modal should match reportID with report screen behind? I think no problem with it. After you dismiss detail modal, goes back to correct report route. Wrong report route disappears and this is current behavior.

@shawnborton gave 👍 to do nothing for now so I think we can go with @s77rt’s proposal to check with reportID. But let’s hold assigning until https://github.com/Expensify/App/pull/14995 is merged

cc: @AndrewGable

Created this PR that fixes the sub-report deep link issue

when enter these deep links on address bar, it always redirects to https://staging.new.expensify.com/r/290129786064182

• https://staging.new.expensify.com/r/290129786064182/details
• https://staging.new.expensify.com/r/290129786064182/participants
• https://staging.new.expensify.com/r/290129786064182/settings

Before #14669, deep link worked correctly without redirect.

Oh I think that’s an edge case that I didn’t consider in https://github.com/Expensify/App/pull/14669, I will check it out today. Thanks for catching that @0xmiroslav!

After merging https://github.com/Expensify/App/pull/14669, cannot reproduce this issue because it always redirects to Report home page. I think this is a regression. i.e. when enter these deep links on address bar, it always redirects to https://staging.new.expensify.com/r/290129786064182

• https://staging.new.expensify.com/r/290129786064182/details
• https://staging.new.expensify.com/r/290129786064182/participants
• https://staging.new.expensify.com/r/290129786064182/settings

Before #14669, deep link worked correctly without redirect.

cc: @marcochavezf