osuosl / timesync

A time tracking API
2 stars 1 forks source link

Clarify usage of ?include_deleted on the times endpoints. #53

Closed MorganEPatch closed 8 years ago

MorganEPatch commented 8 years ago

Fixes #52.

athai commented 8 years ago

+1

Kennric commented 8 years ago

Looks ok, but I think would be more clear with a note pointing out the record with the deleted_at set is visible - otherwise you have to read pretty carefully through the json to catch it.

MorganEPatch commented 8 years ago

To be fair, this does come after a section that already also includes GET /activities?include_deleted=true and GET /projects?include_deleted=true, which in turn is after a section that describes the behavior of include_deleted. This is just adding times to it for completeness (so no assumptions are made regarding the lack of example). I can make a more explicit note if that'd be preferred.

Kennric commented 8 years ago

We should point out that one of the returned records, when using include_deleted, is in fact a deleted record, and that we know this from the deleted_at field. We don't have to do that on every example, but at least one.

pop commented 8 years ago

+1

Kennric commented 8 years ago

Ok, that works. +1