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

recipe-api returns multiple types for dateModified #1121

Closed
newhinton opened this issue Jul 27, 2022 · 12 comments
Closed

recipe-api returns multiple types for dateModified #1121

newhinton opened this issue Jul 27, 2022 · 12 comments
Assignees
Labels
API changes involving using the app api Backend Issue or PR related to the backend code bug Something isn't working stale The PR/issue is marked as stale stalebot-enabled The PR/issue is marked to go stale over time

Comments

@newhinton
Copy link

newhinton commented Jul 27, 2022

Description
/apps/cookbook/api/recipes provides a json list with all recipes.
Each recipe contains a dateModified. As per documentation, this should be a string of this format: "2021-04-28T09:52:59+0000"

Sometimes it does not return such a string, but a unix timestamp. In such cases, dateCreated also does not behave correctly, it should also return the above string, but when dateModified is a unix timestamp, it is usually '0' (of type int)

I even have some entries where both dateCreated and dateModified are not int, but string while beeing unix timestamps.

Reproduction
Steps to reproduce the behavior:

Sadly, i am not able to find a common denominator what makes the "broken" entries the same and different from the rest.

Expected behavior
All recipes use the same format and deal with problems in the backend.
Eg. if dateCreated is not set, it should be the same as dateModified (and stay that way)

Actual behavior
dateModifieddoes not have a clearly defined type and format

Versions
Nextcloud server version: 23
Cookbook version: 0.9.13
Database system: MariaDB

@newhinton newhinton added the bug Something isn't working label Jul 27, 2022
@MarcelRobitaille MarcelRobitaille added Backend Issue or PR related to the backend code API changes involving using the app api labels Jul 27, 2022
@christianlupus
Copy link
Collaborator

Could you please send me an offending recipe? I suspect you know at least one. Just go to the recipe folder and zip the complete folder named as the recipe and send the zip here, if you do not mind.

I suspect that it was generated with an older version that did not filter the dates. I will try to get that fixed but cannot promise it to be released in the next few days. There is a plan ahead but that needs quite some code leverage to be implemented. Eventually, we will need to make a short fix.

@christianlupus
Copy link
Collaborator

Related to #1110

@newhinton
Copy link
Author

Sure!
Italienischer Pizzateig.zip

Oh no worries, i am totally fine if this is only fixed when the stable v1 api is released.

@christianlupus christianlupus self-assigned this Jul 27, 2022
@christianlupus
Copy link
Collaborator

I just had a quick glance at this. With the #1097 merged, that problem is at least partly fixed, as far as I understand. With #1110 this can be made even more stable.

Let's have a look at v0.9.14 if that one fixed your problem.

@christianlupus christianlupus added the stalebot-enabled The PR/issue is marked to go stale over time label Aug 2, 2022
@christianlupus
Copy link
Collaborator

The stale bot will be effective after the next release. So no worries.

@newhinton
Copy link
Author

Oh no, please dont add a stale-bot. They are the most annoying development on github in the past. They tend to keep closing valid issues, encurage "oneplussing", annoy everyone that is subscribed to a ticket and possibly drive down interactions with new people.

Is it really nessessary to have one?

@seyfeb
Copy link
Collaborator

seyfeb commented Aug 2, 2022

Well, it also very helpful, since often users creating an issue completely forget about it (which is totally human) and do not close it if it is resolved. Considering the very small amount of developers in this project, it is quite helpful not to have to read through all the issues regularly just to ask the issue creators if the issue is still valid over and over again ;)

@christianlupus
Copy link
Collaborator

The next release 0.9.14 will be not too long. Normally, I try to push out a release monthly to get the translations out as well. We are almost due for the next release. However there are still some pending issues/PRs open, so it will be eventually 2 or 3 weeks.

The stale bot will not take action before 45 days (!), which is way after the release.

I suspect that a recent refactory of some code solved the problem already. See my comment. You can test with the current development version if you like. We will have to see if the problem is still present then. I was not able to reproduce it anymore but I had only one test case.

@newhinton
Copy link
Author

newhinton commented Aug 5, 2022

Well, it also very helpful, since often users creating an issue completely forget about it (which is totally human) and do not close it if it is resolved. Considering the very small amount of developers in this project, it is quite helpful not to have to read through all the issues regularly just to ask the issue creators if the issue is still valid over and over again ;)

@seyfeb But this is also not the burden of the ticket-creator. If an issue is beeing worked on (or fixed) then the one who solves it should close it.

There are many valid reasons why a ticket may be closed (outdated, fixed, not enough information, duplicate, etc)

None of them can be detected by bots (that's not entirely true, there is a duplicate bot somewhere) and need some form of interaction from a human.

Closing tickets merely because a timeout expired is nearly always a bad idea. It leads to annoyance and closed issues that are actually not solved. That is bad.

@christianlupus I am not worried about this particular ticket. I am generally opposing stalebots because imho they do more harm than good and are a solution to a problem that does not exist in the first place

@github-actions
Copy link

This issue was not updated for 45 days. It is therefore marked as stale. When no update occurs within the next 7 days, this issue will be closed automatically in the next 7 days.

@github-actions github-actions bot added the stale The PR/issue is marked as stale label Sep 20, 2022
@newhinton
Copy link
Author

I have tested this behaviour in 0.9.15. It seems the endpoint moved to /apps/cookbook/webapp/recipes, and the issue seems to be fixed. @christianlupus is this the case that the endpoint moved? Which api's should external apps use? I assume the webapp-one is "unstable" and may change at any time?

@christianlupus
Copy link
Collaborator

Yes, you are right. The /webapp endpoints are for internal usage only. You would need to provide credentials and CSRF token to authenticate. I will provide documentation (also for internal usage) but it might change without prior notification or synchronization.

There is migration information available on the GitHub pages documentation. You see, that all endpoints are located under /api/v1. The old endpoints are still present but no longer accept session-based authentication and are deprecated. The deprecated endpoints have been deleted in the meantime in the dev version and will be rolled out with the next release.

BTW: If you are developing an extension to the app that uses the API, are you using Matrix by chance? There is a room dedicated to 3rd party extensions where such API changes are announced. Apart from that, I wrote some lines in #1199.

I suspect that the problem has settled in the meantime. If it reappears, feel free to comment, we can reopen then.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
API changes involving using the app api Backend Issue or PR related to the backend code bug Something isn't working stale The PR/issue is marked as stale stalebot-enabled The PR/issue is marked to go stale over time
Projects
None yet
Development

No branches or pull requests

4 participants