feat: min max chars for long text by varshith257 · Pull Request #15691 · calcom/cal.com

varshith257 · 2024-07-08T15:34:00Z

What does this PR do?

Part of #15577

This PR adds an optional minimum and maximum character limit to long text question in the booking form.

Loom Video: https://www.loom.com/share/735a6768a92847cabefd1fe2187c499d?sid=2d606562-c6e6-4bd3-92c0-b1fcdab14218

Mandatory Tasks (DO NOT REMOVE)

I have self-reviewed the code (A decent size PR without self-review might be rejected).
I have added a Docs issue here if this PR makes changes that would require a documentation change. If N/A, write N/A here and check the checkbox.
I confirm automated tests are in place that prove my fix is effective or that my feature works.

How should this be tested?

Are there environment variables that should be set?
No
What are the minimal test data to have?
NA
What is expected (happy path) to have (input and output)?
Click on Event Types in the sidebar.
Click on an event.
Inside the event, click on the Advanced tab on the left sidebar.
Add / edit a question.
Set type to Long text
Set the min &/or max characters.
Save the question.
Save the form.
Click on the Preview button in the event header.
Select a date on the calendar.
While filling out the form, click save button with:
a. Min &/or max character limit unsatisfied -> should show red text with error.
b. All the conditions satisfied -> Should save successfully.
Any other important info that could help to test that PR

/claim #15577

…n-max-chars

…chars' into features/long-text-min-max-chars

…long-text-min-max-chars

vercel · 2024-07-08T15:34:06Z

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

A member of the Team first needs to authorize it.

varshith257 · 2024-07-09T04:13:36Z

I feel this is better and cleaner with passing e2e tests too @anikdhabal is also following up with his reviews and am following it.

To work together then let us split the bounty as of efforts taken

WDYT? @hariombalhara . Collaboration is cool in Open Source :)

rajesh-jonnalagadda · 2024-07-10T06:00:05Z

I feel this is better and cleaner with passing e2e tests too @anikdhabal is also following up with his reviews and am following it.

To work together then let us split the bounty as of efforts taken

WDYT? @hariombalhara . Collaboration is cool in Open Source :)

I have closed my pr. please handle the edge case. I have handled it in my pr. if needed take reference.
when user manually clears the max/min inputs and save the question. it gets saved in local only. It is not getting updated in db. that needs to be handled.

packages/features/form-builder/FormBuilder.tsx

            const fieldType = fieldTypesConfigMap[type];
            //handling edge-case. when user manually cleared the maxLength and minLength, making sure default values were set
            if (fieldType?.supportsLengthCheck) {
              if (!data.maxLength) {
                data.maxLength = fieldType.supportsLengthCheck.maxLength || 1000;
              }
              //if NaN, set it to 0
              if (!data.minLength) {
                data.minLength = 0;
              }
            }

varshith257 · 2024-07-10T10:02:22Z

I have closed my pr. please handle the edge case. I have handled it in my pr. if needed take reference.

I think it's not needed IMO

varshith257 · 2024-07-10T18:35:30Z

@hariombalhara @anikdhabal Can have a look into it and merge it?

hariombalhara · 2024-07-12T10:53:39Z

packages/features/form-builder/schema.ts

+      if (hasExceededMaxLength) {
+        ctx.addIssue({
+          code: z.ZodIssueCode.custom,
+          message: m(`Max. ${field.maxLength} characters allowed`),
+        });
+      }


Getting wrong error message(undefined in it) for a field that existed before these changes. Such a field doesn't have field.maxLength set

…issue

hariombalhara · 2024-07-12T10:59:12Z

packages/features/form-builder/FormBuilder.tsx

+function FieldWithLengthCheckSupport({
+  fieldForm,
+  containerClassName = "",
+  className,
+  ...rest


Refactored the new component changes out from the parent component.

hariombalhara · 2024-07-12T11:00:30Z

packages/features/form-builder/schema.ts

+      const maxLength = field.maxLength ?? fieldTypeConfig.supportsLengthCheck?.maxLength;
+      const minLength = field.minLength ?? 0;


Ensured that maxLength and minLength are initialized correctly and use them only and not field props.

hariombalhara

@varshith257 Made some changes to cleanup the code a bit more and fixed a bug.
Please review the changes made by me. It is good to go from my side.

@ThyMinimalDev Please review the v2 API related changes.

varshith257 · 2024-07-12T11:11:02Z

@hariombalhara Thanks for fixing the bug and cleanup. I have semester exams ongoing so hadn't looked at it.

hariombalhara · 2024-07-12T11:16:51Z

@hariombalhara Thanks for fixing the bug and cleanup. I have semester exams ongoing so hadn't looked at it.

No Problem. Good luck with your exams !!

supalarry

Great PR and well done on adding e2e test!

However, the API part needs:

To specifiy min and max length on the individual booking field input and output, not the whole event-type object.
To add this functionality also to the latest event-types release.

Let me know if I can provide more details / explain anything : )

supalarry · 2024-07-18T11:18:29Z

apps/api/v2/src/ee/event-types/event-types_2024_04_15/inputs/update-event-type.input.ts

  @ValidateNested({ each: true })
  @Type(() => Source)
  sources?: Source[];
+


I see that inputs and outputs of event-types in folder event-types_2024_04_15 are updated, but we also need to update inputs and outputs used by next release in the folder event-types_2024_06_14

The controller apps/api/v2/src/ee/event-types/event-types_2024_06_14/controllers/event-types.controller.ts takes inputs from packages/platform/types/event-types/event-types_2024_06_14/inputs and outputs from apps/api/v2/src/ee/event-types/event-types_2024_06_14/outputs that share underlying event-type output defined in packages/platform/types/event-types/event-types_2024_06_14/outputs/event-type.output.ts.

As you see, the inputs and outputs are in the platform-types package , which means that after changing them you have to re-build them by running rm -rf dist && npx tsc --build --force in the packages/platform/types folder.

supalarry · 2024-07-18T11:22:48Z

apps/api/v2/src/ee/event-types/event-types_2024_04_15/inputs/update-event-type.input.ts

+
+  @IsOptional()
+  @IsNumber()
+  maxLength?: number;


I think that maxLength and minLength should not be defined on the input object, but on the level of individual booking field, be it class BookingField_2024_04_15 or TextField_2024_06_14 within packages/platform/types/event-types/event-types_2024_06_14/inputs/booking-fields.input.ts.

Same logic applies as comment above - we need to also handle the 2024_06_14 release of event-types.

supalarry · 2024-07-18T11:23:56Z

apps/api/v2/src/ee/event-types/event-types_2024_04_15/outputs/get-event-type-public.output.ts

+
+  @IsOptional()
+  @IsNumber()
+  maxLength?: number;


Same as with inputs above - I think min and max length should be part of the individual booking field, because right now we are defining min and max length of an event type.

keithwillcode · 2024-07-29T04:04:36Z

@supalarry mind making these changes? Sounds like @varshith257 is busy and might not be able to get them in.

supalarry · 2024-07-29T09:09:38Z

Now that I think, it's better to do API updates seperately, so I will just

Now that I think it's better to have web app only update in this PR and then API update in separate one - I will remove API stuff from this PR so we can ship this already and add to-do for this API task.

supalarry

Great job Vamshi! Tested locally and works!

stale

mhetreayush and others added 29 commits June 28, 2024 11:41

feat: min max chars for long text

8c26d2b

refactor: remove console log

c4c224c

refactor: revert redundant change

f6e9414

refactor: review changes

909d9c2

fix: min max validation

30086cf

fix: fix typo

48ddf6c

refactor: update comments

160a1ff

refactor: review changes

58fe0a8

refactor: review changes

54e4427

test: Add test

444a44f

Merge remote-tracking branch 'origin/main' into features/long-text-mi…

185298e

…n-max-chars

Merge branch 'main' into features/long-text-min-max-chars

5457fa9

Merge remote-tracking branch 'mhetreayush/features/long-text-min-max-…

43402db

…chars' into features/long-text-min-max-chars

Make maxLength more configurable per field type

28eb750

Reduce maxLength to 1000

3dc3b39

Update update-event-type.input.ts

6b457a2

revert changes in main

fddb9c1

Update update-event-type.input.ts

8b2cf01

refactor get-event-type-public.output.ts

6a849ab

add e2e tests

8705580

add min max char common.json

dc1da98

Update FormBuilder.tsx

d64e736

refactor changes

d53287e

Update fieldTypes.ts

992a616

Update schema.ts

ee32d90

add min max length in api-request

3cb9701

Update booking-fields.input.ts

536117d

Merge branch 'calcom:features/long-text-min-max-chars' into features/…

7724a1d

…long-text-min-max-chars

fix e2e tests

8dda10f

Merge branch 'main' into features/long-text-min-max-chars

a1bb03d

varshith257 and others added 4 commits July 11, 2024 00:05

Merge branch 'main' into features/long-text-min-max-chars

50d3928

Merge branch 'main' into features/long-text-min-max-chars

bb2bec1

Abstract out SupportsLengthcheck type Field

56fe192

Revert newline change

5457fcc

hariombalhara reviewed Jul 12, 2024

View reviewed changes

Improve the schema validation a bit and fix undefined in the message …

3bbd090

…issue

hariombalhara reviewed Jul 12, 2024

View reviewed changes

hariombalhara previously approved these changes Jul 12, 2024

View reviewed changes

hariombalhara requested a review from ThyMinimalDev July 12, 2024 11:05

Amit91848 requested a review from a team July 18, 2024 09:01

supalarry requested changes Jul 18, 2024

View reviewed changes

revert: v2 API changes

7094048

supalarry dismissed hariombalhara’s stale review via 7094048 July 29, 2024 09:15

supalarry self-requested a review July 29, 2024 09:21

supalarry approved these changes Jul 29, 2024

View reviewed changes

Merge branch 'main' into features/long-text-min-max-chars

63f1252

hariombalhara approved these changes Jul 30, 2024

View reviewed changes

hariombalhara enabled auto-merge (squash) July 30, 2024 12:02

hariombalhara merged commit b8ba95d into calcom:main Jul 30, 2024

		const maxLength = field.maxLength ?? fieldTypeConfig.supportsLengthCheck?.maxLength;
		const minLength = field.minLength ?? 0;

Conversation

varshith257 commented Jul 8, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What does this PR do?

Mandatory Tasks (DO NOT REMOVE)

How should this be tested?

Uh oh!

vercel bot commented Jul 8, 2024

Uh oh!

varshith257 commented Jul 9, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rajesh-jonnalagadda commented Jul 10, 2024

Uh oh!

varshith257 commented Jul 10, 2024

Uh oh!

varshith257 commented Jul 10, 2024

Uh oh!

hariombalhara Jul 12, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

hariombalhara Jul 12, 2024

Choose a reason for hiding this comment

Uh oh!

hariombalhara Jul 12, 2024

Choose a reason for hiding this comment

Uh oh!

hariombalhara left a comment

Choose a reason for hiding this comment

Uh oh!

varshith257 commented Jul 12, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

hariombalhara commented Jul 12, 2024

Uh oh!

supalarry left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

supalarry Jul 18, 2024

Choose a reason for hiding this comment

Uh oh!

supalarry Jul 18, 2024

Choose a reason for hiding this comment

Uh oh!

supalarry Jul 18, 2024

Choose a reason for hiding this comment

Uh oh!

keithwillcode commented Jul 29, 2024

Uh oh!

supalarry commented Jul 29, 2024

Uh oh!

supalarry left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

varshith257 commented Jul 8, 2024 •

edited

Loading

varshith257 commented Jul 9, 2024 •

edited

Loading

hariombalhara Jul 12, 2024 •

edited

Loading

varshith257 commented Jul 12, 2024 •

edited

Loading

supalarry left a comment •

edited

Loading