integrations/clickup: Add ClickUp integration script. - #921
integrations/clickup: Add ClickUp integration script.#921sathwikshetty33 wants to merge 1 commit into
Conversation
1d63b18 to
daa1323
Compare
|
@0xthedance tagging you for a review. |
|
@zulipbot add "buddy review" |
0xthedance
left a comment
There was a problem hiding this comment.
LGTM. I've run the script for creating and replacing webhooks in my ClickUp app, and it works.
I've included a few slight suggestions, but feel free to skip any that don't feel clarifying
daa1323 to
b869dbc
Compare
|
@0xthedance Thanks for the review! |
|
@zulipbot remove "buddy review" |
|
@zulipbot add "mentor review" |
|
Tagging @sbansal1999 and @prah23 for review. |
sbansal1999
left a comment
There was a problem hiding this comment.
Thanks @sathwikshetty33 for working on this one. I have left some comments on this PR; feel free to create a thread in CZO if you feel any of them needs any discussion.
| if response.status != 200: | ||
| print(f"Error : {response.status}") | ||
| sys.exit(1) |
There was a problem hiding this comment.
- This does an abrupt sys.exit call rather than handling the error properly.
- Can this branch be deleted given
urllibraisesHTTPErrorwhen the response status is not 2XX?
ClickUp payloads contain only entity IDs, so the server needs a stored ClickUp token to resolve them to names. Use a personal API token instead of OAuth: the token must be present in the bot config at bot-creation time, whereas an OAuth flow can only issue one after the bot and webhook URL already exist. Fixes part of zulip/zulip#26529 Co-Authored-By: pieterck <pieterceka123@gmail.com>
b869dbc to
2fa10cf
Compare
|
Thank you @sbansal1999 for the review, I have included all your changes; ready for your review again. |
This is a replacement PR for #824 .
The original PR used an OAuth approach. The bot was created without a ClickUp token, and the script ran an OAuth flow to generate one. That token, along with the team ID, was then appended to the bot's webhook URL as query parameters, so the key essentially lived in the URL.
The server side has since moved on. It now reads the ClickUp token from the bot's config data, where it is a required option at bot creation, and uses it to call ClickUp's API and turn the entity IDs in webhook payloads into readable names.
That is what rules OAuth out. The config option needs the token when the bot is created, but an OAuth flow can only return a token after the bot and its webhook URL already exist, so there is no point at which OAuth could provide what bot creation asks for.
This PR therefore drops OAuth entirely. Instead, the user generates a personal API token from the ClickUp UI (Settings > ClickUp API > API Token), enters it when creating the bot, and passes the same token to this script to register the webhook. The script now registers a plain webhook URL with nothing appended, since the server takes the team ID straight from the payload and the token from config.
Fixes part of zulip/zulip#26529
CZO: thread
How did you test this PR?
Manually invoked the script and added unit tests.
Testing video
Self-review checklist
(variable names, code reuse, readability, etc.).
Communicate decisions, questions, and potential concerns.
Individual commits are ready for review (see commit discipline).
Completed manual review and testing of the following: