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

Add ability to open recipes with specified number of servings from meal plan #2938

Merged

Conversation

patmagauran
Copy link
Contributor

When creating a meal plan, you can specify the number of servings to make which influences the amount of ingredients added to your shopping list. Then, when clicking the recipe from the plan it currently defaults to the recipe's default number of servings. Instead, this PR makes it so it defaults to the number in the meal plan. This only takes effect when navigating from the meal plan and should not interfere or change behavior elsewhere.

I have also added a toggle in the meal plan settings to enable/disable this functionality.

The loading of the correct number of servings is done by introducing an additional url format for recipes of /recipe/id-servings. If the -servings is omitted, it loads as normal. If it is included, the recipe will load with the specified number of servings.

I am open to any changes in the handling of this as well. I have limited knowledge of django and vue so if there is a better way to propagate the number of servings, I will happily implement it as such.

sample URL: http://127.0.0.1:8000/view/recipe/1-3 -- Opens recipe 1 with 3 servings

The modified Settings page:
image

@vabene1111
Copy link
Collaborator

Hi, thanks for the PR. I like the idea but I think there is a much easier way to do this. Please feel free to next time share a concept before investing all the work so I can tell you beforehand.

What I suggest is adding a normal named GET parameter to the recipe view /view/recipe/1/?servings=5 and then load this number in the mounted or after the API retrieve in the recipe view. I would also say that in this case you don't need a setting for the feature and the change is only something like 3 lines of code :)

@patmagauran
Copy link
Contributor Author

I went with the way I did it because I did not see any places you used named parameters and didn't want to break the pattern. It is the more standard way to handle it. The setting doesn't disable the url configuration either, it only disables the meal plan link change. Probably not necessary but I figured there may be some people who prefer that. Although a better way IMO would be to show plainly both the number of servings a recipe is currently being viewed with and the number that it defaults to with a quick option to reset. This may be important for baking recipes where different amounts mean different cooking times, etc.

I am open to changing it to the way you think best, no worries on the unnecessary work on my part

@vabene1111
Copy link
Collaborator

perfect, I see your reasoning.
I would say

  • move to named parameter (I want to migrate to that pattern for those kind of things)
  • remove the setting

I also agree to your point that the "original" servings should be shown in the recipe view, but thats a different matter for #2220

@patmagauran
Copy link
Contributor Author

I have updated it accordingly

@vabene1111
Copy link
Collaborator

perfect, thanks for the changes, this looks good to me.

@vabene1111 vabene1111 merged commit b99443c into TandoorRecipes:develop Feb 16, 2024
3 checks passed
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.

None yet

2 participants