-
Notifications
You must be signed in to change notification settings - Fork 2.1k
feat: Notion Integration #1197
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
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 :) |
|
Hey! Now the UI is here :) Does the Notion API allow to request an updated version of a database? I think we need some kind of 'Refresh' button. I just created new fields in the Notion database but they dont show up in the Formbricks integration:
Secondly: Great job on the mapping UI, it looks really slick!! 🔥 In-between testing, the mapping UI got lost: https://tella.video/johanness-video-8419 Questions do not get reduced from the list when they are selected once:
I think it makes sense, maybe people want to add the same info twice. Just double checking with you if this was the intention :) Just double checking if "URI" is correct or if it should be "URL":
Great to validate that early! What would I map CTA on? I thought its either boolean ( clicked / dismissed ) Do you have a list of what question types you can map on which fields in Notion? Would love to include it to the Docs! Please disable the button Submit button in the modal if an error message is visible:
The new map input is not added below the entry where we hit + https://tella.video/johanness-video-dz5k Lastly: The integration doesnt pipe the answers to Notion :( Not sure what causes it, didnt get an error in the console either. https://tella.video/johanness-video-6yso Thanks!!It's really coming together, excited to have this in the upcoming weekly review 🤓 |
mattinannt
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please put all the images in a subfolder images
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please put this component in a subfolder components
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please put this component in a subfolder components
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please put this component in a subfolder components
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@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) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Dhruwang please check
packages/lib/notion/service.ts
Outdated
| } | ||
| return results; | ||
| } catch (error) { | ||
| if (error instanceof Prisma.PrismaClientKnownRequestError) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Dhruwang please check
|
As discussed in Discord, for the record:
Thanks!! 🙏 |
…feat/notion-integration
…aik/formbricks into feat/notion-integration
|
@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 :) |
…aik/formbricks into feat/notion-integration
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@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