fix: Routing Forms, Number operators were using string operands by hariombalhara · Pull Request #9182 · calcom/cal.diy

hariombalhara · 2023-05-29T13:51:56Z

What does this PR do?

Makes sure that number operators receives input as number instead of string.
Implements missing >, >=, <, <= operators for Reporting.

Fixes #9170 Fixes CAL-1806

Demo

After the fix demo

Type of change

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

How should this be tested?

Add a number type field say age.
- Choose age greater than 50, and select router action as a custom page(You can set the custom page message as 'the age is greater than 50')
- Go to Routing Form and input 100. Submit the form and you would see the Fallback Message instead when infact 100 > 50. This issue is fixed with the PR.
Similarly, for reporting, add a filter of age > 50 and check if you see the just added response with age 100.
- Now, add a filter of age < 50, you would not see the recently added response of age 50. But in the main you would see it.
Do a 'Test Preview' and make sure that adding a 100 their shows the custom page, 'the age is greater than 50'

Checklist

I haven't added tests that prove my fix is effective or that my feature works

vercel · 2023-05-29T13:52:03Z

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

Name	Status	Preview	Comments	Updated (UTC)
cal	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	May 31, 2023 0:35am
ui	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	May 31, 2023 0:35am

1 Ignored Deployment

Name	Status	Preview	Comments	Updated (UTC)
api	⬜️ Ignored (Inspect)			May 31, 2023 0:35am

github-actions · 2023-05-29T14:04:46Z

📦 Next.js Bundle Analysis for @calcom/web

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

