fix: consider sendToAll settings in routingForms response #18226

vijayraghav-io · 2024-12-17T08:00:29Z

What does this PR do?

Fixes [CAL-4912] Routing Form: Send Email, 'Add Future Team members' toggle doesn't consider team members added later. #18223
Fixes CAL-4912

Before Fix:
https://www.loom.com/share/d800f724d7a64636bf30a16b4dc4d999?sid=ffacca4c-bb87-4338-b134-c44cee8ec0a4

After Fix:
https://www.loom.com/share/ad8eb69159344c3f99bce9dd8f9cb505?sid=741fccb5-3278-48d1-8fb4-520bf32cb572

Mandatory Tasks (DO NOT REMOVE)

I have self-reviewed the code (A decent size PR without self-review might be rejected).
- N/A - I have updated the developer docs in /docs 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.

vercel · 2024-12-17T08:00:35Z

@vijayraghav-io is attempting to deploy a commit to the cal Team on Vercel.

A member of the Team first needs to authorize it.

github-actions · 2024-12-17T08:00:42Z

Hey there and thank you for opening this pull request! 👋🏼

We require pull request titles to follow the Conventional Commits specification and it looks like your proposed title needs to be adjusted.

Details:

No release type found in pull request title "update to consider sendToAll". Add a prefix to indicate what kind of release this pull request corresponds to. For reference, see https://www.conventionalcommits.org/

Available types:
 - feat: A new feature
 - fix: A bug fix
 - docs: Documentation only changes
 - style: Changes that do not affect the meaning of the code (white-space, formatting, missing semi-colons, etc)
 - refactor: A code change that neither fixes a bug nor adds a feature
 - perf: A code change that improves performance
 - test: Adding missing tests or correcting existing tests
 - build: Changes that affect the build system or external dependencies (example scopes: gulp, broccoli, npm)
 - ci: Changes to our CI configuration files and scripts (example scopes: Travis, Circle, BrowserStack, SauceLabs)
 - chore: Other changes that don't modify src or test files
 - revert: Reverts a previous commit

graphite-app · 2024-12-17T08:05:11Z

Graphite Automations

"Add consumer team as reviewer" took an action on this PR • (12/17/24)

1 reviewer was added to this PR based on Keith Williams's automation.

"Add community label" took an action on this PR • (12/17/24)

1 label was added to this PR based on Keith Williams's automation.

"Add ready-for-e2e label" took an action on this PR • (12/18/24)

1 label was added to this PR based on Keith Williams's automation.

…utingForms_sendToAll

vijayraghav-io · 2024-12-17T09:33:04Z

Self-reviewed, Tested and verified locally with and without toggling "Add all team members, including future members", working as expected, sending mails to new members added.

Will attach a loom video in some time.
Also will try to add test.

…utingForms_sendToAll

Praashh · 2024-12-17T14:16:38Z

Self-reviewed, Tested and verified locally with and without toggling "Add all team members, including future members", working as expected, sending mails to new members added.

Will attach a loom video in some time. Also will try to add test.

cool, please add test cases 🙏

…utingForms_sendToAll

vijayraghav-io · 2024-12-18T10:49:32Z

cool, please add test cases 🙏

@Praashh, added test cases 🙏

packages/app-store/routing-forms/trpc/response.handler.ts

Praashh

LGTM !!

anikdhabal · 2024-12-18T16:38:30Z

packages/app-store/routing-forms/trpc/response.handler.ts

+      const whereClause: Prisma.MembershipWhereInput = { teamId: form.teamId };
+      if (!settings?.sendToAll) {
+        whereClause.userId = { in: settings.sendUpdatesTo };
+      }


Instead of doing this, shouldn't we find and update sendUpdatesTo to include future members? Why are we currently not adding future members to it?

Just wanted to avoid having an prisma update request on App_RoutingForms_Form table every time there is a response on the routing Form. (in cases of sendToAll)

If we take UI approach to update sendUpdatesTo when switch is selected in RoutingForm, we may miss on the scenario where - new team member is added after the switch in RoutingForm is selected and the sendUpdatesTo wont have future members until the switch is re-toggled.

IMO , even if we keep update sendUpdatesTo in sync, there is no advantage from this. Because any how, here in response.handler.ts we need to check for latest new members added if any.
I understand. that's the reason we have an additional variable sendToAll.

In other case, we will be making an unnecessary update operation for each response.

github-actions · 2024-12-18T16:42:21Z

E2E results are ready!

