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: Added order by most popular in Onboarding screen #9891

Merged
merged 12 commits into from
Jul 13, 2023
Merged

feat: Added order by most popular in Onboarding screen #9891

merged 12 commits into from
Jul 13, 2023
+56 −26

Conversation

JaideepGuntupalli
Copy link
Contributor

What does this PR do?

I have added the same sorting for the most popular apps used in App Store to the onboarding screen. First, I have separated the function into its file and then imported it to the integrations handler. Also added sortByMostPopular flag so it doesn't messup existing usages of the endpoint. I have only enabled the flag in Calendar and Conferencing onboarding screens.

Fixes #9858

https://www.loom.com/share/588776ff1fd147a884eaa8307a68645d?sid=9009ab6c-c322-4564-96b6-a60850b853f5

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • Chore (refactoring code, technical debt, workflow improvements)
  • New feature (non-breaking change which adds functionality)

How should this be tested?

  • When testing locally, you need first to install a few apps, as the most popular apps are compared based on no. of installations it has.
  • Then try to signup with a new account and compare with the following screenshot which is default when no apps are installed.
Calendar Conferencing
10cfeaeb637fbcfb2abc16a733db8d97 7a3618dd1ee4b20f2772291b7e0bb8d4

Mandatory Tasks

  • Make sure you have self-reviewed the code. A decent size PR without self-review might be rejected.

@vercel
Copy link

vercel bot commented Jul 3, 2023 •
edited
Loading

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

Name Status Preview Comments Updated (UTC)
cal ❌ Failed (Inspect) Jul 11, 2023 4:41am
ui ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jul 11, 2023 4:41am

@vercel
Copy link

vercel bot commented Jul 3, 2023

@JaideepGuntupalli is attempting to deploy a commit to the cal Team on Vercel.

A member of the Team first needs to authorize it.

@github-actions
Copy link
Contributor

github-actions bot commented Jul 3, 2023 •
edited
Loading

Thank you for following the naming conventions! 🙏

@github-actions github-actions bot added 2 points Created by SyncLinear.com app-store area: app store, apps, calendar integrations, google calendar, outlook, lark, apple calendar Low priority Created by Linear-GitHub Sync ✨ feature New feature or request 🧹 Improvements Improvements to existing features. Mostly UX/UI labels Jul 3, 2023
@JaideepGuntupalli JaideepGuntupalli changed the title Feat/onboarding screen order by most popular feat: Added order by most popular in Onboarding screen Jul 3, 2023
@PeerRich PeerRich requested a review from zomars July 5, 2023 09:48
@PeerRich
Copy link
Member

PeerRich commented Jul 5, 2023

great iniative! this will help a lot. we will review this this week

@PeerRich PeerRich added Medium priority Created by Linear-GitHub Sync and removed Low priority Created by Linear-GitHub Sync labels Jul 5, 2023
@PeerRich
Copy link
Member

PeerRich commented Jul 6, 2023

CleanShot 2023-07-06 at 15 37 54@2x
seems to be some errors

@JaideepGuntupalli
Copy link
Contributor Author

@PeerRich fixed the error, but getting another error from teamId which is also on the main branch too when i run yarn run type-check:ci
Screenshot 2023-07-07 at 10 42 36

@JaideepGuntupalli @CarinaWolli
Update packages/trpc/server/routers/loggedInViewer/integrations.handl…
ec96066 
…er.ts

Co-authored-by: Carina Wollendorfer <30310907+CarinaWolli@users.noreply.github.com>
@github-actions
Copy link
Contributor

github-actions bot commented Jul 7, 2023 •
edited
Loading

📦 Next.js Bundle Analysis for @calcom/web

This analysis was generated by the Next.js Bundle Analysis action. 🤖

This PR introduced no changes to the JavaScript bundle! 🙌

@JaideepGuntupalli
Copy link
Contributor Author

Screenshot 2023-07-09 at 09 08 56 @PeerRich the issue is resolved now

@JaideepGuntupalli
Copy link
Contributor Author

hey @PeerRich, just a reminder, can you pls review the PR

hariombalhara
hariombalhara approved these changes Jul 13, 2023
View reviewed changes
Copy link
Member

@hariombalhara hariombalhara left a comment

Choose a reason for hiding this comment

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

LGTM!! Thanks again @JaideepGuntupalli.

If you find my comment appropriate you can open up a followup PR to do the renaming

@hariombalhara hariombalhara merged commit 213caa7 into calcom:main Jul 13, 2023
26 of 29 checks passed
@JaideepGuntupalli JaideepGuntupalli mentioned this pull request Jul 13, 2023
Merged
1 task
fritterhoff pushed a commit to hm-edu/cal.com that referenced this pull request Jul 25, 2023
@JaideepGuntupalli @CarinaWolli
@PeerRich @fritterhoff
feat: Added order by most popular in Onboarding screen (calcom#9891)
e0fab8d 
Co-authored-by: Carina Wollendorfer <30310907+CarinaWolli@users.noreply.github.com>
Co-authored-by: Peer Richelsen <peeroke@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@hariombalhara hariombalhara hariombalhara approved these changes

@zomars zomars Awaiting requested review from zomars

@CarinaWolli CarinaWolli Awaiting requested review from CarinaWolli

Assignees
No one assigned
Labels
2 points Created by SyncLinear.com app-store area: app store, apps, calendar integrations, google calendar, outlook, lark, apple calendar ✨ feature New feature or request 🧹 Improvements Improvements to existing features. Mostly UX/UI Medium priority Created by Linear-GitHub Sync
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[CAL-2077] Onboarding / Add conferencing - Order by most popular
4 participants
@JaideepGuntupalli @PeerRich @hariombalhara @CarinaWolli