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: add default schedule while creating org member #16855

Merged
merged 15 commits into from
Nov 18, 2024

Conversation

vijayraghav-io
Copy link
Contributor

@vijayraghav-io vijayraghav-io commented Sep 27, 2024

What does this PR do?

BeforeChanges :
https://www.loom.com/share/232fd6ddfdc8407f93d1a967fe6c191f?sid=c9da5ff0-1f45-4d2a-b8fc-e8053a557be1

AfterChanges :
https://www.loom.com/share/26e808cb4d6b4d8abd4288ed096bfd14?sid=514088a9-9244-442f-8265-cca21a23bf38

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.

Copy link

vercel bot commented Sep 27, 2024

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

A member of the Team first needs to authorize it.

@graphite-app graphite-app bot requested a review from a team September 27, 2024 08:25
@graphite-app graphite-app bot added the community Created by Linear-GitHub Sync label Sep 27, 2024
@github-actions github-actions bot added organizations area: organizations, orgs teams area: teams, round robin, collective, managed event-types 🐛 bug Something isn't working labels Sep 27, 2024
@dosubot dosubot bot added the platform Anything related to our platform plan label Sep 27, 2024
@graphite-app graphite-app bot requested a review from a team September 27, 2024 08:26
Copy link

graphite-app bot commented Sep 27, 2024

Graphite Automations

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

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

"Add community label" took an action on this PR • (09/27/24)

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

"Add platform team as reviewer" took an action on this PR • (09/27/24)

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

@@ -307,6 +310,8 @@ export async function createNewUsersConnectToOrgIfExists({

const isBecomingAnOrgMember = parentId || isOrg;

const availability = getAvailabilityFromSchedule(DEFAULT_SCHEDULE);
Copy link
Contributor

@Ryukemeister Ryukemeister Sep 28, 2024

Choose a reason for hiding this comment

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

How about we rename this to defaultScheduleAvailability or defaultAvailability?

@@ -340,6 +345,21 @@ export async function createNewUsersConnectToOrgIfExists({
accepted: autoAccept, // If the user is invited to a child team, they are automatically accepted
},
},
// Default schedule
Copy link
Contributor

Choose a reason for hiding this comment

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

I think its alright if you remove the comment

Copy link
Contributor

This PR is being marked as stale due to inactivity.

@github-actions github-actions bot added the Stale label Oct 14, 2024
@github-actions github-actions bot removed the Stale label Oct 15, 2024
@hariombalhara hariombalhara added the Urgent Created by Linear-GitHub Sync label Oct 16, 2024
Copy link
Contributor

This PR is being marked as stale due to inactivity.

@github-actions github-actions bot added the Stale label Oct 31, 2024
@vijayraghav-io
Copy link
Contributor Author

Gentle reminder!

@github-actions github-actions bot removed the Stale label Nov 2, 2024
@keithwillcode keithwillcode added this to the v4.7 milestone Nov 3, 2024
@github-actions github-actions bot added the High priority Created by Linear-GitHub Sync label Nov 8, 2024
@vijayraghav-io
Copy link
Contributor Author

added an e2e test

Copy link
Contributor

github-actions bot commented Nov 11, 2024

E2E results are ready!

@keithwillcode keithwillcode removed the platform Anything related to our platform plan label Nov 11, 2024
@keithwillcode keithwillcode modified the milestones: v4.7, v4.8 Nov 18, 2024
@dosubot dosubot bot modified the milestone: v4.8 Nov 18, 2024
@anikdhabal anikdhabal enabled auto-merge (squash) November 18, 2024 13:06
Copy link
Contributor

@anikdhabal anikdhabal left a comment

Choose a reason for hiding this comment

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

LGTM!!

@anikdhabal anikdhabal merged commit 7b94822 into calcom:main Nov 18, 2024
32 of 36 checks passed
@supalarry
Copy link
Contributor

supalarry commented Dec 19, 2024

@anikdhabal @vijayraghav-io the created schedules are missing timeZone. Anik, hasn't this caused any issues?

When createNewUsersConnectToOrgIfExists is invoked we are passing timeZone to it, so we should also use same time zone in the schedules create call for the default working schedules.

Furthermore, if timeZone is not passed for the User model it defaults to "Europe/London" but for Schedule model to nothing, so I think it makes sense to update prisma schema for Schedule to also have "Europe/London" timeZone as default.

@anikdhabal
Copy link
Contributor

Hey @supalarry, I don't think it causes issues. We've done it in other places as well, e.g., in the userRepository. What issue are you anticipating? But yes, adding the timezone would make sense here. @vijayraghav-io, could you check it and do a follow-up?

@vijayraghav-io
Copy link
Contributor Author

Hey @supalarry, I don't think it causes issues. We've done it in other places as well, e.g., in the userRepository. What issue are you anticipating? But yes, adding the timezone would make sense here. @vijayraghav-io, could you check it and do a follow-up?

@anikdhabal, sure will check this and do a follow-up.

@vijayraghav-io
Copy link
Contributor Author

Hi @supalarry, @anikdhabal, i checked and verified this , even if timezone is not explicitly input, the schedule is created and availability shown as per default timezone - Europe/London.

Updated to create schedule as per input timezone, if timezone is input for createNewUsersConnectToOrgIfExists else timezone is saved with default value - Europe/London.

#18288

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🐛 bug Something isn't working community Created by Linear-GitHub Sync High priority Created by Linear-GitHub Sync organizations area: organizations, orgs ready-for-e2e teams area: teams, round robin, collective, managed event-types Urgent Created by Linear-GitHub Sync
Projects
None yet
6 participants