Workflow #48874.2 latest results

…utingForms_sendToAll

vijayraghav-io · 2024-12-19T08:11:38Z

@Praashh , @anikdhabal , FYI/A all tests are passing.

Just FYI,
The newly added second test - newly added team member Not gets mail updates, when "Add all team members, including future members" switch is Not selected was failing earlier NOT because of flakiness or timeout issues.

But, in case of Team RoutingForm, when AssignAllMembers toggle is off, email is not sent to owner as well by default. The owner also has to be selected manually from list. This step was missed earlier. Now added and tests are working fine.

vijayraghav-io · 2025-01-01T11:50:00Z

@Praashh , @anikdhabal , Reminding that comments are addressed

Praashh · 2025-01-01T14:50:24Z

can you please resolve conflicts @vijayraghav-io

…utingForms_sendToAll

vijayraghav-io · 2025-01-01T15:57:23Z

can you please resolve conflicts @vijayraghav-io

@Praashh, resolved conflicts.

Praashh

LGTM!!

github-actions · 2025-01-26T00:21:29Z

This PR is being marked as stale due to inactivity.

vijayraghav-io · 2025-01-26T08:14:00Z

Gentle reminder!

…utingForms_sendToAll

vijayraghav-io · 2025-01-26T15:39:56Z

resolved merge conflicts

…utingForms_sendToAll

github-actions · 2025-02-11T00:21:43Z

This PR is being marked as stale due to inactivity.

…utingForms_sendToAll

vijayraghav-io · 2025-02-12T06:52:08Z

@Praashh,
resolved conflicts.
Gentle reminder to kindly approve again and take it to merge.

apps/api/v2/src/modules/organizations/inputs/create-organization-team.input.ts

anikdhabal

Looks Good

vijayraghav-io · 2025-02-24T18:28:17Z

Thank you! 🙏

* update to consider sendToAll * added tests for team routingForms selectAllMembers cases * chore * chore * chore * updated test case in case of toggle off * tweak --------- Co-authored-by: Anik Dhabal Babu <[email protected]> Co-authored-by: unknown <[email protected]>

update to consider sendToAll

3e3acbf

graphite-app bot added the community Created by Linear-GitHub Sync label Dec 17, 2024

graphite-app bot requested a review from a team December 17, 2024 08:00

github-actions bot added emails area: emails, cancellation email, reschedule email, inbox, spam folder, not getting email routing-forms area: routing forms, routing, forms 🐛 bug Something isn't working labels Dec 17, 2024

vijayraghav-io changed the title ~~update to consider sendToAll~~ fix: consider sendToAll settings in routingForms response Dec 17, 2024

Merge branch 'main' of https://github.com/calcom/cal.com into fix__ro…

9d57a06

…utingForms_sendToAll

Merge branch 'main' of https://github.com/calcom/cal.com into fix__ro…

885bc4d

…utingForms_sendToAll

vijayraghav-io added 2 commits December 18, 2024 15:24

added tests for team routingForms selectAllMembers cases

3fa8b7f

Merge branch 'main' of https://github.com/calcom/cal.com into fix__ro…

3119d90

…utingForms_sendToAll

Praashh reviewed Dec 18, 2024

View reviewed changes

packages/app-store/routing-forms/trpc/response.handler.ts Outdated Show resolved Hide resolved

Praashh previously approved these changes Dec 18, 2024

View reviewed changes

graphite-app bot added the ready-for-e2e label Dec 18, 2024

anikdhabal requested changes Dec 18, 2024

View reviewed changes

chore

3755e37

vijayraghav-io dismissed Praashh’s stale review via 3755e37 December 18, 2024 17:06

vijayraghav-io added 5 commits December 18, 2024 23:25

chore

342f24d

Merge branch 'main' of https://github.com/calcom/cal.com into fix__ro…

ffdd1f7

…utingForms_sendToAll

chore

41747df

updated test case in case of toggle off

ac09aac

Merge branch 'main' of https://github.com/calcom/cal.com into fix__ro…

8e02471

…utingForms_sendToAll

Merge branch 'main' of https://github.com/calcom/cal.com into fix__ro…

e59faf7

…utingForms_sendToAll

Praashh previously approved these changes Jan 1, 2025

View reviewed changes

github-actions bot added the Stale label Jan 26, 2025

Merge branch 'main' of https://github.com/calcom/cal.com into fix__ro…

fd1ad32

…utingForms_sendToAll

