Right now if Clockodo adds a new property to their response, some of our tests that are expecting properties to match one-to-one would fail. We should consider removing this check altogether, or allow for additional properties to be present.

lumpSum is used for non-unit-lump sums. Currently only unit-lump-sums will be mapped correctly, since they use lumpSums_id and lumpSums_amount.

Discovered by @meaku when attempting to use the API like so:

const entries = await clockodo.getEntryGroups(
        {
            timeSince: startTime,
            timeUntil: endTime,
            grouping: ["projects_id"]
        },
        {
           filterCustomerId: [60928, 514726]
        },
    );

This leads to the filters looking like filter[customers_id][]=60928&filter[customers_id][]=514726, which is the format required for grouping in the getEntryGroups endpoint but not here. So we need to find a way to fix that.

Also fun fact, I am using the node qs library thinking it was https://www.npmjs.com/package/query-string. Surprising we got this far without noticing that...

There are two issues with the current filter interface (that is used by getEntries for instance):

• We need to maintain a separate query param mapping
• The return type of a filtered response is not accurate. If I call getEntries() with a filter on customersId, I'd expect the returned filter property to only have customersId.

I'd like to have an interface like this:

const response = await clockodo.getEntries({
   filter: {
      customersId: 123
   }
});

// response is now a type that looks like this

type Response = {
    paging: Paging
    filter: {
        customersId: number
    },
    entries: Array<Entry>
}

This allows us to type the filter for each endpoint. This should be doable with generics.

A few methods already have types for their parameters, such as getEntries. At the bare minimum the rest should also get this.

Hello,
currently the clockodo rest API supports a small but cool feature in which you can have billability based on your projects settings, by omitting the billable key (in the api request) from the parameters. (https://www.clockodo.com/en/api/clock/#c15189-headline).

If i do the same thing with the javascript clockodo api, i get the following error: Unhandled error: Error: Missing required parameter "billable" at Proxy.checkRequired

The request is currently done like in this example:

    const parameters = {
        customersId: customersId,
        projectsId: projectsId,
        servicesId: servicesId,
        text: text,
        usersId: usersId
    }
    if (unbillable) {
        parameters.billable = 0;
    }
    await clockodoApi.startClock(parameters);

If now the variable unbillable is false [to get the projects settings billability], the field billable will be undefined which causes the above mentioned error. I also don't know whether i am doing a mistake, or whether the feature is not implemented yet.

Thanks! :)

The function parameters are slightly off which makes the function unusable. customerId should be customersId and serviceId should be servicesId.

The Clockodo Api has some very expencive requests e.g. getUserReport.
In Checkodo there are multiple functions using these expensive requests.
This results in multiple simliar requests for loading the dashboard for example. To reduce these redundant requests it would be good to add an optional cache behavior to the Clockodo SDK

Creating a Clockodo instance with cache should look like that:

const clockodo = new Clockodo({
    apiKey: "key",
    user: "usermail",
    cacheTime: 15 * 60 * 1000      //time of the cache in ms, here it would be 15 minutes
});

• The chache entry is valid as long as there wasn't a PUT, POST, PATCH or DELETE request to the same url.
• When one of those requests is made or the time is over there will be made a new request which response will be stored in the cache again.
• If the instance is created without cacheTime it should behave the same as before.

I would like to migrate our Open Source modules to GitHub actions for integration tests.

We should add our integration test account as secrets so that we can also run integration tests

Follow up of #35

Currently we do not support: https://www.clockodo.com/de/api/holidaysquota/
And: https://www.clockodo.com/de/api/holidayscarry/

There's a header for it:

X-ClockodoEnableIsoUtcDateTimes: 1 or 0

As soon as one of our dependencies switches to ESM-only, we will also need to switch to ESM-only. This already happened in the past when updating of one of sindre's packages (who switched to ESM-only).

The Clockodo SDK has been previously designed in a way to allow additional parameters that haven't been typed yet. E.g. imagine there is a new parameter we haven't added to the function type yet:

