Conversation
| this.config({ client, authentication, baseUrl }); | ||
| } | ||
|
|
||
| // The fact that this can be called again was a source of great confusion. I had changed the if below to throw if (!baseUrl || typeof baseUrl !== "string"), but turns out sometimes we don't need it |
There was a problem hiding this comment.
Remove this comment later lol
|
@jhnns I think I might need to see that function you mentioned in the Clockodo project for detecting error message content , but in general feedback would be appreciated. |
jhnns
left a comment
jhnns
left a comment
There was a problem hiding this comment.
In general: Looks good 👍 that's definitely the right direction. I've added some minor comments.
| } | ||
|
|
||
| request.headers["X-Requested-With"] = "XMLHttpRequest"; | ||
| // defaults.withCredentials = true; |
There was a problem hiding this comment.
This is something we need to preserve. The regular SDK authentication is done via API key. However, at Clockodo itself, we sometimes need to do cookie auth instead. That's why we need to send cookies along each request once authentication has been set to undefined.
Using the fetch() API, this is done by setting credentials: "include" (see https://developer.mozilla.org/en-US/docs/Web/API/Request/credentials)
Looking back I think this config could be improved in the way that authentication is set explicitly to "cookie" (makes it more obvious). If you want, you can change that :) (since this is a breaking change, we need to include it in the changelog).
|
@tannerbaum Let me know if you want me to review again :) |
2 participants
No description provided.