vijayraghav-io dismissed Praashh’s stale review via fd1ad32 January 26, 2025 15:22

github-actions bot removed the Stale label Jan 27, 2025

Merge branch 'main' of https://github.com/calcom/cal.com into fix__ro…

3a0771f

…utingForms_sendToAll

github-actions bot added the Stale label Feb 11, 2025

Merge branch 'main' of https://github.com/calcom/cal.com into fix__ro…

215578a

…utingForms_sendToAll

vijayraghav-io requested a review from a team as a code owner February 12, 2025 06:27

github-actions bot removed the Stale label Feb 13, 2025

Merge branch 'main' into fix__routingForms_sendToAll

40f65f5

github-actions bot added the Stale label Feb 24, 2025

anikdhabal reviewed Feb 24, 2025

View reviewed changes

apps/api/v2/src/modules/organizations/inputs/create-organization-team.input.ts Outdated Show resolved Hide resolved

tweak

c54dc03

anikdhabal approved these changes Feb 24, 2025

View reviewed changes

Merge branch 'main' into fix__routingForms_sendToAll

cea6225

anikdhabal enabled auto-merge (squash) February 24, 2025 17:29

anikdhabal merged commit caade08 into calcom:main Feb 24, 2025
31 of 36 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: consider sendToAll settings in routingForms response #18226

fix: consider sendToAll settings in routingForms response #18226

vijayraghav-io commented Dec 17, 2024 •

edited

Loading

vercel bot commented Dec 17, 2024

github-actions bot commented Dec 17, 2024

graphite-app bot commented Dec 17, 2024 •

edited

Loading

vijayraghav-io commented Dec 17, 2024

Praashh commented Dec 17, 2024

vijayraghav-io commented Dec 18, 2024

Praashh left a comment

anikdhabal Dec 18, 2024

vijayraghav-io Dec 18, 2024

vijayraghav-io Dec 19, 2024 •

edited

Loading

github-actions bot commented Dec 18, 2024 •

edited

Loading

vijayraghav-io commented Dec 19, 2024 •

edited

Loading

vijayraghav-io commented Jan 1, 2025

Praashh commented Jan 1, 2025

vijayraghav-io commented Jan 1, 2025

Praashh left a comment

github-actions bot commented Jan 26, 2025

vijayraghav-io commented Jan 26, 2025

vijayraghav-io commented Jan 26, 2025

github-actions bot commented Feb 11, 2025

vijayraghav-io commented Feb 12, 2025

anikdhabal left a comment

vijayraghav-io commented Feb 24, 2025

fix: consider sendToAll settings in routingForms response #18226

fix: consider sendToAll settings in routingForms response #18226

Conversation

vijayraghav-io commented Dec 17, 2024 • edited Loading

What does this PR do?

Mandatory Tasks (DO NOT REMOVE)

vercel bot commented Dec 17, 2024

github-actions bot commented Dec 17, 2024

graphite-app bot commented Dec 17, 2024 • edited Loading

Graphite Automations

vijayraghav-io commented Dec 17, 2024

Praashh commented Dec 17, 2024

vijayraghav-io commented Dec 18, 2024

Praashh left a comment

Choose a reason for hiding this comment

anikdhabal Dec 18, 2024

Choose a reason for hiding this comment

vijayraghav-io Dec 18, 2024

Choose a reason for hiding this comment

vijayraghav-io Dec 19, 2024 • edited Loading

Choose a reason for hiding this comment

github-actions bot commented Dec 18, 2024 • edited Loading

E2E results are ready!

vijayraghav-io commented Dec 19, 2024 • edited Loading

vijayraghav-io commented Jan 1, 2025

Praashh commented Jan 1, 2025

vijayraghav-io commented Jan 1, 2025

Praashh left a comment

Choose a reason for hiding this comment

github-actions bot commented Jan 26, 2025

vijayraghav-io commented Jan 26, 2025

vijayraghav-io commented Jan 26, 2025

github-actions bot commented Feb 11, 2025

vijayraghav-io commented Feb 12, 2025

anikdhabal left a comment

Choose a reason for hiding this comment

vijayraghav-io commented Feb 24, 2025

vijayraghav-io commented Dec 17, 2024 •

edited

Loading

graphite-app bot commented Dec 17, 2024 •

edited

Loading

vijayraghav-io Dec 19, 2024 •

edited

Loading

github-actions bot commented Dec 18, 2024 •

edited

Loading

vijayraghav-io commented Dec 19, 2024 •

edited

Loading