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

fix: remove redundant AND #3833

Merged
merged 1 commit into from
Aug 15, 2022
Merged

fix: remove redundant AND #3833

merged 1 commit into from
Aug 15, 2022

Conversation

hussamkhatib
Copy link
Contributor

@hussamkhatib hussamkhatib commented Aug 13, 2022

What does this PR do?

AND is unesscessary here as only I condition is provided inside it.

However the motivation before creating the PR was to fix

// Order is important to ensure that given a slug if there are duplicates, we choose the same event type consistently when showing in event-types list UI(in terms of ordering and disabled event types)
// TODO: If we can ensure that there are no duplicates for a [slug, userId] combination in existing data, this requirement might be avoided.

I find the comment bit misleading as there can be no duplicate combination for [slug,userId]. (schema.prisma)

model EventType{
...
  id                      Int                     @id @default(autoincrement())
  users                   User[]                  @relation("user_eventtype")
  @@unique([userId, slug])
}

Due to which I tried to convert
const eventTypes = await prisma.eventType.findMany({
to
const eventTypes = await prisma.eventType.findUnique({
as there exist only 1 such pair.
After some time I found out that it also returns true if user exists in the username is exist in users( see model EventType above)
Eg Alice and Bob both have a event-type with 15min as slug.
the row where Alice is the owner (the event-type created by her) has users: ["alice","Bob"] in column users( see model EventType above)
Current Behaviour: Navigating to /bob/15min is resulted in the event-type where bob is the owner.
Suppise If Bob does not have this event created , It will result to the one Alice created (as Bob is included in users column of Alice craeted event)

The problem:

  • same link can be resolved in two different cases mentioned above.
  • There can be better solution than findMany , as we just need 1 row.

The first problem is probably the expected behaviour.
For the second I though of doing one of the following

  • split into two queries, if record found where userId in model EventType matches the id of the username provided in slug.Then return the result of this and skip the next one. The first condition fails then only perform next one.
  • doing a prisma raw query that will do it in one sql statement.

If any of the above two sounds good, I will go ahead and include them or you can go ahead with merging the existing change if your happy with it.

Type of change

  • Bug fix (non-breaking change which fixes an issue)

@vercel
Copy link

vercel bot commented Aug 13, 2022

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

A member of the Team first needs to authorize it.

Copy link
Member

@zomars zomars left a comment

Choose a reason for hiding this comment

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

Thank you for your contribution 🙏

@zomars zomars merged commit 732acb5 into calcom:main Aug 15, 2022
zomars added a commit that referenced this pull request Aug 23, 2022
* Close.com App (#3709)

* WIP close.com app

* Removing leaked dev key (now invalid)

* Misspelled env variable

* Making progress still WIP

* Progress + tests

* Final touches

* More unit tests

* Finished up tests

* Merge main

* Removing unneeded stuff + submodules

* Removing static props, fields fix

* Removing unneeded stuff p2

* Commenting

* Standarizing APIs

* Zodifying

Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com>
Co-authored-by: zomars <zomars@me.com>

* fix: padding of kBarTrigger (#3846)

* fixes issue with variables not working (#3859)

Co-authored-by: CarinaWolli <wollencarina@gmail.com>

* fix: remove redundant AND (#3833)

Co-authored-by: hussamkhatib <hussamkhatib@gmail.com>

* Hotfix: Embed - Fix issue in accessing sessionStorage in certain scenarios (#3851)

* Fixes issue when sessionStorage is not accessible in privacy focussed modes in various browsers

* Fix eslint errors

Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com>

* fixed iframe in lark suite, added lark contact info (#3866)

* fixed iframe in lark suite, added lark contact info

* Update _metadata.ts

* Update Railway Template (#3862)

* Update Railway Template

* Update URL

Co-authored-by: Peer Richelsen <peeroke@gmail.com>

* New Crowdin translations by Github Action (#3857)

Co-authored-by: Crowdin Bot <support+bot@crowdin.com>

* Attempt at redirect to 404 (#3877)

* Attempt at redirect to 404

404 should be able to handle the traffic no problem - better ideas welcome. Just a temp fix I imagine; we're not going to want to keep this nor can we expect the User-Agent to continue identifying the traffic source.

* Update next.config.js

Co-authored-by: zomars <zomars@me.com>

* hotfix dynamic issue (#3864)

* Hotfix/dos mitigation attempt error configuration (#3879)

* Fixes 'Task timed out after 60.03 seconds'

* DDoS rewrite fixes

Co-authored-by: zomars <zomars@me.com>

* Fix desktop app link (#3883)

* fixed 404 logo on cal video (#3885)

* Update all Yarn dependencies (2022-07-29) (#3599)

* Update all Yarn dependencies (2022-07-29)

* Adds syncpack config

* Fixes mismatches

* Update yarn.lock

* RRule fixes

* Locking dayjs to fix build

* Type fixes

* Fixes mismatches

* Submodule sync

* Update yarn.lock

* Update event.ts

* Conflict fixes

* Fixes prisma warnings

* Liting

* Upgrade next, zod

* Prevents articfact overwriting

* Yarn fixes

* Jest fixes

* Submodule sync

* Formatting

* Submodule sync

* Adds provider for react-tooltip

* Removed dotenv-cli

* Readds dotenv-cli

* Skips getSchedule tests

Until prisma is mocked properly

* Fixes

* Revert prisma seed script

* E2E fixes

* test

* Removed deprecated req.page in middleware

* Make tests stable

* Unskip getSchedule tests

* fixed 404 logo on cal video (#3885)

* Removed PW aliases as aren't needed anymore

Co-authored-by: depfu[bot] <23717796+depfu[bot]@users.noreply.github.com>
Co-authored-by: zomars <zomars@me.com>
Co-authored-by: Hariom Balhara <hariombalhara@gmail.com>
Co-authored-by: Alex van Andel <me@alexvanandel.com>
Co-authored-by: Peer Richelsen <peeroke@gmail.com>

* Submodule sync

* temporally removed missing calendar badge on event-types (#3889)

* New Crowdin translations by Github Action (#3878)

Co-authored-by: Crowdin Bot <support+bot@crowdin.com>

* use default cursor on desktop (#3876)

* New Crowdin translations by Github Action (#3890)

Co-authored-by: Crowdin Bot <support+bot@crowdin.com>
Co-authored-by: Peer Richelsen <peeroke@gmail.com>

* NEXTAUTH_DOMAIN isn't used

* Sync submodules

* New Crowdin translations by Github Action (#3897)

Co-authored-by: Crowdin Bot <support+bot@crowdin.com>

* adjust react version

* Revert "adjust react version"

This reverts commit 6e161da.

* cancel booking new design added (#3660)

* feat:  cancel booking new design added

* style: add space-y in button

* style: ui fixed for recurring booking

* fix: scroll in recurring event and use translation

* style: text 2xl instead of text-[24px]

* fix: typo, spacing

* fix: replaced multiple h2 tags with div

* fix: padding and Icon name

* Revert " cancel booking new design added" (#3922)

This reverts commit a06c564.

* New Crowdin translations by Github Action (#3909)

Co-authored-by: Crowdin Bot <support+bot@crowdin.com>

* New Crowdin translations by Github Action (#3924)

Co-authored-by: Crowdin Bot <support+bot@crowdin.com>

* New Crowdin translations by Github Action (#3926)

Co-authored-by: Crowdin Bot <support+bot@crowdin.com>

* fix/auto-connect-calendar-3582 (#3891)

* Added alert when there isn't default calendar connected

* Add default calendar externalId to select placeholder

* Fix typos

* Fixes prisma import

Co-authored-by: Peer Richelsen <peeroke@gmail.com>
Co-authored-by: zomars <zomars@me.com>

* redesigned cancel page to new design (#3923)

Co-authored-by: Alex van Andel <me@alexvanandel.com>

* New Crowdin translations by Github Action (#3927)

Co-authored-by: Crowdin Bot <support+bot@crowdin.com>

* New Crowdin translations by Github Action (#3929)

Co-authored-by: Crowdin Bot <support+bot@crowdin.com>

* Design issues on success page in some languages (#3900)

Co-authored-by: gitstart <gitstart@users.noreply.github.com>
Co-authored-by: Nitesh Singh <nitesh.singh@gitstart.dev>
Co-authored-by: Júlio Piubello da Silva Cabral <julio.piubello@gitstart.dev>
Co-authored-by: Matheus Muniz <87545749+matheusmuniz03@users.noreply.github.com>
Co-authored-by: Matheus Benini <matheus_benini@hotmail.com>
Co-authored-by: Grace Nshokano <grace.devolop@gmail.com>
Co-authored-by: Matheus Muniz <matheusmuniz100@hotmail.com>
Co-authored-by: gitstart <gitstart@gitstart.com>
Co-authored-by: Murilo Amaral <87545137+MuriloAmarals@users.noreply.github.com>
Co-authored-by: Peer Richelsen <peeroke@gmail.com>

* fix: developer docs url (#3914)

* fix: developer docs url added

* chore : remove /

* chore : import url

Co-authored-by: Zach Waterfield <zlwaterfield@gmail.com>
Co-authored-by: Peer Richelsen <peeroke@gmail.com>

* Fixes collective availability for teams with overlapping day timezones (#3898)

* WIP

* Fix for team availability with time offsets

* Prevent empty schedule from opening up everything

* When no utcOffset or timeZone's are given, default to 0 utcOffset (UTC)

* timeZone should not be part of getUserAvailability

* Prevents {days:[X],startTime:0,endTime:0} error entry

* Added getAggregateWorkingHours() (#3913)

* Added test for getAggregateWorkingHours

* Timezone isn't used here anymore

* fix: developer docs url (#3914)

* fix: developer docs url added

* chore : remove /

* chore : import url

Co-authored-by: Zach Waterfield <zlwaterfield@gmail.com>
Co-authored-by: Peer Richelsen <peeroke@gmail.com>

* Test fixes

* Reinstate prisma (generate only) and few comments

* Test fixes

* Skipping getSchedule again

* Added await to expect() as it involves async logic causing the promise to timeout

* Test cleanup

* Update jest.config.ts

Co-authored-by: Alan <alannnc@gmail.com>
Co-authored-by: Alex van Andel <me@alexvanandel.com>
Co-authored-by: Udit Takkar <53316345+Udit-takkar@users.noreply.github.com>
Co-authored-by: Zach Waterfield <zlwaterfield@gmail.com>
Co-authored-by: Peer Richelsen <peeroke@gmail.com>

* v1.9.1

Co-authored-by: Leo Giovanetti <hello@leog.me>
Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com>
Co-authored-by: zomars <zomars@me.com>
Co-authored-by: Udit Takkar <53316345+Udit-takkar@users.noreply.github.com>
Co-authored-by: Carina Wollendorfer <30310907+CarinaWolli@users.noreply.github.com>
Co-authored-by: CarinaWolli <wollencarina@gmail.com>
Co-authored-by: mohammed hussam <52914487+hussamkhatib@users.noreply.github.com>
Co-authored-by: hussamkhatib <hussamkhatib@gmail.com>
Co-authored-by: Hariom Balhara <hariombalhara@gmail.com>
Co-authored-by: Jake Cooper <jake.elijah.cooper@gmail.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: Crowdin Bot <support+bot@crowdin.com>
Co-authored-by: Alex van Andel <me@alexvanandel.com>
Co-authored-by: Syed Ali Shahbaz <52925846+alishaz-polymath@users.noreply.github.com>
Co-authored-by: depfu[bot] <23717796+depfu[bot]@users.noreply.github.com>
Co-authored-by: Julian Benegas <julianbenegas99@gmail.com>
Co-authored-by: alannnc <alannnc@gmail.com>
Co-authored-by: GitStart <1501599+gitstart@users.noreply.github.com>
Co-authored-by: gitstart <gitstart@users.noreply.github.com>
Co-authored-by: Nitesh Singh <nitesh.singh@gitstart.dev>
Co-authored-by: Júlio Piubello da Silva Cabral <julio.piubello@gitstart.dev>
Co-authored-by: Matheus Muniz <87545749+matheusmuniz03@users.noreply.github.com>
Co-authored-by: Matheus Benini <matheus_benini@hotmail.com>
Co-authored-by: Grace Nshokano <grace.devolop@gmail.com>
Co-authored-by: Matheus Muniz <matheusmuniz100@hotmail.com>
Co-authored-by: gitstart <gitstart@gitstart.com>
Co-authored-by: Murilo Amaral <87545137+MuriloAmarals@users.noreply.github.com>
Co-authored-by: Zach Waterfield <zlwaterfield@gmail.com>
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.

2 participants