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.

Sign up for GitHub

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

Jump to bottom

feat: vercel integation #1304

Merged

BiswaViraj merged 12 commits into main from vercel-integration

Oct 11, 2022

Member

BiswaViraj commented Sep 27, 2022

What kind of change does this PR introduce? (Bug fix, feature, docs update, ...)
- feature
Why this change was needed? (You can also link to an open issue here)
- This enables adding Novu as an integration from the vercel integration marketplace
- Upon selecting Novu from the marketplace, we're injecting the app identifier and the secret key to the vercel projects as environment variables.
- Demo:

Demo-Novu-Integration.webm

Other information:
Concerns/Future scope:
- Would be better to have totally separate UI elements and pages for vercel integration flow.
- During the vercel onboarding, add an organization selection screen, also add an option to create a new organization

gitpod-io bot commented Sep 27, 2022

BiswaViraj commented

View reviewed changes

apps/api/src/app/vercel-integration/usecases/setup-integration/setup-integration.usecase.ts Outdated Show resolved Hide resolved

BiswaViraj commented

View reviewed changes

apps/web/src/api/hooks/use-vercel-integration.ts Outdated Show resolved Hide resolved

BiswaViraj commented

View reviewed changes

apps/web/src/components/auth/CreateOrganizationForm.tsx Show resolved Hide resolved

BiswaViraj commented

View reviewed changes

apps/web/src/api/hooks/use-vercel-integration.ts Show resolved Hide resolved

BiswaViraj commented

View reviewed changes

apps/web/src/components/auth/LoginForm.tsx Show resolved Hide resolved

BiswaViraj commented

View reviewed changes

apps/web/src/components/auth/SignUpForm.tsx Show resolved Hide resolved

BiswaViraj requested review from ainouzgali, scopsy, davidsoderberg, djabarovgeorge and p-fernandez

September 27, 2022 15:12

BiswaViraj self-assigned this

venarius reviewed

View reviewed changes

Contributor

venarius left a comment

Great stuff! 🔥 I have just added some feedback I have from having a quick look at the code 🙂

apps/api/src/.env.development Show resolved Hide resolved

apps/api/src/app/vercel-integration/usecases/index.ts Outdated Show resolved Hide resolved

apps/api/src/app/vercel-integration/usecases/setup-integration/setup-integration.command.ts Outdated Show resolved Hide resolved

apps/api/src/app/vercel-integration/usecases/setup-integration/setup-integration.usecase.ts Outdated Show resolved Hide resolved

apps/api/src/app/vercel-integration/usecases/setup-integration/setup-integration.usecase.ts Outdated Show resolved Hide resolved

apps/web/src/components/auth/LoginForm.tsx Show resolved Hide resolved

apps/web/src/components/auth/CreateOrganizationForm.tsx Show resolved Hide resolved

apps/web/src/components/auth/CreateOrganizationForm.tsx Show resolved Hide resolved

apps/api/src/app/vercel-integration/vercel-integration.module.ts Outdated Show resolved Hide resolved

apps/api/src/app/vercel-integration/usecases/setup-integration/setup-integration.command.ts Outdated Show resolved Hide resolved

BiswaViraj force-pushed the vercel-integration branch from 73f2633 to 4d68fa9 Compare

September 28, 2022 11:21

Member Author

BiswaViraj commented Sep 28, 2022

@venarius , thank you for the review, it was really helpful :)

scopsy reviewed

View reviewed changes

apps/api/src/app/vercel-integration/vercel-integration.controller.ts Outdated Show resolved Hide resolved

Contributor

raikasdev commented Sep 28, 2022

Also writing documentation about this (letting know this exists mostly and how to setup) would be great

Contributor

scopsy commented Sep 28, 2022

@raikasdev this one is actually an internal integration with vercel, so I don't see any specific reason to document this on docs. As it's not really relevant for anyone except us :P Might create a public notion for it tho

scopsy reviewed

View reviewed changes

apps/api/src/app/vercel-integration/vercel-integration.module.ts Outdated Show resolved Hide resolved

LetItRock reviewed

View reviewed changes

Contributor

LetItRock left a comment

few suggestions ;)

apps/web/src/api/hooks/use-vercel-integration.ts Outdated Show resolved Hide resolved

apps/api/src/app/vercel-integration/usecases/setup-integration/setup-integration.usecase.ts Outdated Show resolved Hide resolved

apps/api/src/app/vercel-integration/usecases/setup-integration/setup-integration.usecase.ts Outdated Show resolved Hide resolved

apps/api/src/app/vercel-integration/usecases/setup-integration/setup-integration.usecase.ts Outdated Show resolved Hide resolved

apps/api/src/app/vercel-integration/usecases/setup-integration/setup-integration.usecase.ts Outdated Show resolved Hide resolved

apps/api/src/app/vercel-integration/usecases/setup-integration/setup-integration.usecase.ts Outdated Show resolved Hide resolved

apps/web/src/components/auth/LoginForm.tsx Outdated Show resolved Hide resolved

BiswaViraj force-pushed the vercel-integration branch from 9e54e8a to c6f0432 Compare

September 30, 2022 14:26

BiswaViraj requested a review from scopsy

September 30, 2022 14:27

p-fernandez approved these changes

View reviewed changes

Contributor

p-fernandez left a comment •

edited

Loading

Great work. 👏🏻
Overall it looks great. Just left few comments.
I totally agree with the part of if having separate Vercel components would clean the implementation.

...p/partner-integrations/usecases/setup-vercel-integration/setup-vercel-integration.usecase.ts Show resolved Hide resolved

...p/partner-integrations/usecases/setup-vercel-integration/setup-vercel-integration.usecase.ts Show resolved Hide resolved

apps/web/src/components/auth/LoginForm.tsx Outdated Show resolved Hide resolved

LetItRock reviewed

View reviewed changes

apps/web/src/api/hooks/use-vercel-integration.ts Show resolved Hide resolved

BiswaViraj force-pushed the vercel-integration branch from 3f0121e to e7edf94 Compare

October 9, 2022 11:55

Contributor

scopsy commented Oct 11, 2022

@BiswaViraj could you merge the conflicts so we can merge this ? 🙏

scopsy added the accepted label

BiswaViraj added 2 commits

October 11, 2022 18:26


          feat: vercel integation

05cf3e9


          fix: loader position

5da7e38

BiswaViraj added 9 commits

October 11, 2022 18:28


          refactor: implementated changes from the review

0fd19e9


          refactor: updated it upon the review, added types and more error hand…

cb74590

…ling


          refactor: updated the imports order

78d0971


          refactor: removed vercel-integration from the swagger

d34e6c6


          refactor: changed to partner-integrations module

e964e3e


          feat: enabled vercel integration for github auth flow

72909fd


          fix: github signup flow for vercel

43bb6b0


          refactor: auth token setup

207a963


          refactor: Split useVercel hooks into two hooks

ba670ee

BiswaViraj force-pushed the vercel-integration branch from 5bc75e1 to ba670ee Compare

October 11, 2022 12:59


          fix: merge conflicts

f18b556

BiswaViraj merged commit 6961ef5 into main

BiswaViraj deleted the vercel-integration branch

October 11, 2022 13:20

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

LetItRock LetItRock left review comments

venarius venarius left review comments

p-fernandez p-fernandez approved these changes

ainouzgali Awaiting requested review from ainouzgali

davidsoderberg Awaiting requested review from davidsoderberg

djabarovgeorge Awaiting requested review from djabarovgeorge

scopsy Awaiting requested review from scopsy

Labels