Skip to content

feat: booking filters [CAL-1801]#9692

Merged
leog merged 9 commits intomainfrom
feat/booking-filters
Jul 5, 2023
Merged

feat: booking filters [CAL-1801]#9692
leog merged 9 commits intomainfrom
feat/booking-filters

Conversation

@Udit-takkar
Copy link
Copy Markdown
Contributor

@Udit-takkar Udit-takkar commented Jun 21, 2023

What does this PR do?

Fixes https://linear.app/calcom/issue/CAL-1801/org-bookings
Fixes https://linear.app/calcom/issue/CAL-2022/booking-filters-pending-and-possibly-all-members-are-showing-up-as

  • Team Filter ( similar to what we have in /event-types)
  • Add Filter Button to add additional filters such as Event type, People and Location
Screen.Recording.2023-06-21.at.11.47.39.PM.mov

Type of change

  • New feature (non-breaking change which adds functionality)

How should this be tested?

  • Go to /bookings page
  • Try filters on top right.

@linear
Copy link
Copy Markdown

linear Bot commented Jun 21, 2023

CAL-1801 Org bookings

Implement the different filters in the bookings page for an organization based on designs

Org → Bookings - 5. Organizations (Figma)

@vercel
Copy link
Copy Markdown

vercel Bot commented Jun 21, 2023

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

Name Status Preview Comments Updated (UTC)
api ❌ Failed (Inspect) Mar 19, 2024 2:50pm
cal ✅ Ready (Inspect) Visit Preview 💬 Add feedback Mar 19, 2024 2:50pm
ui ❌ Failed (Inspect) Mar 19, 2024 2:50pm
web-staging 🔄 Building (Inspect) Visit Preview Mar 19, 2024 2:50pm
1 Ignored Deployment
Name Status Preview Comments Updated (UTC)
dev ⬜️ Ignored (Inspect) Mar 19, 2024 2:50pm

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Jun 21, 2023

Thank you for following the naming conventions! 🙏

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Jun 21, 2023

📦 Next.js Bundle Analysis for @calcom/web

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

Nine Pages Changed Size

The following pages changed size from the code in this PR compared to its base branch:

Page Size (compressed) First Load % of Budget (350 KB)
/[user]/book 253.15 KB 406.61 KB 116.18% (🟡 +0.14%)
/bookings/[status] 354.48 KB 507.94 KB 145.13% (🟡 +0.60%)
/d/[link]/book 252.8 KB 406.26 KB 116.08% (🟡 +0.14%)
/new-booker/[user]/[type] 396.76 KB 550.23 KB 157.21% (🟡 +0.14%)
/new-booker/[user]/[type]/embed 396.79 KB 550.26 KB 157.22% (🟡 +0.14%)
/new-booker/d/[link]/[slug] 396.72 KB 550.18 KB 157.20% (🟡 +0.14%)
/new-booker/team/[slug]/[type] 396.77 KB 550.24 KB 157.21% (🟡 +0.14%)
/new-booker/team/[slug]/[type]/embed 396.81 KB 550.27 KB 157.22% (🟡 +0.14%)
/team/[slug]/book 252.8 KB 406.26 KB 116.08% (🟡 +0.14%)
Details

Only the gzipped size is provided here based on an expert tip.

First Load is the size of the global bundle plus the bundle for the individual page. If a user were to show up to your website and land on a given page, the first load size represents the amount of javascript that user would need to download. If next/link is used, subsequent page loads would only need to download that page's bundle (the number in the "Size" column), since the global bundle has already been downloaded.

Any third party scripts you have added directly to your app using the <script> tag are not accounted for in this analysis

The "Budget %" column shows what percentage of your performance budget the First Load total takes up. For example, if your budget was 100kb, and a given page's first load size was 10kb, it would be 10% of your budget. You can also see how much this has increased or decreased compared to the base branch of your PR. If this percentage has increased by 20% or more, there will be a red status indicator applied, indicating that special attention should be given to this. If you see "+/- <0.01%" it means that there was a change in bundle size, but it is a trivial enough amount that it can be ignored.

@deploysentinel
Copy link
Copy Markdown

deploysentinel Bot commented Jun 21, 2023

Current Playwright Test Results Summary

✅ 127 Passing - ⚠️ 4 Flaky

Run may still be in progress, this comment will be updated as current testing workflow or job completes...

(Last updated on 07/04/2023 02:43:23pm UTC)

Run Details

Running Workflow PR Update on Github Actions

Commit: 497d14e

Started: 07/04/2023 02:39:25pm UTC

⚠️ Flakes

📄   apps/web/playwright/manage-booking-questions.e2e.ts • 1 Flake

Test Case Results

Test Case Last 7 days Failures Last 7 days Flakes
Manage Booking Questions For Team EventType -- old-booker Do a booking with a user added question and verify a few thing in b/w
Retry 1Initial Attempt
0% (0) 0 / 233 runs
failed over last 7 days
2.15% (5) 5 / 233 runs
flaked over last 7 days

📄   packages/embeds/embed-core/playwright/tests/action-based.e2e.ts • 2 Flakes

Top 1 Common Error Messages

null

2 Test Cases Affected

Test Case Results

Test Case Last 7 days Failures Last 7 days Flakes
Popup Tests -- new-booker should be able to reschedule
Retry 1Initial Attempt
12.86% (18) 18 / 140 runs
failed over last 7 days
59.29% (83) 83 / 140 runs
flaked over last 7 days
Popup Tests -- old-booker should be able to reschedule
Retry 1Initial Attempt
2.88% (4) 4 / 139 runs
failed over last 7 days
44.60% (62) 62 / 139 runs
flaked over last 7 days

📄   apps/web/playwright/booking-seats.e2e.ts • 1 Flake

Test Case Results

Test Case Last 7 days Failures Last 7 days Flakes
Reschedule for booking with seats -- old-booker Should reschedule booking with seats
Retry 1Initial Attempt
0% (0) 0 / 234 runs
failed over last 7 days
0.43% (1) 1 / 234 run
flaked over last 7 days

View Detailed Build Results


@Udit-takkar Udit-takkar marked this pull request as ready for review June 21, 2023 18:19
Comment on lines +28 to +30
// label: "date_range",
// StartIcon: Calendar,
// },
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Filter by date range will be added later

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

You can reuse the component I used on insights 👍

@linear
Copy link
Copy Markdown

linear Bot commented Jun 22, 2023

CAL-2022 Booking filters: Pending (and possibly all) members are showing up as blank people

In bookings tab I can see 3 blank users. The org has 1 confirmed member and 2 pending.

CleanShot 2023-06-22 at 09.49.032x.png

Comment thread apps/web/public/static/locales/en/common.json Outdated
Comment thread packages/features/bookings/components/FiltersContainer.tsx
Comment on lines +415 to +433
export const FilterSearchField = forwardRef<HTMLInputElement, InputFieldProps>(function TextField(
props,
ref
) {
return (
<div
dir="ltr"
className="focus-within:ring-brand-default group relative mx-3 mb-1 mt-2.5 flex items-center rounded-md focus-within:outline-none focus-within:ring-2">
<div className="addon-wrapper border-default [input:hover_+_&]:border-emphasis [input:hover_+_&]:border-l-default [&:has(+_input:hover)]:border-emphasis [&:has(+_input:hover)]:border-r-default flex h-7 items-center justify-center rounded-l-md border border-r-0">
<Search className="ms-3 h-4 w-4" />
</div>
<Input
ref={ref}
className="disabled:bg-subtle disabled:hover:border-subtle !my-0 h-7 rounded-l-none border-l-0 !ring-0 disabled:cursor-not-allowed"
{...props}
/>
</div>
);
});
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Should this be here or in filters? Im not too sure myself so just wanted to know what others think

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This field can be used in other pages such as insights

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This should be a feature component IMHO. It belongs to features/filters/components.

@Udit-takkar
Copy link
Copy Markdown
Contributor Author

@ericrommel I had to remove location filter for now because eventType.locations has Json type(@zod.custom(imports.eventTypeLocations)) in the schema and PostgreSQL does not support filtering on object key value inside array https://www.prisma.io/docs/concepts/components/prisma-client/working-with-fields/working-with-json-fields#filtering-on-object-key-value-inside-array.

so we can't use this query inside booking get handler

    {
          eventType: {
            locations: {
                 path: 'type',
                 array_contains: input.filters?.locationValues,
            },
          },
        },

while i can filter location in the Booking table because it is of type string but we won't be able to filter for location that you mentioned (Conferencing/Organizer, Phone/Attendee and Phone/Organizer)

@CarinaWolli fixed the mobile view

Screen.Recording.2023-07-01.at.2.49.22.PM.mov

ericrommel
ericrommel previously approved these changes Jul 1, 2023
Copy link
Copy Markdown
Contributor

@ericrommel ericrommel left a comment

Choose a reason for hiding this comment

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

LGTM

// },
],
isFilterActive: (label: addFilterOption["label"]) => {
return !get().addFilterOptions.some((option) => option.label === label);
Copy link
Copy Markdown
Member

@hariombalhara hariombalhara Jul 3, 2023

Choose a reason for hiding this comment

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

nit: I am not a fan of using translation key as identifier. We should have a separate id field maybe to identify an item uniquely. That way changing translation strings doesn't impact the logic

state.addFilterOptions,
state.toggleOption,
state.isFilterActive,
shallow,
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I don't think we are passing it correctly. It should have been an argument to useBookingMultiFilterStore

isFilterActive: (label: addFilterOption["label"]) => boolean;
};

export const useBookingMultiFilterStore = create<BookingMultiFilterStore>((set, get) => ({
Copy link
Copy Markdown
Member

@hariombalhara hariombalhara Jul 3, 2023

Choose a reason for hiding this comment

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

nit: I am not sure if we need to use a zustand store for it. It could have been a local state to FiltersContainer. I am just learning zustand @sean-brydon what do you think?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Something simpler will be good I guess as @hariombalhara says.

type FilterTypes = "teams" | "people" | "eventType";
const PeopleFilter = () => {
const { t } = useLocale();
const { data: query, pushItemToKey, removeItemByKeyAndValue, removeByKey } = useFilterQuery();
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
const { data: query, pushItemToKey, removeItemByKeyAndValue, removeByKey } = useFilterQuery();
const { data: query, pushItemToKey, removeItemByKeyAndValue } = useFilterQuery();

Unused.

const members = trpc.viewer.teams.listMembers.useQuery({});

const filteredMembers = members?.data
?.filter((member) => member.accepted)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

nit: It's already fetching accepted team members only. So it might not be necessary

Comment on lines +21 to +22
Trigger?: React.ReactNode;
defaultOpen?: boolean;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I don't see these two props being used by any of the AnimatedPopover instances.

"no_status_bookings_yet_description": "You have no {{status}} bookings. {{description}}",
"event_between_users": "{{eventName}} between {{host}} and {{attendeeName}}",
"bookings": "Bookings",
"booking_not_found": "Booking not found",
Copy link
Copy Markdown
Member

@hariombalhara hariombalhara Jul 3, 2023

Choose a reason for hiding this comment

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

Suggested change
"booking_not_found": "Booking not found",

Looks unused to me.
Screenshot 2023-07-03 at 7 48 01 PM

hariombalhara
hariombalhara previously approved these changes Jul 3, 2023
Copy link
Copy Markdown
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!! Great work @Udit-takkar. No blocker. Just a few nits.

@kodiakhq
Copy link
Copy Markdown
Contributor

kodiakhq Bot commented Jul 3, 2023

This PR currently has a merge conflict. Please resolve this and then re-add the automerge label.

alannnc
alannnc previously approved these changes Jul 3, 2023
Copy link
Copy Markdown
Contributor

@alannnc alannnc left a comment

Choose a reason for hiding this comment

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

Handling nits and merge conflicts should be enough before merge with main.

Copy link
Copy Markdown
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.

Approving again

@leog leog merged commit 8d7b2cf into main Jul 5, 2023
@leog leog deleted the feat/booking-filters branch July 5, 2023 03:24
fritterhoff pushed a commit to hm-edu/cal.com that referenced this pull request Jul 25, 2023
* feat: booking filters

Signed-off-by: Udit Takkar <udit.07814802719@cse.mait.ac.in>

* fix: responsive issues

Signed-off-by: Udit Takkar <udit.07814802719@cse.mait.ac.in>

* fix: only show accepted members in filter

Signed-off-by: Udit Takkar <udit.07814802719@cse.mait.ac.in>

* chore: translations

Signed-off-by: Udit Takkar <udit.07814802719@cse.mait.ac.in>

* fix: remove organizer default

Signed-off-by: Udit Takkar <udit.07814802719@cse.mait.ac.in>

* chore

Signed-off-by: Udit Takkar <udit.07814802719@cse.mait.ac.in>

* fix: remove locationValues

Signed-off-by: Udit Takkar <udit.07814802719@cse.mait.ac.in>

* fix: remove unused and mobile

Signed-off-by: Udit Takkar <udit.07814802719@cse.mait.ac.in>

---------

Signed-off-by: Udit Takkar <udit.07814802719@cse.mait.ac.in>
Co-authored-by: Leo Giovanetti <hello@leog.me>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

High priority Created by Linear-GitHub Sync organizations area: organizations, orgs

Projects

No open projects
Status: High priority

Development

Successfully merging this pull request may close these issues.

7 participants