API docs are wrong for GET entity (ignores serialization group)

[j0hannesr0th]

Is your feature request related to a problem? Please describe. The documentation says /api/activities returns:

[
  {
    "id": 0,
    "name": "string",
    "comment": "string",
    "visible": true,
    "fixed_rate": 0,
    "hourly_rate": 0,
    "project": 0
  }
]

but actually it returns:

[
    {
        "id": 3,
        "name": "act 4 customer 1",
        "visible": true
    },
    {
        "id": 1,
        "name": "develop",
        "visible": true
    },
    {
        "id": 2,
        "name": "test",
        "visible": true
    }
]

the fields: "comment", "fixed_rate", "hourly_rate", "project" are missing.

---

expedted output from /api/customers

[
  {
    "id": 0,
    "name": "string",
    "number": "string",
    "comment": "string",
    "visible": true,
    "company": "string",
    "contact": "string",
    "address": "string",
    "country": "string",
    "currency": "string",
    "phone": "string",
    "fax": "string",
    "mobile": "string",
    "email": "string",
    "homepage": "string",
    "timezone": "string",
    "fixed_rate": 0,
    "hourly_rate": 0
  }
]

output I get:

[
    {
        "id": 1,
        "name": "Customer 1",
        "visible": true
    },
    {
        "id": 2,
        "name": "Customer 2",
        "visible": true
    },
    {
        "id": 3,
        "name": "Customer 3",
        "visible": true
    }
]

---

expected output from /api/projects

[
  {
    "id": 0,
    "name": "string",
    "comment": "string",
    "visible": true,
    "budget": 0,
    "order_number": "string",
    "fixed_rate": 0,
    "hourly_rate": 0,
    "customer": 0
  }
]

output I get:

[
    {
        "id": 1,
        "name": "prj 4 cus 1",
        "visible": true,
        "customer": 1
    },
    {
        "id": 2,
        "name": "prj 4 cus 2",
        "visible": true,
        "customer": 2
    },
    {
        "id": 4,
        "name": "prj 4 cus 3",
        "visible": true,
        "customer": 3
    },
    {
        "id": 3,
        "name": "prj 41 cus 1",
        "visible": true,
        "customer": 1
    }
]

Describe the solution you'd like Please add the missing fields the way they are described in the documentation, because I need to know for the Kimai 2 app which activities are global activities and which are activities allocated to a project.

[kevinpapst]

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 behavior is not a bug but intended.

The collection calls return a limited data-set as configured e.g. for Activity: https://github.com/kevinpapst/kimai2/blob/master/config/serializer/App/Entity.Activity.yml There are different serialization groups: Entity (only returned for an explicit GET on the ID), Collection (included in the GETs for the entire collection) and Default (included in both result sets).

The master includes the project field in the activities collection, which is the identifier you need to check. If it is missing, the activity is global.

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 cache:clear command, but delete the entire cache directory rm -rf var/cache/*.

I'll leave this open to fix the documentation.

[j0hannesr0th]

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

App\Entity\Activity:
    exclusion_policy: All
    custom_accessor_order: [id, name, comment, visible, project, fixedRate, hourlyRate]
    properties:
        id:
            include: true
            groups: [Default]
        name:
            include: true
            groups: [Default]
        comment:
            include: true
            groups: [Entity]
        visible:
            include: true
            groups: [Default]
        fixedRate:
            include: true
            groups: [Default]
        hourlyRate:
            include: true
            groups: [Default]
        project:
            include: false
            exclude: true
            groups: [Default]
    virtual_properties:
        getProject:
            serialized_name: project
            exp: "object.getProject() === null ? null : object.getProject().getId()"
            type: integer
            groups: [Default]

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:

App\Entity\Activity:
    exclusion_policy: All
    custom_accessor_order: [id, name, comment, visible, project, fixedRate, hourlyRate]
    properties:
        id:
            include: true
            groups: [Collection]
        name:
            include: true
            groups: [Collection]
        comment:
            include: true
            groups: [Collection]
        visible:
            include: true
            groups: [Collection]
        fixedRate:
            include: true
            groups: [Collection]
        hourlyRate:
            include: true
            groups: [Collection]
        project:
            include: true
            exclude: false
            groups: [Collection]
    virtual_properties:
        getProject:
            serialized_name: project
            exp: "object.getProject() === null ? null : object.getProject().getId()"
            type: integer
            groups: [Collection]

I deleted and recreated the cache folder, warmed up cache again with

 php bin/console cache:warmup --env=prod

but the output is the same:

[
    {
        "id": 3,
        "name": "act 4 customer 1",
        "visible": true
    },
    {
        "id": 1,
        "name": "develop",
        "visible": true
    },
    {
        "id": 2,
        "name": "test",
        "visible": true
    }
]

[kevinpapst]

Yes correctly explained. BUT: only fields which have a value are rendered. So if you don't see project the activity is global, if you don't see hourly_rate then none is set. Besides the comment all columns are already exposed on the collection call. That is working for me as expected.

[j0hannesr0th]

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.

[kevinpapst]

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 :(

[kevinpapst]

I think I fixed the API docs and also the serialization of null values. Can you have a look at the attached PR?

[j0hannesr0th]

If you have any idea on how to make that clearer / improve the UX: please let me know.

maybe make both fields required?

---

I've tested the api with some GET Requests in Postman and it seems to work now as expected:

customers:

[
    {
        "id": 1,
        "name": "Customer 1",
        "visible": true,
        "fixed_rate": null,
        "hourly_rate": null
    },
    {
        "id": 2,
        "name": "Customer 2",
        "visible": true,
        "fixed_rate": null,
        "hourly_rate": null
    },
    {
        "id": 3,
        "name": "Customer 3",
        "visible": true,
        "fixed_rate": null,
        "hourly_rate": null
    }
]

---

projects:

[
    {
        "id": 1,
        "name": "Project 1 - Customer 1",
        "visible": true,
        "fixed_rate": null,
        "hourly_rate": null,
        "customer": 1
    },
    {
        "id": 2,
        "name": "Project 1 - Customer 2",
        "visible": true,
        "fixed_rate": null,
        "hourly_rate": null,
        "customer": 2
    },
    {
        "id": 5,
        "name": "Project 1 - Customer 3",
        "visible": true,
        "fixed_rate": null,
        "hourly_rate": null,
        "customer": 3
    },
    {
        "id": 3,
        "name": "Project 2 - Customer 1",
        "visible": true,
        "fixed_rate": null,
        "hourly_rate": null,
        "customer": 1
    },
    {
        "id": 4,
        "name": "Project 2 - Customer 2",
        "visible": true,
        "fixed_rate": null,
        "hourly_rate": null,
        "customer": 2
    },
    {
        "id": 6,
        "name": "Project 2 - Customer 3",
        "visible": true,
        "fixed_rate": null,
        "hourly_rate": null,
        "customer": 3
    }
]

---

activities:

[
    {
        "id": 1,
        "name": "General task - no project",
        "visible": true,
        "fixed_rate": null,
        "hourly_rate": null,
        "project": null
    },
    {
        "id": 2,
        "name": "task - project 1 - customer 1",
        "visible": true,
        "fixed_rate": null,
        "hourly_rate": null,
        "project": 1
    },
    {
        "id": 3,
        "name": "task - project 2 - customer 2",
        "visible": true,
        "fixed_rate": null,
        "hourly_rate": null,
        "project": 4
    }
]

---

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.

In the documentation there is still the wronge date time format for POST example: the begin and end should be in mysql datetime format.

The response of the api looks like this:

{
    "id": 1,
    "duration": 660,
    "rate": 0,
    "description": "string",
    "fixed_rate": 0,
    "hourly_rate": 0,
    "exported": false,
    "begin": "2019-03-24T07:49:00+0100",
    "end": "2019-03-24T08:00:00+0100",
    "activity": 1,
    "project": 1,
    "user": 1
}

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.

[j0hannesr0th]

@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.

Here is how it looked before:

---

Here is how it looks now:

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?

[kevinpapst]

Thanks for the feedback! I am still working on the PR, but you are right: this was a major cleanup and I am the first time happy with the status (not everything is pushed right now).

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.

Customer The customer was removed, as it is not part of a timesheet record. It is related to the project, but that is all about it. I don't need it and it just showed up due to a configuration problem wit the API.

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!!!! But lets be strict now, otherwise I will have to battle with that wrong design decision for the next years.

I think you shouldn't store the customer in the first place with the timesheet record. If you want to show it in the list, grab it directly from the linked project. Easy to say as I I don't know your data structure ;-) but that's what I would propose. Don't store customers with the timesheet records, instead use a customer hashmap for the ID lookup via the project.getCustomer(). This hashmap can be stored until the next sync and should be as fast as storing customers directly with the records.

Dates This one feels a bit weird, as input and output differs - I know. Rule of thumb: if it feels weird, it is wrong. But I thought about that for two days and can't find a better solution. Kimai handles date times with timezones and therefor I decided that the Kimai does all the heavy lifting and the apps simply decide how to render the time. So both time formats you saw are actually the ones I intended to use.

Output -contains the timezone information: I don't know how your frontend works but you have for sure a library like moment.js which can be used to display the ISO 8601 formatted begin and end that you get from the API. Maybe you want to convert it to the users current timezone in a "management view for distributed teams with all times in UTC". Please take that into account. I know that Kimai is used in Europe, the US, Asia, Russia, India ... so basically covering a lot of timezones, make sure your app handles that properly. I think for the start you are good to go if you display the time as given, as this is the users local time. But you never know what the future brings.

Input - does not contain the timezone information: Regarding POST and PATCH I decided to go with HTML 5 local-date (see also RFC 3339) mainly for these reasons:

• its a subset of ISO 8601 and natively supported by moment.js as well with DATETIME_LOCAL / DATETIME_LOCAL_SECONDS
• It is upwards compatible with ISO 8601, so we can add timezone support in the future if needed
• I don't want that API consumers need to add timezone support, you can always use the local time when posting
• It is the format that every browsers sents when using <input type="date-time"> so its basically THE web standard for dates.

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?

[j0hannesr0th]

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.

---

I am still working on the PR, but you are right: this was a major cleanup and I am the first time happy with the status (not everything is pushed right now).

What else will be changed?

[kevinpapst]

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 dates (you already know that one)
• changed variables from snake_case (like hourly_rate) to camelCase (hourlyRate). The name of 4 fields have changed + the i18n-locale configuration object.

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). So I decided here as well: do it now, before any APP is released into the wild...

Again: sorry for the BC changes, I hope it is not too much trouble to change it on your end. For the future I will really try to avoid such BC breaks, I know that they are bad for business. But as long as Kimai is not 1.0 released, I need to cleanup some parts. But I have a strong feeling, that the API is now BC safe ... after so many updates in that PR.

[j0hannesr0th]

@kevinpapst I've finished with refactoring the app. The api works and I just want to share the current status with you:

The app is ready to be deployed...

[kevinpapst]

Wow, really nice! One wish for me: description should be optional

[lock[bot]]

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. If you use Kimai on a daily basis, please consider donating to support further development of Kimai.