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

Support unspecified to/from dates for fetching meal plan ical #3004

Merged
merged 6 commits into from
Mar 1, 2024

Conversation

c0mputerguru
Copy link
Contributor

I've combined #2978 and #2977 and written up some basic unit tests for the API.

@vabene1111
Copy link
Collaborator

awesome, thank you very much!

@vabene1111 vabene1111 merged commit d040757 into TandoorRecipes:develop Mar 1, 2024
4 checks passed
@vabene1111
Copy link
Collaborator

I had to disable the changes made in this PR and comment out the PR. The reason is that multiple urls with different parameters lead to conflicting operation IDs on schema generation.

On the feature/vue3 branch we have djang spectacular which changes how the API schema is generated so this might be a possibility to fix this. As getting the new frontend working is the number 1 priority I will not work out a proper fix for this on the old branch. We can take another look at this and also at implementing a proper ical server after the frontend rewrite.

@c0mputerguru
Copy link
Contributor Author

Can you provide a bit more context on the error so I can try to debug it further? I simply followed the example in https://docs.djangoproject.com/en/5.0/topics/http/urls/#example that had different sets of parameters.

@c0mputerguru
Copy link
Contributor Author

I'm going to guess that the problem is with the operation id not being unique for the 3 different APIs? In particular, both the from_date and from_date/to_date APIs look like they have the same operation id: retrieveget_plan_ical.

Poking around, it looks like you can override the schema for the function (https://www.django-rest-framework.org/api-guide/views/#view-schema-decorator). It's not clear to me whether or not its a good idea to override the _get_operation_id() function, but that's what it seems like the answer is in encode/django-rest-framework#6739.

That said, if we're moving forward with the new frontend, I agree that its probably not worth trying to figure it out on the old one.

@c0mputerguru
Copy link
Contributor Author

I've put together #3038 that should fix the API issues in both the old frontend and the new frontend.

@vabene1111
Copy link
Collaborator

thanks, sorry I did not reply in time, I will continue discussion on the new PR

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.

2 participants