clockodo.getUser({
  newParam: true
});

In TypeScript, this would show an error because of the "excess property check". We extend all params with Record<string, unknown> to allow new, untyped params.

As a downside, this also makes it possible to missspell properties and TypeScript will allow it:

clockodo.getUser({
  // TS allows this because it thinks it's a new param
  oldButMisspelledParam: true
});

I think, we should remove & Record<string, unknown> thus making the type check more strict. As a downside, the SDK will now show an error if a new, untyped param is used. However, I think this can be easily fixed:

clockodo.getUser({
  // New, untyped param. Remove this once it got added to the SDK
  // @ts-expect-error
  newParam: true
});

This way, it will show a TypeScript error as soon as the type is added and the ts-expect-error is not necessary anymore. With us working more actively on the SDK, I think we should try to get the complete API typed and up-to-date.

What do you think @mrclickbits @anki247 @dsumer ?

Currently with some endpoint methods that have a required ID, we have it as a separate argument from the typical object passed in with all the other parameters(required or not). Example:

However, others with required parameters are lumped together in a single object:

Should we make required parameters separate, or keep them as is?

Needs to be added to api.js. Would also be nice if a test was written for it in api.integration.tests.js.

You will need to wait for #1 where the other endpoint methods are defined.

In order to authenticate via API key, we need to add the email address as HTTP header. If that email address contains a special character, there might be problems authenticating.

We should:

• check if that is handled by axios
• write a test to verify that it will work in the future (e.g. when we remove axios #80)

The code is currently failing the Travis builds because of this.

We should check if our TypeScript types match the actual objects coming from the API. We could use this tool: https://github.com/ForbesLindesay/typescript-json-validator

I documented the time I have spent trying to figure this out elsewhere on Github:
microsoft/TypeScript#24746 (comment)

We made decide in the end it isn't even necessary, but I feel I am one step away from having the Intellisense so useful that there would be no need for the user of this API to leave their editor. But it shouldn't hold up the conversion to Typescript.

Currently we only have one Post endpoint in the module. Now that this package is published, we should make it a priority to map out the remaining endpoints. I will start with POST.

Right now I just copied the response structure found on Clockodo's API. I think it would be a lot more helpful to show the actual key/value pairs that are returned in the response data.

• It should be possible to configure the API endpoint
• We should not rename query parameters, only change the casing and remove special characters
• Remove nmprc

The tests should be executed on the dist directory.

Based on the configuration of https://github.com/peerigon/eslint-config-peerigon (see https://github.com/peerigon/eslint-config-peerigon/blob/master/.github/workflows/release.yml)

I find myself forgetting to build dist before committing. A pre-commit hook would be great here.

As we changed clockodo to typescript, we need to adjust the eslintrc.js.

• Add peerigon/typescript to use the matching eslint rules.
• Edit tsconfig.js and create new one in src folder
• Fix tests and linting errors

Clockodo recently added this to their api:
https://www.clockodo.com/en/api/targethours/

It's good practice that async methods (async function or functions that return a promise) do never throw. They should reject the promise. Otherwise, it leaves the consumers of our lib in a situation where they need to do things like:

try {
clockodo
    .doSomething()
    .then(continueSomething)
    .catch(console.error); // catch the async error
} catch (err) {
    console.error(err); // also catch the sync error
}

You get the gist :)

Since we don't have async functions in our api.js and we're throwing on _checkRequired, our API wrapper will throw an error (instead of rejecting the promise).

Could you rewrite the methods to async functions instead? Async functions reject the promise automatically. The refactoring shouldn't be very hard.

We should use Travis CI to run our tests for every PR.

The editEntryGroup and deleteEntryGroup calls still point to the – non existing – v1 endpoint.
The endpoint needs to be adjust to /v2:

• https://github.com/peerigon/clockodo/blob/main/src/clockodo.ts#L506
• https://github.com/peerigon/clockodo/blob/main/src/clockodo.ts#L614

The camelcase-keys package uses toLocaleUpperCase() internally which uses the browser's locale to transform object keys.

