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.

Sign up for GitHub

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

Jump to bottom

feat: Round Robin weights future members toggle #17782

Open
wants to merge 16 commits into
base: main
Choose a base branch
from
Open

feat: Round Robin weights future members toggle #17782

wants to merge 16 commits into from
+334 −18

Conversation

sean-brydon
Copy link
Member

@sean-brydon sean-brydon commented Nov 21, 2024 •
edited
Loading

What does this PR do?

Fixes: #17781 CAL-4771

Loom: https://www.loom.com/share/2df7aad3c6764094b3002f0c3530d2e4
CleanShot 2024-11-22 at 09 25 35

How should this be tested?

Assign a RR to a team event
Select all team members
Edit weights for users
Test out form - save - check sheet/db that the weights updated
Download the current csv
Modify the weights of some of the users
Upload the CSV with the new weights
Save
Test out form - save - check sheet/db that the weights updated

Checklist

  • I haven't read the contributing guide
  • My code doesn't follow the style guidelines of this project
  • I haven't commented my code, particularly in hard-to-understand areas
  • I haven't checked if my changes generate no new warnings

@graphite-app graphite-app bot requested a review from a team November 21, 2024 13:50
@github-actions github-actions bot added the High priority Created by Linear-GitHub Sync label Nov 21, 2024
@dosubot dosubot bot added event-types area: event types, event-types teams area: teams, round robin, collective, managed event-types ✨ feature New feature or request labels Nov 21, 2024
@keithwillcode keithwillcode added consumer core area: core, team members only labels Nov 21, 2024
sean-brydon
sean-brydon commented Nov 21, 2024
View reviewed changes
packages/features/eventtypes/components/EditWeightsForAllTeamMembers.tsx
const line = lines[i].trim();
if (!line) continue;

const [, , email, weightStr] = line.split(",");
Copy link
Member Author

Choose a reason for hiding this comment

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

We expect the format of the downloaded CSV. In the future we can rely on checking headers for a ID/EMAIL + weight so any CSV/sheets can be used

@graphite-app Graphite App
Copy link

graphite-app bot commented Nov 21, 2024

Graphite Automations

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

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

sean-brydon
sean-brydon commented Nov 21, 2024
View reviewed changes
packages/features/eventtypes/components/AddMembersWithSwitch.tsx Outdated
@@ -267,6 +270,15 @@ function AddMembersWithSwitch({
}}
onInactive={onAssignAllTeamMembersInactive}
/>
{!assignRRMembersUsingSegment ? (
Copy link
Member Author

Choose a reason for hiding this comment

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

Currently only render when were not using RR member assigment for attributes as the teamMember logic comes from internal and gets pretty complex. Will do this in a follow up to support weights on attribute assignees too

@vercel Vercel
Copy link

vercel bot commented Nov 21, 2024 •
edited
Loading

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

2 Skipped Deployments
Name Status Preview Comments Updated (UTC)
cal ⬜️ Ignored (Inspect) Visit Preview Nov 22, 2024 9:47am
calcom-web-canary ⬜️ Ignored (Inspect) Visit Preview Nov 22, 2024 9:47am

@sean-brydon sean-brydon force-pushed the feat/rr-weights-future-members-toggle branch from 7211443 to 7403547 Compare November 21, 2024 14:23
CarinaWolli
CarinaWolli requested changes Nov 21, 2024
View reviewed changes
Copy link
Member

@CarinaWolli CarinaWolli left a comment

Choose a reason for hiding this comment

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

Nice work 👏🏻 Left some small but blocking comments

Button looks a little lost there. Not sure how to make it look better, but maybe you have an idea (otherwise also fine for now)

Screenshot 2024-11-21 at 11 09 04 AM

packages/features/eventtypes/components/AddMembersWithSwitch.tsx Outdated Show resolved Hide resolved
packages/features/eventtypes/components/EditWeightsForAllTeamMembers.tsx Show resolved Hide resolved
packages/features/eventtypes/components/AddMembersWithSwitch.tsx Outdated Show resolved Hide resolved
@github-actions github-actions bot marked this pull request as draft November 21, 2024 16:15
@CarinaWolli
Copy link
Member

Screenshot 2024-11-21 at 11 22 37 AM

Can we make the toggle to enable weights visible and move the button up under this toggle?

@sean-brydon
Copy link
Member Author

Screenshot 2024-11-21 at 11 22 37 AM

Can we make the toggle to enable weights visible and move the button up under this toggle?
CleanShot 2024-11-22 at 09 27 13
CleanShot 2024-11-22 at 09 27 18
Fixed this :)

@sean-brydon sean-brydon marked this pull request as ready for review November 22, 2024 14:38
@github-actions GitHub Actions
Copy link
Contributor

E2E results are ready!

@CarinaWolli CarinaWolli self-requested a review November 22, 2024 16:03
@CarinaWolli
Copy link
Member

When I go to 'edit weight', it correctly shows the weights. However, when I then click 'cancel' and go back to 'edit weight' all weights show as 100. See loom https://www.loom.com/share/1d73169aeab341d7b6a1376c3af63bb6

CarinaWolli
CarinaWolli requested changes Nov 26, 2024
View reviewed changes
Copy link
Member

@CarinaWolli CarinaWolli left a comment

Choose a reason for hiding this comment

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

see comment: #17782 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@CarinaWolli CarinaWolli CarinaWolli requested changes

Requested changes must be addressed to merge this pull request.

Assignees
No one assigned
Labels
consumer core area: core, team members only event-types area: event types, event-types ✨ feature New feature or request High priority Created by Linear-GitHub Sync ready-for-e2e teams area: teams, round robin, collective, managed event-types
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[CAL-4771] UI Weights in RR when all team members assigned
3 participants
@sean-brydon @CarinaWolli @keithwillcode