Skip to content
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: Url Shortener #895

Merged
merged 19 commits into from
Oct 5, 2023
Merged

feat: Url Shortener #895

merged 19 commits into from
Oct 5, 2023

Conversation

raysubham
Copy link
Contributor

@raysubham raysubham commented Oct 2, 2023 •
edited

What does this PR do?

Fixes # (issue)
#880

Loom Video: https://www.loom.com/share/0a89c1bbcf2c49d5ac63232be6a06fbd?sid=3a4f29bc-0fce-42ab-9fc6-ee7a52275971

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • Chore (refactoring code, technical debt, workflow improvements)
  • Enhancement (small improvements)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change adds a new database migration
  • This change requires a documentation update

How should this be tested?

  • Test A
  • Test B

Checklist

  • Added a screen recording or screenshots to this PR
  • Filled out the "How to test" section in this PR
  • Read the contributing guide
  • Self-reviewed my own code
  • Commented on my code in hard-to-understand bits
  • Ran pnpm build
  • Checked for warnings, there are none
  • Removed all console.logs
  • Merged the latest changes from main onto my branch with git pull origin main
  • My changes don't cause any responsiveness issues
  • Updated the Formbricks Docs if changes were necessary

@vercel
Copy link

vercel bot commented Oct 2, 2023

@raysubham is attempting to deploy a commit to the formbricks Team on Vercel.

A member of the Team first needs to authorize it.

@github-actions
Copy link
Contributor

github-actions bot commented Oct 2, 2023 •
edited

Thank you for following the naming conventions for pull request titles! 🙏

@Dhruwang Dhruwang self-assigned this Oct 2, 2023
@Dhruwang
Copy link
Contributor

Dhruwang commented Oct 2, 2023

Thank you @raysubham for the PR, looks pretty great 💪😊

I left some comments for improving code quality
Also I noticed one minor issue, where after closing url shortening modal, its fields does not gets reset

Screen-Recording-2023-10-02-at-1.59.14-PM.mp4

Dhruwang
Dhruwang reviewed Oct 2, 2023
View reviewed changes
apps/web/env.mjs Outdated Show resolved Hide resolved
@raysubham
Copy link
Contributor Author

@Dhruwang Thanks so much for catching the issues in the code. I have addressed all the comments & bugs. Feel free to re-test this once when you have time 😊

Dhruwang
Dhruwang requested changes Oct 2, 2023
View reviewed changes
packages/database/schema.prisma Outdated Show resolved Hide resolved
@Dhruwang
Copy link
Contributor

Dhruwang commented Oct 2, 2023

Thank you for addressing all the updates! 🙌🏻
Just one minor adjustment left to make.

@mattinannt mattinannt merged commit ff39086 into formbricks:main Oct 5, 2023
3 of 9 checks passed
@ShubhamPalriwala ShubhamPalriwala mentioned this pull request Oct 9, 2023
Closed
waseemrabani pushed a commit to LuminosoInsight/formbricks that referenced this pull request Feb 2, 2024
@raysubham @Dhruwang @mattinannt
feat: add Url Shortener (formbricks#895)
04569e5 
* WIP

* added prisma actions

* remove console.logs

* some more fixes

* tweaks

* addressed all PR review comments

* remove hits from the prisma schema and all its corresponding service logic

* add nanoid

* corrected placeholders

* change database model, bring shortUrl service up to Formbricks code conventions

* update UI and shortUrl endpoint to work with new service

---------

Co-authored-by: Dhruwang <dhruwangjariwala18@gmail.com>
Co-authored-by: Matthias Nannt <mail@matthiasnannt.com>
waseemrabani pushed a commit to LuminosoInsight/formbricks that referenced this pull request Feb 2, 2024
@raysubham @Dhruwang @mattinannt
feat: add Url Shortener (formbricks#895)
a8bcaa9 
* WIP

* added prisma actions

* remove console.logs

* some more fixes

* tweaks

* addressed all PR review comments

* remove hits from the prisma schema and all its corresponding service logic

* add nanoid

* corrected placeholders

* change database model, bring shortUrl service up to Formbricks code conventions

* update UI and shortUrl endpoint to work with new service

---------

Co-authored-by: Dhruwang <dhruwangjariwala18@gmail.com>
Co-authored-by: Matthias Nannt <mail@matthiasnannt.com>
waseemrabani pushed a commit to LuminosoInsight/formbricks that referenced this pull request Feb 2, 2024
@raysubham @Dhruwang @mattinannt
feat: add Url Shortener (formbricks#895)
b6b0f92 
* WIP

* added prisma actions

* remove console.logs

* some more fixes

* tweaks

* addressed all PR review comments

* remove hits from the prisma schema and all its corresponding service logic

* add nanoid

* corrected placeholders

* change database model, bring shortUrl service up to Formbricks code conventions

* update UI and shortUrl endpoint to work with new service

---------

Co-authored-by: Dhruwang <dhruwangjariwala18@gmail.com>
Co-authored-by: Matthias Nannt <mail@matthiasnannt.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Dhruwang Dhruwang Dhruwang approved these changes

Assignees

@Dhruwang Dhruwang

Labels
❗️ .env changes hacktoberfest-accepted ❗️ migrations
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@raysubham @Dhruwang @mattinannt