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: Add Server-side Filtering to the Surveys Page #2277

Conversation

gupta-piyush19
Copy link
Contributor

@gupta-piyush19 gupta-piyush19 commented Mar 18, 2024

What does this PR do?

Add Server-side Filtering to the Surveys Page

Fixes
https://linear.app/formbricks/issue/FOR-2026/add-server-side-filtering-to-the-surveys-page
#2377

How should this be tested?

  • filter the surveys on the dashboard

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 Mar 18, 2024

@gupta-piyush19 is attempting to deploy a commit to the formbricks Team on Vercel.

A member of the Team first needs to authorize it.

Copy link
Contributor

github-actions bot commented Mar 18, 2024

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

@mattinannt mattinannt self-assigned this Mar 21, 2024
…piyush/for-2026-add-server-side-filtering-to-the-surveys-page
Copy link
Contributor

@Dhruwang Dhruwang left a comment

Choose a reason for hiding this comment

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

Hey @gupta-piyush19 , looks pretty good and works super well 🚀😊

Noticed some minor issues

  1. Load more button disappears after clearing filters
Screen-Recording-2024-04-03-at-1.37.48.PM.mp4
  1. When deleting a survey, survey list is not updated with a newer survey (still having more surveys on next page)
Screen-Recording-2024-04-03-at-1.40.07.PM.mp4
  1. As you can see in the video below, on initial load grid view is selected but I still see the list view and when i click on list view button, it does not respond 🤔
Screen-Recording-2024-04-03-at-1.55.22.PM.mp4

packages/ui/SurveysList/index.tsx Outdated Show resolved Hide resolved
packages/types/surveys.ts Outdated Show resolved Hide resolved
@@ -547,4 +547,27 @@ export interface TSurveyQuestionSummary<T> {
}[];
}

export const ZSurveyFilterCriteria = z.object({
Copy link
Contributor

Choose a reason for hiding this comment

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

Is it possible to eliminate one from TSurveyFilter and TSurveyFilterCriteria, looks quite similar 🤔

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, these two are very similar, but one is being used for client-side filtering types and the other is for server-side filter types. It would be a bit hacky to use one at both places. We can discuss if it needs to be fixed or if we can rename the types to make them more meaningful.

packages/ui/SurveysList/actions.ts Outdated Show resolved Hide resolved
packages/ui/SurveysList/components/SurveyFilters.tsx Outdated Show resolved Hide resolved
…piyush/for-2026-add-server-side-filtering-to-the-surveys-page
…piyush/for-2026-add-server-side-filtering-to-the-surveys-page
…piyush/for-2026-add-server-side-filtering-to-the-surveys-page
…piyush/for-2026-add-server-side-filtering-to-the-surveys-page
@gupta-piyush19
Copy link
Contributor Author

@Dhruwang Thanks for the review 🚀 
I've updated the PR with fixes and a few comments.
Also, for 

When deleting a survey, survey list is not updated with a newer survey (still having more surveys on next page)

Should we fetch one more response? The user can still click on load more; fetching one response and handling the state would just complicate the logic. What do you think? @mattinannt 

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 Looks and works great! :-)
I just made smaller bugfixes and increased the debounce limit as well as well as the surveys per page :-) Great job! 👏

You can merge as soon as @Dhruwang gives his approval :-)

Copy link
Contributor

@Dhruwang Dhruwang left a comment

Choose a reason for hiding this comment

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

Looks good 😊🚀

@Dhruwang Dhruwang enabled auto-merge April 11, 2024 03:45
@Dhruwang Dhruwang added this pull request to the merge queue Apr 11, 2024
Merged via the queue into formbricks:main with commit 4100949 Apr 11, 2024
9 of 11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants