feat: Notion Integration#1197
Conversation
|
@PratikAwaik is attempting to deploy a commit to the formbricks Team on Vercel. A member of the Team first needs to authorize it. |
|
Thank you for following the naming conventions for pull request titles! 🙏 |
|
@PratikAwaik thanks a ton, really excited to try this out :) Can you add a barebones docs page so I know what to do to test this locally? :) I can then just rewrite it into a fully fledged docs page and it can go live together 😍 |
|
@jobenjada Sure thing. Will let you know once done |
|
Hey @PratikAwaik - thanks a lot for shipping this :))
When I set it up, I couldn't select any of the databases for a good minute. Suddenly, it worked. Any idea why this could be? And how we could prevent the impression that it doesnt work? I think we should have some kind of loading indicator, because when I checked back in later on, I found 20 in the list. We should convey to the user that we're still loading all databases from Notion.
Im not really sure what this tells me :)
Oh I think there is the mapping UI missing, no?
When I click on an integration to make changes to it, I'm getting the above error. In order to keep the solution simple, let's just change the error message to this: "A connection with this database is live. Please make changes with caution." I wasn't able to test the integration end-to-end. If you can please also resolve merge conflicts and merge main :) |
mattinannt
left a comment
There was a problem hiding this comment.
@PratikAwaik thank you for the PR :-) We also did some changes to the folder structure for the google sheets and airtable integration last week. Please check out the updated structure and make sure your integration fits right in :-)
There was a problem hiding this comment.
please put all the images in a subfolder images
There was a problem hiding this comment.
please put this component in a subfolder components
There was a problem hiding this comment.
please put this component in a subfolder components
There was a problem hiding this comment.
please put this component in a subfolder components
There was a problem hiding this comment.
please put this component in a subfolder components
packages/lib/notion/service.ts
Outdated
| } | ||
| }; | ||
|
|
||
| export const getNotionIntegration = cache( |
There was a problem hiding this comment.
please check the latest changes from main. We now use the integration service to get the integration by type and only have the service for the integration, e.g. here the notion service, to communicate with notion
packages/types/v1/integrations.ts
Outdated
There was a problem hiding this comment.
Please check the latest changes in the main branch and how we reorganised the types for the different integrations
packages/lib/constants.ts
Outdated
| export const NOTION_OAUTH_CLIENT_ID = env.NOTION_OAUTH_CLIENT_ID; | ||
| export const NOTION_OAUTH_CLIENT_SECRET = env.NOTION_OAUTH_CLIENT_SECRET; | ||
| export const NOTION_AUTH_URL = env.NOTION_AUTH_URL; | ||
| export const NOTION_REDIRECT_URI = env.NOTION_REDIRECT_URI; |
There was a problem hiding this comment.
isn't the NOTION_REDIRECT_URI always the same and ${WEBAPP_URL}/api/...callback?
If so, please use this instead of a new environment variable
apps/web/app/api/notion/route.ts
Outdated
There was a problem hiding this comment.
please check the latest changes in main and move the notion route to apps/web/app/api/v1/integrations/notion/callback/route.ts
mattinannt
left a comment
There was a problem hiding this comment.
@Dhruwang thanks you for making the changes 😊💪 Can you please solve the latest merge conflicts and check the few things that I pointed out?
@jobenjada will do a final check on the functionality and then we can merge this :-)
packages/lib/notion/service.ts
Outdated
| }), | ||
| }); | ||
| } catch (error) { | ||
| if (error instanceof Prisma.PrismaClientKnownRequestError) { |
packages/lib/notion/service.ts
Outdated
| } | ||
| return results; | ||
| } catch (error) { | ||
| if (error instanceof Prisma.PrismaClientKnownRequestError) { |
|
As discussed in Discord, for the record:
Thanks!! 🙏 |
|
@PratikAwaik works smoooooothly, great work!! Matti pls have a look if the setup process is intuitive and well documented and last look at the migration and then LGTM por fiiiin :) |
There was a problem hiding this comment.
@PratikAwaik Great job! 👏🚀
Sorry that it took us so long to merge this PR.









What does this PR do?
Fixes #598
Type of change
How should this be tested?
Checklist
Required
pnpm buildconsole.logsgit pull origin mainAppreciated