-
-
Notifications
You must be signed in to change notification settings - Fork 719
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
Feature: "I Made This" Dialog #1801
Merged
hay-kot
merged 10 commits into
mealie-recipes:mealie-next
from
michael-genson:feat/I-made-this
Nov 13, 2022
Merged
Feature: "I Made This" Dialog #1801
hay-kot
merged 10 commits into
mealie-recipes:mealie-next
from
michael-genson:feat/I-made-this
Nov 13, 2022
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
changed timeline event "message" -> "event_message" added "last made" timestamp to recipe
removed references in action menu and context menu refactored dialog to be triggered by a button instead added route to update recipe last made timestamp added visual for last made timestamp to recipe header and title
hay-kot
requested changes
Nov 11, 2022
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me overall. Just need to take another look at the Migration and see if we can use a Pydantic Alias to avoid altering the column since SQLite doesn't support that feature which will put is in a weird state.
alembic/versions/2022-11-03-13.10.24_1923519381ad_renamed_timeline_event_message_and_.py
Outdated
Show resolved
Hide resolved
@hay-kot should be good now :) |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What type of PR is this?
(REQUIRED)
What this PR does / why we need it:
(REQUIRED)
This adds a "Last Made" property to recipes and a new "I Made This" dialog on the recipe page. It creates a new event on the recipe event timeline and updates the recipe's
lastMade
timestamp to today (unless the user chooses a specific date).The last made date is also displayed on the recipe page, and recipes can be sorted by this property.
Works on both landscape and non-landscape view, and does not appear on the public recipe page. The date format is localized
If the recipe has never been made
The comment isn't viewable in the UI yet, pending a recipe timeline view
Which issue(s) this PR fixes:
(REQUIRED)
Works towards #1451. This utilizes the timeline API, but does not implement the timeline frontend.
Special notes for your reviewer:
(fill-in or delete this section)
Originally I had this in the recipe Action/Context menus, but I moved it to the recipe header/title instead since I think it works better there. It also made manipulating the last made date significantly easier.
I wasn't sure how to add new text localizations, so at the moment everything is just in English. If we want to add localizations in this PR I'm happy to do so, if you could help point me in the right direction. Not too much text was added, and it was only added in two places (the new component and the sort menu) so it should be pretty easy.
Also, to avoid the plugin conflict we talked about on discord, I modified the timeline event API to use
event_message
instead ofmessage
. This ended up being the easiest solution by far, since the plugin directly accesses thePOST
response. I also needed a new database migration for this PR anyway.Testing
(fill-in or delete this section)
I ran through the following scenarios:
Release Notes
(REQUIRED)