psolymos / clickrup

Interacting with the ClickUp v2 API from R
https://peter.solymos.org/clickrup/
MIT License
18 stars 3 forks source link

Time tracking API updates #13

Open psolymos opened 2 years ago

psolymos commented 2 years ago

https://clickup.canny.io/feature-requests/p/get-time-entries-by-task-id-from-api

We recently updated our Time Tracking 2.0 API! We now include the ability to request task, List, Folder, and Space ids and names, along with task tags when using the Get time entries within a date range and get singular time entry endpoints.

You can also filter results by task, List, Folder, or Space using the new parameters on the Get time entries within a date range endpoint.

Refer to our API documentation for more details!

psolymos commented 2 years ago

This one tripped me up and made me thing in general, how to pass path vs query elements to requests.

This is the Time Tracking 2.0 / Get time entries within a date range example:

https://api.clickup.com/api/v2/team/team_Id/time_entries?start_date=&end_date=&assignee=&include_task_tags=&include_location_names=&space_id=&folder_id=&list_id=&task_id=&custom_task_ids=&team_id=

We have team_Id as path and team_id as query param.

The R code had this before:

cu_get_time_entries_within_date_range <- function(team_id, start_date, end_date, assignee) {...}

But now making team_Id lower case would lead to 2x the same arg, and mixed/lower case will be confusing to users and can easily be misspecified.

Should we do this instead? Pass all query params as a list?

cu_get_time_entries_within_date_range <- function(team_id, params = list()) {...}

This lends itself nicely to passing the list on to .cu_get(..., query=params). I would call it params or parameters because that is how the API docs refer to these.

This, however, brings up a more general question about following this pattern for the rest of the functions, or at least trial it for the Time Tracking API. As long as these params are optional, this should be fine. The problem could be if some of the params are required -- which we'll have to investigate.

@krlmlr what do you think?

krlmlr commented 2 years ago

Thanks. I don't understand the purpose of the team_id parameter in the path. If the path and query parameters are truly two different things, we can give them clear names. I prefer arguments over a params:

psolymos commented 2 years ago

As I understand, the .../api/v2/team/team_Id/... part identifies the workspace the users PAT applies to.

The description is bit vague, but it says that team_id is _Only used when the parameter is set to custom_task_ids=true_, which to me means that custom task IDs are only unique within a workspace (that team_id refers to).

I will stick with arguments for now and find a reasonable lower case alternative (like custom_team_id) and document it.