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.

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: Refactor Triggers and combine Action Classes and Inline Triggers #2562

Conversation

gupta-piyush19
Copy link
Contributor

@gupta-piyush19 gupta-piyush19 commented Apr 30, 2024

What does this PR do?

  • Refactor Triggers and combine Action Classes and Inline Triggers
  • removes inline triggers completely

Fixes
https://github.com/formbricks/internal/issues/101

How should this be tested?

  • run the npx prisma migrate dev to migrate the DB schema
  • then run the data migration script pnpm run data-migration:refactor-actions after navigating into the packages/database directory.
  • Test UI changes for any website or app survey.

Checklist

Required

  • Filled out the "How to test" section in this PR
  • Read How we Code at Formbricks
  • 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

Appreciated

  • If a UI change was made: Added a screen recording or screenshots to this PR
  • Updated the Formbricks Docs if changes were necessary

Copy link

vercel bot commented Apr 30, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
formbricks-cloud 🛑 Canceled (Inspect) May 7, 2024 2:43pm
2 Ignored Deployments
Name Status Preview Comments Updated (UTC)
formbricks-com ⬜️ Ignored (Inspect) Visit Preview May 7, 2024 2:43pm
formbricks-docs ⬜️ Ignored (Inspect) Visit Preview May 7, 2024 2:43pm

Copy link
Contributor

github-actions bot commented Apr 30, 2024

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

Copy link
Member

@mattinannt mattinannt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@gupta-piyush19 thanks a lot for the feature! :-) 💪
I already had a very brief look at the code and added some comments :-)

apps/demo/pages/app/index.tsx Outdated Show resolved Hide resolved
apps/demo/pages/website/index.tsx Outdated Show resolved Hide resolved
packages/js-core/src/website/lib/sync.ts Outdated Show resolved Hide resolved
packages/lib/action/service.ts Outdated Show resolved Hide resolved
packages/lib/survey/service.ts Outdated Show resolved Hide resolved
packages/types/js.ts Outdated Show resolved Hide resolved
…line-triggers' of https://github.com/formbricks/formbricks into 101-refactor-triggers-and-combine-action-classes-and-inline-triggers
…101-refactor-triggers-and-combine-action-classes-and-inline-triggers
…101-refactor-triggers-and-combine-action-classes-and-inline-triggers
Copy link
Member

@jobenjada jobenjada left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey Piyush!

I came across a few more issues:

  1. There is an issue with the selection of the actions:
actions-search.mp4

After seraching, it shows a wider range of options to choose from.


  1. Saving Error + it does not prevent Survey Editor closing:
actions-saving-bug.mp4

I'm getting the Save Error with preexisting surveys, not with new ones. Maybe the migration did not work as planned?

image image

Looks like it is because I created the surveys with the Slick Card PR.

But pls still look into why the Save & Close closes the editor even though we throw an error.


  1. Remove duplicate forms: Use the Action Create form from the Survey Editor on the Actions page:

replace:
image

with:
image

Thanks a lot! :)

@gupta-piyush19
Copy link
Contributor Author

Hey Piyush!

I came across a few more issues:

  1. There is an issue with the selection of the actions:

actions-search.mp4
After seraching, it shows a wider range of options to choose from.

  1. Saving Error + it does not prevent Survey Editor closing:

actions-saving-bug.mp4
I'm getting the Save Error with preexisting surveys, not with new ones. Maybe the migration did not work as planned?

image image
Looks like it is because I created the surveys with the Slick Card PR.

But pls still look into why the Save & Close closes the editor even though we throw an error.

  1. Remove duplicate forms: Use the Action Create form from the Survey Editor on the Actions page:

replace: image

with: image

Thanks a lot! :)

Thanks, @jobenjada, for the review. 🙌

  1. fixed the search actions bug. 🔍
  2. It seems like your DB schema is not migrated correctly. Both slick card changes and the actions PR migrations are implemented on your DB. Can you please try calibrating the DB schema from the main branch and then check out the actions PR?
  3. Done ✅

@mattinannt mattinannt enabled auto-merge May 7, 2024 13:46
@mattinannt mattinannt added this pull request to the merge queue May 7, 2024
Merged via the queue into main with commit 6bfd027 May 7, 2024
10 of 12 checks passed
@mattinannt mattinannt deleted the 101-refactor-triggers-and-combine-action-classes-and-inline-triggers branch May 7, 2024 13:59
@vercel vercel bot temporarily deployed to Preview – formbricks-cloud May 7, 2024 14:31 Inactive
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants