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

feat: Apply default tag on recipe import (by URL) #2930

Draft
wants to merge 22 commits into
base: mealie-next
Choose a base branch
from

Conversation

boc-the-git
Copy link
Collaborator

What type of PR is this?

  • feature

What this PR does / why we need it:

The intent here is to allow for an "inbox" type of tag, that would get populated on every URL import and allows a user to review recipes that they haven't reviewed/checked for accuracy.
It might be that someone wants to go in and rename foods. Someone else might want to confirm nutrition looks right, etc..
Many reasons someone might want to confirm the end result of the parsing is exactly what they'd want.

This is just an enabler to make that easier to do - the tag gets added to all recipes imported by URL, bulk or single. It does not get added to other import types (e.g. zip, migration) as it's assumed those are already coming in in a clean state.

By default it's disabled, and requires the user to take action to enable it (in group settings).

Which issue(s) this PR fixes:

None.

Testing

Manual for the most part. Have added unit tests as well.

@boc-the-git
Copy link
Collaborator Author

boc-the-git commented Jan 9, 2024

Few minor issues to work through, will sort in the coming days and raise a new PR. Closing the PR might've been a bit dramatic, the effort to resolve type errors wasn't that great - though I really do need to be in bed!

@boc-the-git boc-the-git closed this Jan 9, 2024
@boc-the-git boc-the-git reopened this Jan 9, 2024
@boc-the-git
Copy link
Collaborator Author

boc-the-git commented Jan 9, 2024

I guess SQLite doesn't enforce the foreign key constraints in tests, but Postgres does. Will resolve hopefully tomorrow. Review/comments are welcome in the meantime should anyone wish - I'm expecting the subsequent changes will be local to the unit test only.
Solution is to actually create a tag, not just generate a uuid.

@boc-the-git boc-the-git marked this pull request as draft January 12, 2024 11:41
@boc-the-git
Copy link
Collaborator Author

Have set this back to draft because I need to actually test the recent changes. Expecting it to go smoothly, but it won't get done today.

@boc-the-git
Copy link
Collaborator Author

Tested and ready for review again :)

@hay-kot
Copy link
Collaborator

hay-kot commented Jan 22, 2024

We have this old issue/task

That I think has some cross concerns here. This implementation is very specific, and I'm wondering if there is a better way to generalize the implementation to be more flexible. Otherwise if #1106 ever gets implemented most of this work would likely have to be redone/removed.

Having a db structure for Import Automations might be a good step? We could store something like this

class ImportAutomation:
    rules: RegexAutomation | AlwaysAutomation | TagAutomation # json column maybe? 
    type: str
    groupd_id: UUID 
    order: int

Then we could have multiple rules that could apply in series on every import.

Copy link
Contributor

github-actions bot commented Mar 8, 2024

This PR is stale because it has been open 45 days with no activity.

@github-actions github-actions bot added the stale label Mar 8, 2024
@boc-the-git
Copy link
Collaborator Author

FWIW, I agree with the prior comment and it is my intention to at some point make this more generalised.. I have no timeline for when though.

@boc-the-git boc-the-git marked this pull request as draft March 27, 2024 09:08
@github-actions github-actions bot removed the stale label Mar 28, 2024
Copy link
Contributor

This PR is stale because it has been open 45 days with no activity.

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