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

fix: Bulk Update Owner Removes Some Recipe Data #4393

Conversation

michael-genson
Copy link
Collaborator

What type of PR is this?

(REQUIRED)

  • bug

What this PR does / why we need it:

(REQUIRED)

For more insight into why this happened, see: https://github.com/mealie-recipes/mealie/pull/2168/files

Which issue(s) this PR fixes:

(REQUIRED)

N/A, reported on Discord

Testing

(fill-in or delete this section)

Manually verified data is not lost

Copy link
Collaborator

@boc-the-git boc-the-git left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you summarise the problem in a couple sentences?

Why is patch working for you in a way out didn't? Patch updates only the fields changed, whereas put is replacing the whole object?

I added a note suggesting adding logic into the test confirming those differences could be good.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems like we expect a different end result from patch, compared to put. Could be good to add validation of that difference in the test?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unfortunately this is a frontend issue, not backend, so I can't really write a test for it. The frontend is sending a valid payload to the backend, which is accepted. It's just missing data

@michael-genson
Copy link
Collaborator Author

Basically on the frontend we only have a RecipeSummary which doesn't have instructions, assets, etc. When calling update the backend assumes this is a Recipe, and defaults to empty instructions, assets, etc.

The patch method merges the old data with the new data before committing to the database

@michael-genson michael-genson merged commit 543a53c into mealie-recipes:mealie-next Oct 19, 2024
13 checks passed
@michael-genson michael-genson deleted the fix/bulk-update-owner-bug branch October 19, 2024 20:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants