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: Changed Recipe's Last Made button to allow editing Last Made Date. #4986

Open
wants to merge 6 commits into
base: mealie-next
Choose a base branch
from

Conversation

PancakeZik
Copy link
Contributor

@PancakeZik PancakeZik commented Jan 29, 2025

There is currently no way of changing or removing the "last made" date.
Added option in the UI to do so.
Changed API to accept null for Last Made date so it can be set back to Never.

Which issue(s) this PR fixes:

Feature Request #4974

Special notes for your reviewer:

I added some styles to RecipeLastMade.vue, but I'm not sure this is the correct place to do so?

@PancakeZik PancakeZik changed the title Feat: Changed Recipe's Last Made to accept clicking Last Made Date to edit. feat: Changed Recipe's Last Made to accept clicking Last Made Date to edit. Jan 30, 2025
@PancakeZik PancakeZik marked this pull request as draft January 30, 2025 08:16
@PancakeZik PancakeZik changed the title feat: Changed Recipe's Last Made to accept clicking Last Made Date to edit. feat: Changed Recipe's Last Made button to allow editing Last Made Date. Jan 30, 2025
@PancakeZik PancakeZik marked this pull request as ready for review January 30, 2025 10:34
max-width="290px"
min-width="auto"
>
<template #activator="{ on, attrs }">

Choose a reason for hiding this comment

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

Indentation went a bit wonky!

// If lastMade exists, init the date picker to that date, else set to 'today'
lastMadeEdit.value = lastMade.value
? new Date(lastMade.value).toISOString().substring(0, 10)
: new Date().toISOString().substring(0, 10);

Choose a reason for hiding this comment

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

Can you just do new Date(lastMade.value).toISOString().substring(0, 10) ; because if it's null it'd back back to current date? (or pull the bit that is different - last edited || now) out to DRY?

@@ -459,7 +459,8 @@ def update_last_made(self, slug: str, data: RecipeLastMade):
"""Update a recipe's last made timestamp"""

try:
recipe = self.service.update_last_made(slug, data.timestamp)
# If timestamp is None, update with None
recipe = self.service.update_last_made(slug, data.timestamp if data.timestamp is not None else None)

Choose a reason for hiding this comment

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

Wouldn't

Suggested change
recipe = self.service.update_last_made(slug, data.timestamp if data.timestamp is not None else None)
recipe = self.service.update_last_made(slug, data.timestamp)

make more sense? If it's !!None, doesn't that mean it's None?

@CloCkWeRX
Copy link

Seems mostly sensible; not having python as a daily driver the !!None thing seems a bit odd; but if it's idiomatic... great.

@PancakeZik
Copy link
Contributor Author

Makes sense, thanks for that! Looking forward to having it merged

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.

None yet

2 participants