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: Can't Delete All Notes #2759

Merged

Conversation

michael-genson
Copy link
Collaborator

What type of PR is this?

(REQUIRED)

  • bug

What this PR does / why we need it:

(REQUIRED)

If you add notes to a recipe, save, then remove all of them, save, the notes aren't deleted. I'm not sure why, but I traced it back to our update function - when you pass an empty list of notes to the SQLAlchemy object, it just.... ignores it?

To fix this, I added an explicit check:

def update(self, *args, **kwargs):
    self.__init__(*args, **kwargs)  # this used to be the only line

    for k, v in kwargs.items():
        if hasattr(self, k) and v == []:
            setattr(self, k, v)

What's weird is that it works for other relationships (categories, tools, etc).

Which issue(s) this PR fixes:

(REQUIRED)

Fixes #2756

Testing

(fill-in or delete this section)

Tested through the frontend manually, and added a new backend test.

Copy link
Contributor

@p0lycarpio p0lycarpio left a comment

Choose a reason for hiding this comment

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

Tested on my side and it works. Thanks !

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.

Looks good Michael! :)

@boc-the-git boc-the-git merged commit 416d752 into mealie-recipes:mealie-next Nov 26, 2023
6 checks passed
@michael-genson michael-genson deleted the fix/delete-all-notes branch November 26, 2023 02:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[BUG] - Deleting recipe notes doesn't work
3 participants