App icon indicating copy to clipboard operation
App copied to clipboard

Can't add new report actions in Debug mode

Open DylanDylann opened this issue 1 year ago β€’ 3 comments

If you haven’t already, check out our contributing guidelines for onboarding and email [email protected] to request to join our Slack channel!


Coming from https://github.com/Expensify/App/issues/53027#issuecomment-2500692010

https://github.com/Expensify/App/blob/3f4e93ee40462b42f5fe331fdf9db5c9bbb1a421/src/pages/Debug/ReportAction/DebugReportActionCreatePage.tsx#L117

we use the set method but we don't add the previous data. It caused when adding new report action, all previous report actions will be removed

cc @pac-guerreiro @puneetlath @JKobrynski

DylanDylann avatar Dec 06 '24 07:12 DylanDylann

Proposal

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

We are not keeping the old state in onyx

What is the root cause of that problem?

We are using set method of onyx, we should use merge

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

here: https://github.com/Expensify/App/blob/3baf965618ced48845ddd7534b0fad4d4110b5de/src/libs/actions/Debug.ts#L10

we can create one method for merge as well as:

function mergeDebugData<TKey extends OnyxKey | `${OnyxCollectionKey}${string}`>(onyxKey: TKey, onyxValue: OnyxMergeInput<TKey>) {
    Onyx.merge(onyxKey, onyxValue);
}

and then use it

Shahidullah-Muffakir avatar Dec 06 '24 08:12 Shahidullah-Muffakir

@pac-guerreiro or @JKobrynski will handle this issue. No require proposals

DylanDylann avatar Dec 06 '24 08:12 DylanDylann

Hi, I'm Julian from Callstack - expert agency - and I would like to work on this issue.

JKobrynski avatar Dec 06 '24 10:12 JKobrynski

@JKobrynski Feel free to raise a PR

DylanDylann avatar Dec 09 '24 10:12 DylanDylann

@JKobrynski do you have an ETA for a PR?

puneetlath avatar Dec 11 '24 16:12 puneetlath

@DylanDylann @puneetlath I'm back from my vacation so I'm taking this from @JKobrynski πŸ˜„

I'll raise a PR at the end of my work day πŸ˜„

pac-guerreiro avatar Dec 11 '24 16:12 pac-guerreiro

Welcome back!

puneetlath avatar Dec 11 '24 16:12 puneetlath

@puneetlath thanks! πŸ˜„

@DylanDylann I just opened a draft PR - https://github.com/Expensify/App/pull/53969

I'll need to add screen recordings tomorrow!

If the PR is too big, I can split it in two πŸ˜‰

pac-guerreiro avatar Dec 11 '24 22:12 pac-guerreiro

Looks ok to me in terms of size, thanks!

puneetlath avatar Dec 12 '24 16:12 puneetlath

Today:

  • [x] Add testing steps
  • [x] Add screen recordings

Todo:

  • [ ] Fix Not Found screen shown when navigating back after deleting a report

pac-guerreiro avatar Dec 12 '24 23:12 pac-guerreiro

@puneetlath @DylanDylann The PR is now ready for review! πŸ˜„

There's a minor issue where the Not Found page is shown if the user goes back after deleting a report. I'll look for a fix for this issue tomorrow!

pac-guerreiro avatar Dec 12 '24 23:12 pac-guerreiro

@DylanDylann @puneetlath

Last friday (12/13/24)

  • I didn't have much time to focus on the remaining issue but I did some investigation and got some ideas to try

Today

  • I tried some ideas and managed to find one that fixed the Not Found page showing after deleting a report, going to concierge and pressing back button on the header

pac-guerreiro avatar Dec 17 '24 00:12 pac-guerreiro

Waiting on @DylanDylann review πŸ˜„

pac-guerreiro avatar Dec 18 '24 00:12 pac-guerreiro

@puneetlath

@DylanDylann approved the PR πŸ₯³ Feel free to chime in and let me know if there's anything that needs my attention πŸ˜„

pac-guerreiro avatar Dec 18 '24 11:12 pac-guerreiro

The PR was finally merged yesterday! πŸ₯³

pac-guerreiro avatar Dec 19 '24 10:12 pac-guerreiro

⚠️ Looks like this issue was linked to a Deploy Blocker here

If you are the assigned CME please investigate whether the linked PR caused a regression and leave a comment with the results.

If a regression has occurred and you are the assigned CM follow the instructions here.

If this regression could have been avoided please consider also proposing a recommendation to the PR checklist so that we can avoid it in the future.

melvin-bot[bot] avatar Dec 24 '24 07:12 melvin-bot[bot]

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

melvin-bot[bot] avatar Dec 26 '24 20:12 melvin-bot[bot]

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

  • https://github.com/Expensify/App/pull/53969

If no regressions arise, payment will be issued on 2025-01-02. :confetti_ball:

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

  • @pac-guerreiro does not require payment (Contractor)
  • @DylanDylann requires payment (Needs manual offer from BZ)

melvin-bot[bot] avatar Dec 26 '24 20:12 melvin-bot[bot]

Issue is ready for payment but no BZ is assigned. @mallenexpensify you are the lucky winner! Please verify the payment summary looks correct and complete the checklist. Thanks!

melvin-bot[bot] avatar Jan 02 '25 09:01 melvin-bot[bot]

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 (@mallenexpensify)

  • [ ] 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

melvin-bot[bot] avatar Jan 02 '25 09:01 melvin-bot[bot]

@DylanDylann does this require payment or is it a regression from a previous PR?

puneetlath avatar Jan 02 '25 20:01 puneetlath

I think no

DylanDylann avatar Jan 03 '25 09:01 DylanDylann

Got it. Closing out then!

puneetlath avatar Jan 03 '25 15:01 puneetlath