Join GitHub today
GitHub is home to over 36 million developers working together to host and review code, manage projects, and build software together.Sign up
API docs are wrong for GET entity (ignores serialization group) #701
Is your feature request related to a problem? Please describe.
but actually it returns:
the fields: "comment", "fixed_rate", "hourly_rate", "project" are missing.
expedted output from /api/customers
output I get:
expected output from /api/projects
output I get:
Describe the solution you'd like
Hm, I still don't know if that is a "bug" in the API documentation module or me being too stupid to configure it properly. I guess the latter one... you are right, the output is different from the docu, BUT:
The collection calls return a limited data-set as configured e.g. for Activity:
The master includes the
If you need some changes in the collection results, please open a PR with the config changes (in this directory). Be aware: there is come aggressive caching going on in the API libraries. If config changes won't load, don't use the
I'll leave this open to fix the documentation.
So do I understand you correctly? - with setting groups to [Default] it should be included in both result sets?
I don't think it is like this, because at the moment my Entitiy.Activity.yml looks like this
And the fields are not showing as expected. So this is the config which I use now: everything set to Collection and project include/exclude changed:
I deleted and recreated the cache folder, warmed up cache again with
but the output is the same:
Yes correctly explained. BUT: only fields which have a value are rendered.
Hm, if you add a customer to an activity and hit the save button it says "saved changes successfuly" - but actually it's not saved. It only gets saved if you enter a project, too:
I think it'd make more sense to to always show all fields with their "real" value - this is the behavior I know from many others APIs, too.
For example if an activity is not allocated to a project -> project: null. It's a pain in the ass if you always have to check first "is the key there" and if the key is there check "what's the value of the key". It's just unnecessary complex.
An activity doesn't have a customer, the field is only there to make the project selection more convenient. If you have any idea on how to make that clearer / improve the UX: please let me know.
Regarding the API: yes, it needs improvement... I still hope someone jumps in who has knowledge of the used libraries, as I don't have the time right now :(
maybe make both fields required?
I've tested the api with some GET Requests in Postman and it seems to work now as expected:
I've tested the POST request which is necessary for the Kimai 2 app, too.
I saw you've removed the customer from the POST request /api/timesheets.
The response of the api looks like this:
Is it possible to return mysql datetime format, too?
Since I don't use the date I get returned from api it doesn't matter if this is in a different format - but the datetime format in the documentation should be changed.
@kevinpapst hm, I've tested my app again and noticed - since the removal of the customer in the activity post request - my view of saved times doesn't work like it did before.
When I've tried to send the json object with the key "customer" in the object, the api responded with the error "This form should not contain extra fields." I fixed this with just not saving the customer when saving a time. But with this now I can't access the saved customers and show the customer's name in the overview.
I could do it like this: save the customer and remove the customer key before sending the object to the api. But in case the requests fails I have to add the customer key to the saved object again...
Is there a way to turn off the validation of extra fields in the form? What do you suggest on how to solve this?
Thanks for the feedback!
But some problems may arise now, as it definitely contains a couple of BC breaks. Sorry! But I have to do it now, before it is actively used and can't be changed any longer.
The documentation is correct and matches expected input and output. AFAIk all your points are good findings but still valid, so let me explain that in detail for a better understanding.
Can your app show existing entries from the API? If so, how do you get the customer there? If not: maybe a feature for the future? In both cases you want to use the same data structure, right?
I don't know if strict validation could be turned off, but actually I don't really want to. I understand that it is frustrating as it worked before and I am really sorry for the breaking changes!!!!
I think you shouldn't store the customer in the first place with the timesheet record.
Output -contains the timezone information:
Every date library should have built-in support for those two formats, so I'd say we are save to use them. I am against over engineering, but I want to prevent terrible mistakes that will bite me in the ass later on. And I believe this is the best choice we have, considering that Kimai handles timezones. Mysql Datetime is a vendor specific format but almost like RFC 336, just replace the space with a 'T'. From my experience, most libraries can handle both variants, with and without T.
As dates should always be handled with a library, I expect the change in your app is "just" a matter of switching a constant in the formatter.
To make it short: docu shows what is really given and expected and I don't want to change that, unless you have good technical arguments.
How do you store the dates in the app?
Thanks for your explanation!
Date: Accepted. Currently I use moment.js with the users local datetime and send it to the backend. So no changes here for now.
Customer: Good idea with the hashmap. Will implement it.
What else will be changed?
The PR is complete now. So you can update and give it a try. I did a lot of changes last night and there are new methods as well...
I introduced two BC breaks:
The second part was done yesterday. Unfortunately the library used for serialization uses snake_case by default and I didn't turn that off in the first place. But it brought a lot of problems and didn't match the internal variable names, thus bringing a lot of extra effort on the server side (and it was impossible to fix the documentation for post and patch).
Again: sorry for the BC changes, I hope it is not too much trouble to change it on your end.