Ten 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`	`254.97 KB`	406.16 KB	116.05% (🟢 -0.14%)
`/apps/[slug]/[...pages]`	`459.24 KB`	610.43 KB	174.41% (🟡 +0.26%)
`/auth/setup`	`174.9 KB`	326.09 KB	93.17% (🟡 +0.16%)
`/d/[link]/book`	`254.62 KB`	405.81 KB	115.95% (🟢 -0.15%)
`/event-types/[type]`	`479.62 KB`	630.81 KB	180.23% (🟡 +0.19%)
`/getting-started/[[...step]]`	`426.28 KB`	577.47 KB	164.99% (🟢 -0.22%)
`/new-booker/[user]/[type]`	`290.24 KB`	441.42 KB	126.12% (🟢 -0.14%)
`/new-booker/team/[slug]/[type]`	`290.24 KB`	441.43 KB	126.12% (🟢 -0.14%)
`/settings/my-account/calendars`	`252.55 KB`	403.74 KB	115.35% (🟢 -0.23%)
`/team/[slug]/book`	`254.62 KB`	405.81 KB	115.95% (🟢 -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.

…tors

linear · 2023-05-30T05:02:42Z

CAL-1806 Routing Form: number comparison operators seem to operate on strings instead of numbers

Greater than operator b/w 500 and 1000 considers 500 as bigger.

https://calcominc.slack.com/archives/C050R8Z2RP0/p1685143897762489

hariombalhara · 2023-05-30T05:18:39Z

+  ">": {
+    operator: "gt",
+    secondaryOperand: null,
+  },
+  ">=": {
+    operator: "gte",
+    secondaryOperand: null,
+  },
+  "<": {
+    operator: "lt",
+    secondaryOperand: null,
+  },
+  "<=": {
+    operator: "lte",
+    secondaryOperand: null,
+  },


These operators were missing implementation

hariombalhara · 2023-05-30T07:39:25Z

-    const fieldResponse =
-      field.type === "multiselect" ? rawFieldResponse.split(",").map((r) => r.trim()) : rawFieldResponse;
+    const fieldResponse = fieldsResponses[getFieldIdentifier(field)] || "";
+    const value =


Renamed fieldResponse as value(that better describes what it holds). Use, fieldResponse in place of rawFieldResponse.

hariombalhara · 2023-05-30T07:40:26Z

    z.object({
      label: z.string(),
-      value: z.union([z.string(), z.array(z.string())]),
+      value: z.union([z.string(), z.number(), z.array(z.string())]),


Allow number to be stored now.

hariombalhara · 2023-05-30T08:00:39Z

        } else {
-          serializedValue = escapeCsvText(value);
+          // value can be a number as well for type Number field
+          serializedValue = escapeCsvText(String(value));


value has type string|number here. Make it string for CSV.

hariombalhara · 2023-05-30T08:01:58Z

+  const secondaryOperandAsNumber = typeof secondaryOperand === "string" ? Number(secondaryOperand) : null;
+
+  let prismaWhere;
+  if (secondaryOperandAsNumber) {


I rely on jsonLogicToPrisma.test.ts here.
Have added 4 new tests for >,>=, <, <=

hariombalhara · 2023-05-30T08:03:16Z

      return;
    }
+    const valueAsStringOrStringArray =
+      typeof fieldResponse.value === "number" ? String(fieldResponse.value) : fieldResponse.value;


fieldResponse.value is number|string|string[] here. Make it string|string[]

hariombalhara · 2023-05-30T08:03:56Z

-              path: ["505d3c3c-aa71-4220-93a9-6fd1e1087939", "value"],
-              string_contains: "a",
-            },
+    describe("Number Type", () => {


Added these new tests and restructured others.

hariombalhara

Self review done

hariombalhara · 2023-05-30T08:06:03Z

@@ -1,7 +1,7 @@
-import { GetServerSidePropsContext, GetServerSidePropsResult } from "next";
-import { CalendsoSessionUser } from "next-auth";
+import type { GetServerSidePropsContext, GetServerSidePropsResult } from "next";


Prettier did it.

deploysentinel · 2023-05-30T08:20:25Z

No failed tests 🎉

zomars

Tested and it's working. Nice job @hariombalhara

Fix operators for number

7c0218f

vercel Bot deployed to Preview – ui May 29, 2023 13:55 View deployment

vercel Bot deployed to Preview – cal May 29, 2023 13:56 View deployment

Merge remote-tracking branch 'origin/main' into fix/number-type-opera…

64bd71e

…tors

Add number operator unit tests

03aea42

hariombalhara commented May 30, 2023

View reviewed changes

vercel Bot deployed to Preview – ui May 30, 2023 05:21 View deployment

vercel Bot deployed to Preview – cal May 30, 2023 05:23 View deployment

vercel Bot deployed to Preview – cal May 30, 2023 06:24 View deployment

hariombalhara force-pushed the fix/number-type-operators branch from dbada0f to 518d125 Compare May 30, 2023 07:21

vercel Bot deployed to Preview – cal May 30, 2023 07:26 View deployment

hariombalhara commented May 30, 2023

View reviewed changes

hariombalhara changed the title ~~Fix operators for number~~ Fix: Routing Forms, Number operators were using string operands May 30, 2023

hariombalhara force-pushed the fix/number-type-operators branch from 518d125 to 89aba8a Compare May 30, 2023 07:46

vercel Bot deployed to Preview – cal May 30, 2023 07:52 View deployment

hariombalhara force-pushed the fix/number-type-operators branch from 89aba8a to 53ec285 Compare May 30, 2023 07:54

hariombalhara changed the title ~~Fix: Routing Forms, Number operators were using string operands~~ fix: Routing Forms, Number operators were using string operands May 30, 2023

hariombalhara marked this pull request as ready for review May 30, 2023 07:59

hariombalhara requested review from CarinaWolli and zomars May 30, 2023 07:59

hariombalhara commented May 30, 2023

View reviewed changes

vercel Bot deployed to Preview – cal May 30, 2023 08:01 View deployment

hariombalhara commented May 30, 2023

View reviewed changes

hariombalhara force-pushed the fix/number-type-operators branch from 53ec285 to 2df59f3 Compare May 30, 2023 08:05

hariombalhara commented May 30, 2023

View reviewed changes

hariombalhara added the High priority Created by Linear-GitHub Sync label May 30, 2023

vercel Bot deployed to Preview – cal May 30, 2023 08:10 View deployment

PeerRich added the 🧹 Improvements Improvements to existing features. Mostly UX/UI label May 30, 2023

hariombalhara added 🐛 bug Something isn't working and removed 🧹 Improvements Improvements to existing features. Mostly UX/UI labels May 30, 2023

Fixes and remove between operators

f17c068

hariombalhara force-pushed the fix/number-type-operators branch from 2df59f3 to f17c068 Compare May 30, 2023 09:57

vercel Bot deployed to Preview – cal May 30, 2023 10:02 View deployment

Merge branch 'main' into fix/number-type-operators

5d7742c

vercel Bot deployed to Preview – ui May 31, 2023 00:34 View deployment

vercel Bot deployed to Preview – cal May 31, 2023 00:35 View deployment

zomars approved these changes Jun 1, 2023

View reviewed changes

zomars added this pull request to the merge queue Jun 1, 2023

Merged via the queue into main with commit 484f603 Jun 1, 2023

zomars deleted the fix/number-type-operators branch June 1, 2023 20:41

PeerRich added the core area: core, team members only label Jul 24, 2023

Conversation

hariombalhara commented May 29, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What does this PR do?

Demo

Type of change

How should this be tested?

Checklist

Uh oh!

vercel Bot commented May 29, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions Bot commented May 29, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

📦 Next.js Bundle Analysis for @calcom/web

Ten Pages Changed Size

Uh oh!

linear Bot commented May 30, 2023

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

hariombalhara left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

deploysentinel Bot commented May 30, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

zomars left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

hariombalhara commented May 29, 2023 •

edited

Loading

vercel Bot commented May 29, 2023 •

edited

Loading

github-actions Bot commented May 29, 2023 •

edited

Loading

deploysentinel Bot commented May 30, 2023 •

edited

Loading