The Turkish language, however, has surprising capitalization rules:

console.log(city.toLocaleUpperCase('en-US'));
// expected output: "ISTANBUL"

console.log(city.toLocaleUpperCase('TR'));
// expected output: "İSTANBUL"

This leads to the problem that for Turkish users, entries are transformed into this:

            "customersİd": 619336,
            "projectsİd": 631365,
            "usersİd": 62488,

semantic-release's Github plugin triggers a "Secondary rate limit": https://github.com/peerigon/clockodo/actions/runs/4665573239/jobs/8259117117 since a couple of weeks. This will hopefully be fixed with semantic-release/github#487

References
semantic-release/semantic-release#2204

The current public API has been designed without TypeScript in mind. Back in the days we wanted to separate required from optional parameters. However, with TypeScript, this is not necessary anymore.

While this decision was ok for GET methods, I think it also leads to an awkward API when using POST and PUT because it requires you to split the entity like this:

// Instead of this...
clockodo.addEntry(entry);
// ... I have to do this
clockodo.addEntry({customerId: entry.customersId, serviceId: entry.servicesId, billable: entry.billable}, entry);

We should refactor the public API like this:

type ApiModelParam<Model, RequiredProps extends keyof Model> = Required<Pick<Model, RequiredProps>> &
    Partial<Omit<Model, RequiredProps>>;

export default class Clockodo {
    addEntry = async (entry: ApiModelParam<ApiEntry, 'customersId' | 'servicesId' | 'billable'>) => {
        /* ... */
    };

    getEntry = async (id: ApiEntry['id']) => {
        /* ... */
    };

    getEntries = async ({
        timeSince,
        timeUntil,
    }: {
        timeSince: ApiDateTime;
        timeUntil: ApiDateTime;
        /* plus optional properties... */
    }) => {
        /* ... */
    };

    editEntry = async (entry: ApiModelParam<ApiEntry, 'id'>) => {
        /* ... */
    };

    deleteEntry = async (id: ApiEntry['id']) => {
        /* ... */
    };
}

• addEntry should be called with the entry model with the mandatory properties and everything else optional
• getEntry just with an id
• getEntries with a query object with some properties mandatory
• editEntry with the entry model where the id is mandatory and everything else is optional
• deleteEntry with just the id

I also think that we should return the whole response, not just the response body, because sometimes it's necessary to also get meta information about the response. The response should look similar to the Response object (although we can't use it exactly since the SDK should also work in Node.js where we don't have a full Response object.

The returned object should have the following properties:

• headers
• ok
• status
• body

• It should be possible to use cookie auth
• It should be possible to configure the API endpoint -> #53
• Refactor method params: Since we're using TypeScript, we don't need to split between required and optional parameters anymore. In a lot of instances, it's easier for the consumer to just use one method parameter (e.g. addEntry or editEntry).
• Add API errors
• We should get rid of the lodash dependency, the bundle is currently too big see https://bundlephobia.com/[email protected] -> partially solved in #55
• We should not rename query parameters, only change the casing and remove special characters -> #53
• There should be a modern build without polyfills
• Make axios-cache-adapter optional -> #55
• Harmonize entry type with Clockodo's internal entry type
• Expose low-level get, post, put, delete with common error handling
• Return response object (with statusCode, body and headers) instead just the body

We should get rid of the lodash dependency, the bundle is currently too big see https://bundlephobia.com/[email protected]

• Replace deep-map-key
• Make axios-cache-adapter an optional import, so only users who want to use cache have the loadsh dependecy

Currently the generated typings specify Promise<any as return type for all functions as those are not specified. I think it would be beneficial to supply interfaces for the actual return values.

Doesn't really make sense to have typings if we don't supply these types in my opinion. This is of course a bit tedious as we'd have to create the interfaces by hand from the api docs and keep them up to date. Does that effort outweigh the benefit of having typed return types?

Edit: came upon this during checked typescript rewrite.

Clockodo will release in q3 the reading access to the planned hours. I will integrate it to the node api.