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 2023-08-30] Re-render the ReportActionItem when transaction is deleted #24608

Closed
mountiny opened this issue Aug 15, 2023 · 26 comments
Assignees
Labels
Internal Requires API changes or must be handled by Expensify staff NewFeature Something to build that is a new item. Reviewing Has a PR in review Weekly KSv2

Comments

@mountiny
Copy link
Contributor

mountiny commented Aug 15, 2023

Following on this PR #24590, once we are going to switch from using IOU report actions for the transaction dat to transaction objects which should be done here #24345, we should stop hiding the MoneyRequestView as we do here but we should add transaction as props to the ReportActionItem and re-render when it changes. Deleted transaction is wiped from the onyx.

cc @dukenv0307

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~0195265ecd3403c746
  • Upwork Job ID: 1692289426335956992
  • Last Price Increase: 2023-08-17
@mountiny mountiny added Daily KSv2 NewFeature Something to build that is a new item. labels Aug 15, 2023
@mountiny mountiny self-assigned this Aug 15, 2023
@melvin-bot
Copy link

melvin-bot bot commented Aug 15, 2023

@melvin-bot melvin-bot bot added Weekly KSv2 and removed Daily KSv2 labels Aug 15, 2023
@melvin-bot
Copy link

melvin-bot bot commented Aug 15, 2023

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

@mountiny
Copy link
Contributor Author

No need for design review here. sorry for the ping

@mountiny
Copy link
Contributor Author

I will ask in Callstack or SWM channels if anyone wants to take this one

@luacmartins
Copy link
Contributor

I can work on this one

@luacmartins luacmartins assigned luacmartins and unassigned mountiny Aug 17, 2023
@luacmartins luacmartins added the Internal Requires API changes or must be handled by Expensify staff label Aug 17, 2023
@melvin-bot
Copy link

melvin-bot bot commented Aug 17, 2023

Job added to Upwork: https://www.upwork.com/jobs/~0195265ecd3403c746

@melvin-bot melvin-bot bot added the Reviewing Has a PR in review label Aug 17, 2023
@melvin-bot melvin-bot bot removed the Weekly KSv2 label Aug 17, 2023
@melvin-bot
Copy link

melvin-bot bot commented Aug 17, 2023

Triggered auto assignment to Contributor Plus for review of internal employee PR - @eVoloshchak (Internal)

@melvin-bot melvin-bot bot added the Weekly KSv2 label Aug 17, 2023
@luacmartins
Copy link
Contributor

PR up - #25435

@JmillsExpensify
Copy link

Nice, I see that PR was merged!

@luacmartins
Copy link
Contributor

Yes, should go to prod in today's deploy

@Pujan92
Copy link
Contributor

Pujan92 commented Aug 22, 2023

@luacmartins @mountiny Seems regression https://expensify.slack.com/archives/C049HHMV9SM/p1692729074593069 from this PR as comment structure inconsistency after edit money request.

Screenshot 2023-08-22 at 23 58 06

For newly request money we are making the. comment structure as an object here

const commentJSON = {comment};

But on the edit money request we received the comment as a string in the api onyx response and that makes the inconsistency.

@luacmartins
Copy link
Contributor

I think this has already been fixed here already

@Pujan92
Copy link
Contributor

Pujan92 commented Aug 24, 2023

Seems to reproduce with the open report response.

Screen.Recording.2023-08-25.at.01.22.13.mp4

@luacmartins
Copy link
Contributor

We're currently refactoring this command. I'll check again once those PRs have been deployed

@luacmartins luacmartins changed the title Re-render the ReportActionItem when transaction is deleted [HOLD for payment 2023-08-30] Re-render the ReportActionItem when transaction is deleted Aug 25, 2023
@luacmartins
Copy link
Contributor

The App PR was deployed to prod 2 days ago. Updating the title with the payment date.

@garrettmknight
Copy link
Contributor

@luacmartins could this be causing #26205 as well?

@luacmartins
Copy link
Contributor

No, I don't think that issue is related to this.

@CortneyOfstad
Copy link
Contributor

So according to this SO @eVoloshchak gets paid via eChat. Has payment been received? If not, please let me know and we can get that bumped. TIA!

@eVoloshchak
Copy link
Contributor

@CortneyOfstad, I just requested the payment via NewDot, thanks!

@CortneyOfstad
Copy link
Contributor

@eVoloshchak just confirming if the payment was successful or not? If so, we can close this out 👍

@JmillsExpensify
Copy link

@CortneyOfstad This is missing a payment summary. I need that before I can pay via NewDot.

@CortneyOfstad
Copy link
Contributor

From what I can see, Eugene was assigned to the issue on 8/17. It shows that the PR went into production on 8/22, but it shows that a QA delayed the PR going into production, so while they did not qualify for the bonus, they would not receive the penalty.

Payment due: $1000 to @eVoloshchak

CC @JmillsExpensify

@CortneyOfstad
Copy link
Contributor

@JmillsExpensify @eVoloshchak was payment confirmed in NewDot? Just checking to see if this can be closed — thanks!

@eVoloshchak
Copy link
Contributor

@CortneyOfstad, it wasn't confirmed yet, but I requested it, I think the issue can be closed (we can open it back again if needed)

@JmillsExpensify
Copy link

$1,000 payment for @eVoloshchak approved based on BZ summary.

@CortneyOfstad
Copy link
Contributor

Looks good to close! Thanks everyone!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Internal Requires API changes or must be handled by Expensify staff NewFeature Something to build that is a new item. Reviewing Has a PR in review Weekly KSv2
Projects
None yet
Development

No branches or pull requests

8 participants