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

Mealplan improvements without the scrolling overhaul #2497

Closed

Conversation

Grygon
Copy link
Contributor

@Grygon Grygon commented Aug 13, 2023

What type of PR is this?

  • bug
  • cleanup
  • feature

What this PR does / why we need it:

  • Re-adds the ratings to recipes on the Meal Plan
  • Adds the ability to specify quantities to recipes that have been added to the meal plan
  • Validates that note titles are populated
  • Moves the daily scheduled tasks to occur at midnight, so that the Mealplan -> Timeline events happen at the end of the day, and don't result in duplicates when people want to mark their meals as completed themselves

Which issue(s) this PR fixes:

Fixes #2256
Fixed #2446

Special notes for your reviewer:

Same as PR #2399, just cherry-picked around the changes for the scrolling UI

Testing

For frontend changes, tested manually and confirmed data displays appropriately now.

Ratings

Visually verify stars are present.
image

Quantities

Validate that the recipe indicates a quantity when it isn't 1.
image

Add Recipes to the Meal Plan in all possible workflows and verify that specified quantities populate:
image
image

Verify that the Edit page properly propagates quantities when modified
image

For backend changes, modified existing unit test and did additional validation via API calls.

Release Notes

Show ratings on the meal plan.
Associate quantities with recipes on a meal plan
Perform validation on meal plan notes
Daily scheduled tasks now occur at midnight

@hay-kot hay-kot force-pushed the mealplan-improvements-no-layout branch from 3d36cfd to ce35e3f Compare October 7, 2023 21:19
@@ -19,7 +19,8 @@


def create_mealplan_timeline_events(group_id: UUID4 | None = None):
event_time = datetime.now(timezone.utc)
# Move it back by 1 minute to ensure that midnight processing logs it to the prev day
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is this following a precedent set somewhere else?
Personally I consider 00:00:00 on 27/10/2023 to be on 27/10. Sounds like you're wanting to say that's actually 26/10?

Ultimately consistency across the app is the most important thing, hence wondering if there's a precedent.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The thought is that it's "end of day" processing--at midnight, it handles events for the previous day.
So since we're doing this at midnight, we want our event to happen at 23:59 to log it to the day that it was assigned to.

Copy link
Contributor

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

@github-actions github-actions bot added the stale label Jan 24, 2024
@boc-the-git
Copy link
Collaborator

I'm quite invested in this FWIW, just time poor. The meal planner is all but my most used feature and improvements are hugely welcome.
I might look to take parts of this and split it out into separate smaller PRs, where it that makes sense.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants