-
Notifications
You must be signed in to change notification settings - Fork 331
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
chore: Make meeting series naming consistent #9928
Conversation
Make the meeting name a separate parameter so it can be used for the meeting, meeting series and gcal event. This makes everything more consistent.
WalkthroughThe changes across multiple files focus on simplifying the handling of recurrence settings by removing the Changes
Recent review detailsConfiguration used: CodeRabbit UI Files selected for processing (6)
Files skipped from review as they are similar to previous changes (2)
Additional context usedLearnings (1)
Additional comments not posted (4)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
@@ -889,3 +889,27 @@ export const fileStoreAsset = (parent: RootDataLoader) => { | |||
} | |||
) | |||
} | |||
|
|||
export const meetingCount = (parent: RootDataLoader) => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it's used in multiple nested calls now and instead of passing the count around, I opted to just add it to the dataloader.
@@ -32,7 +32,7 @@ export default class MeetingAction extends Meeting { | |||
phases, | |||
facilitatorUserId, | |||
meetingType: 'action', | |||
name: name ?? `Check-in #${meetingCount + 1}` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think naming an instance is not part of the data type, but belongs into the mutation, so I moved it out consistently.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 6
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (26)
- packages/client/components/ActivityLibrary/ActivityDetailsSidebar.tsx (3 hunks)
- packages/client/components/ActivityLibrary/ScheduleMeetingButton.tsx (3 hunks)
- packages/client/components/Recurrence/EndRecurringMeetingModal.tsx (1 hunks)
- packages/client/components/Recurrence/UpdateRecurrenceSettingsModal.tsx (2 hunks)
- packages/client/components/ScheduleDialog.tsx (5 hunks)
- packages/client/mutations/StartRetrospectiveMutation.ts (1 hunks)
- packages/client/mutations/StartTeamPromptMutation.ts (1 hunks)
- packages/client/mutations/UpdateRecurrenceSettingsMutation.ts (1 hunks)
- packages/server/tests/startRetrospective.test.ts (4 hunks)
- packages/server/database/types/MeetingAction.ts (2 hunks)
- packages/server/database/types/MeetingPoker.ts (2 hunks)
- packages/server/dataloader/customLoaderMakers.ts (1 hunks)
- packages/server/graphql/mutations/helpers/createGcalEvent.ts (5 hunks)
- packages/server/graphql/mutations/helpers/safeCreateRetrospective.ts (2 hunks)
- packages/server/graphql/mutations/startSprintPoker.ts (4 hunks)
- packages/server/graphql/public/mutations/startCheckIn.ts (3 hunks)
- packages/server/graphql/public/mutations/startRetrospective.ts (6 hunks)
- packages/server/graphql/public/mutations/startTeamPrompt.ts (3 hunks)
- packages/server/graphql/public/mutations/updateRecurrenceSettings.ts (2 hunks)
- packages/server/graphql/public/typeDefs/CreateGcalEventInput.graphql (1 hunks)
- packages/server/graphql/public/typeDefs/_legacy.graphql (1 hunks)
- packages/server/graphql/public/typeDefs/startCheckIn.graphql (1 hunks)
- packages/server/graphql/public/typeDefs/startRetrospective.graphql (2 hunks)
- packages/server/graphql/public/typeDefs/startTeamPrompt.graphql (2 hunks)
- packages/server/graphql/public/typeDefs/updateRecurrenceSettings.graphql (1 hunks)
- packages/server/graphql/public/types/CreateGcalEventInput.ts (2 hunks)
Files skipped from review due to trivial changes (2)
- packages/client/mutations/StartRetrospectiveMutation.ts
- packages/server/graphql/public/typeDefs/CreateGcalEventInput.graphql
Additional context used
Biome
packages/server/graphql/public/types/CreateGcalEventInput.ts
[error] 39-39: Shouldn't redeclare 'GcalVideoTypeEnum'. Consider to delete it or rename it.
'GcalVideoTypeEnum' is defined here:
(lint/suspicious/noRedeclare)
Additional comments not posted (58)
packages/server/graphql/public/typeDefs/startCheckIn.graphql (1)
23-25
: Ensure the new parametername
is handled correctly.The
name
parameter has been added to thestartCheckIn
mutation. Ensure that the corresponding resolver and related code handle this parameter correctly.Verification successful
The new parameter
name
is handled correctly.The
name
parameter in thestartCheckIn
mutation is properly destructured from the input, assigned a default value if not provided, and used in the meeting object.
packages/server/graphql/public/mutations/startCheckIn.ts
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the resolver and related code handle the new `name` parameter correctly. # Test: Search for the resolver implementation. Expect: The resolver should handle the `name` parameter. rg --type js --type ts $'startCheckIn'Length of output: 536
Script:
#!/bin/bash # Description: Verify the handling of the `name` parameter in the startCheckIn resolver. # Search for the usage of the `name` parameter in the startCheckIn resolver file. rg 'name' packages/server/graphql/public/mutations/startCheckIn.tsLength of output: 170
packages/server/graphql/public/typeDefs/startRetrospective.graphql (2)
11-13
: Ensure the new parametername
is handled correctly.The
name
parameter has been added to thestartRetrospective
mutation. Ensure that the corresponding resolver and related code handle this parameter correctly.Verification successful
The
name
parameter is handled correctly.The resolver implementation, test cases, and client-side mutation all correctly handle the
name
parameter.
packages/server/graphql/public/mutations/startRetrospective.ts
: Resolver implementation includes thename
parameter.packages/server/__tests__/startRetrospective.test.ts
: Test cases verify the correct naming of the meeting.packages/server/__tests__/disableAnonymity.test.ts
: The mutation call is valid without thename
parameter as it is optional.packages/client/mutations/StartRetrospectiveMutation.ts
: Client-side mutation includes and correctly passes thename
parameter.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the resolver and related code handle the new `name` parameter correctly. # Test: Search for the resolver implementation. Expect: The resolver should handle the `name` parameter. rg --type js --type ts $'startRetrospective'Length of output: 1317
Script:
#!/bin/bash # Extract the resolver implementation for startRetrospective echo "Resolver implementation in startRetrospective.ts:" rg --type js --type ts --context 10 'startRetrospective' packages/server/graphql/public/mutations/startRetrospective.ts # Extract test cases involving startRetrospective in startRetrospective.test.ts echo -e "\nTest cases in startRetrospective.test.ts:" rg --type js --type ts --context 10 'startRetrospective' packages/server/__tests__/startRetrospective.test.ts # Extract test cases involving startRetrospective in disableAnonymity.test.ts echo -e "\nTest cases in disableAnonymity.test.ts:" rg --type js --type ts --context 10 'startRetrospective' packages/server/__tests__/disableAnonymity.test.ts # Extract client-side mutation handling in StartRetrospectiveMutation.ts echo -e "\nClient-side mutation handling in StartRetrospectiveMutation.ts:" rg --type js --type ts --context 10 'startRetrospective' packages/client/mutations/StartRetrospectiveMutation.tsLength of output: 6508
15-17
: Ensure the new parameterrrule
is handled correctly.The
rrule
parameter has been added to thestartRetrospective
mutation. Ensure that the corresponding resolver and related code handle this parameter correctly.Verification successful
The
rrule
parameter is handled correctly.The
startRetrospective
resolver function inpackages/server/graphql/public/mutations/startRetrospective.ts
includes therrule
parameter. It is utilized in several places within the function, such as in thecreateMeetingSeriesTitle
andstartNewMeetingSeries
calls, and is also passed to thecreateGcalEvent
function. Additionally, there are relevant test cases that include therrule
parameter, ensuring its functionality is verified.
startRetrospective
function handlingrrule
:
createMeetingSeriesTitle
startNewMeetingSeries
createGcalEvent
Test files:
packages/server/__tests__/startRetrospective.test.ts
packages/server/__tests__/processRecurrence.test.ts
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the resolver and related code handle the new `rrule` parameter correctly. # Test: Search for the resolver implementation. Expect: The resolver should handle the `rrule` parameter. rg --type js --type ts $'startRetrospective'Length of output: 1317
Script:
#!/bin/bash # Extract the function body of `startRetrospective` from the resolver file ast-grep --lang typescript --pattern $'const startRetrospective: MutationResolvers[\'startRetrospective\'] = async ($_$$$) => { $$$ }' --json # Check for the usage of the `rrule` parameter within the `startRetrospective` function rg --type js --type ts 'rrule' packages/server/graphql/public/mutations/startRetrospective.ts # Verify the presence of tests that include the `rrule` parameter rg --type js --type ts 'rrule' packages/server/__tests__/Length of output: 1823
Script:
#!/bin/bash # Correct the `ast-grep` pattern to accurately extract the `startRetrospective` function body ast-grep --lang typescript --pattern $'const startRetrospective: MutationResolvers[\'startRetrospective\'] = async ($$$) => { $$$ }' packages/server/graphql/public/mutations/startRetrospective.ts --jsonLength of output: 8761
packages/server/graphql/public/typeDefs/startTeamPrompt.graphql (2)
15-17
: Ensure the new parameterrrule
is handled correctly.The
rrule
parameter has been added to thestartTeamPrompt
mutation. Ensure that the corresponding resolver and related code handle this parameter correctly.Verification successful
Verified: The
rrule
parameter is handled correctly in thestartTeamPrompt
resolver.The
rrule
parameter is extracted and used appropriately within the resolver function, ensuring it is processed as expected.
startTeamPrompt
resolver inpackages/server/graphql/public/mutations/startTeamPrompt.ts
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the resolver and related code handle the new `rrule` parameter correctly. # Test: Search for the resolver implementation. Expect: The resolver should handle the `rrule` parameter. rg --type js --type ts $'startTeamPrompt'Length of output: 600
Script:
#!/bin/bash # Description: Verify the `rrule` parameter is handled in the `startTeamPrompt` resolver. # Extract the function body of the `startTeamPrompt` resolver to inspect the handling of `rrule` ast-grep --lang typescript --pattern $'const startTeamPrompt: MutationResolvers[\'startTeamPrompt\'] = async ($_args) => { $$$ }' --jsonLength of output: 146
Script:
#!/bin/bash # Description: Extract the content of the `startTeamPrompt` resolver to verify the handling of the `rrule` parameter. # Display the content of the `startTeamPrompt` resolver file cat packages/server/graphql/public/mutations/startTeamPrompt.tsLength of output: 3364
11-13
: Ensure the new parametername
is handled correctly.The
name
parameter has been added to thestartTeamPrompt
mutation. Ensure that the corresponding resolver and related code handle this parameter correctly.Verification successful
The
name
parameter is handled correctly in thestartTeamPrompt
resolver.The resolver implementation includes the
name
parameter in its function signature and appropriately uses it within the function body.
packages/server/graphql/public/mutations/startTeamPrompt.ts
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the resolver and related code handle the new `name` parameter correctly. # Test: Search for the resolver implementation. Expect: The resolver should handle the `name` parameter. rg --type js --type ts $'startTeamPrompt'Length of output: 600
Script:
#!/bin/bash # Description: Verify the handling of the `name` parameter in the `startTeamPrompt` resolver implementation. # Extract the resolver implementation for `startTeamPrompt` ast-grep --lang typescript --pattern $'const startTeamPrompt: MutationResolvers[\'startTeamPrompt\'] = async ($_)' --jsonLength of output: 129
Script:
#!/bin/bash # Description: Extract the `startTeamPrompt` resolver implementation to verify the handling of the `name` parameter. # Extract lines around the definition of the `startTeamPrompt` resolver rg --context 10 'const startTeamPrompt: MutationResolvers' packages/server/graphql/public/mutations/startTeamPrompt.tsLength of output: 1028
Script:
#!/bin/bash # Description: Extract more lines from the `startTeamPrompt` resolver implementation to verify the handling of the `name` parameter. # Extract additional lines from the `startTeamPrompt` resolver implementation rg --context 50 'const startTeamPrompt: MutationResolvers' packages/server/graphql/public/mutations/startTeamPrompt.tsLength of output: 2956
packages/server/database/types/MeetingAction.ts (2)
35-35
: Ensure thename
field is correctly passed to the superclass.The
name
field has been added to the constructor of theMeetingAction
class and is passed to the superclass. Ensure that the superclass handles thename
field correctly.Verification successful
The superclass
Meeting
handles thename
field correctly.
- The
Meeting
class has a constructor that assigns thename
field using the statementthis.name = name ?? ...
.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the superclass handles the `name` field correctly. # Test: Search for the superclass implementation. Expect: The superclass should handle the `name` field. rg --type ts $'class Meeting'Length of output: 1378
Script:
#!/bin/bash # Description: Extract the constructor of the `Meeting` class to verify if it handles the `name` field correctly. # Extract the constructor of the `Meeting` class from the specified file. ast-grep --lang typescript --pattern $'class Meeting { constructor($_) { $$$ } }' packages/server/database/types/Meeting.tsLength of output: 132
Script:
#!/bin/bash # Description: Verify if the `Meeting` class handles the `name` field correctly. # Search for the constructor and the `name` field in the `Meeting.ts` file. rg 'constructor' packages/server/database/types/Meeting.ts rg 'name' packages/server/database/types/Meeting.tsLength of output: 280
12-12
: Ensure thename
field is correctly handled.The
name
field has been made required in theInput
interface. Ensure that all instances whereInput
is used handle thename
field correctly.Verification successful
The
name
field is correctly handled.The
name
field in theInput
interface forMeetingAction
is properly managed across various parts of the codebase. The field is consistently passed, assigned, and utilized, ensuring its correct handling.
packages/server/graphql/public/mutations/startCheckIn.ts
packages/server/graphql/mutations/helpers/notifications/MSTeamsNotifier.ts
packages/server/database/types/MeetingAction.ts
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all instances where `Input` is used handle the `name` field correctly. # Test: Search for instances of `Input`. Expect: All instances should handle the `name` field. rg --type ts $'Input'Length of output: 116872
Script:
#!/bin/bash # Description: Verify the handling of the `name` field in the `Input` interface specific to `MeetingAction`. # Step 1: Find the definition of the `Input` interface in `MeetingAction.ts`. rg --type ts 'interface Input' -g '**/MeetingAction.ts' # Step 2: Search for usage of the `Input` interface from `MeetingAction.ts`. rg --type ts 'MeetingAction.Input'Length of output: 158
Script:
#!/bin/bash # Description: Verify the handling of the `name` field in the `Input` interface specific to `MeetingAction`. # Step 1: Search for the usage of the `Input` interface within the `MeetingAction.ts` file. rg --type ts 'Input' packages/server/database/types/MeetingAction.ts # Step 2: Find files that import `MeetingAction` to check for usage of the `Input` interface. rg --type ts 'MeetingAction' -lLength of output: 866
Script:
#!/bin/bash # Description: Verify the handling of the `name` field in the `Input` interface specific to `MeetingAction`. # Step 1: Search for the usage of `MeetingAction` in the identified files. rg --type ts 'MeetingAction' packages/server/postgres/types/Meeting.d.ts packages/client/components/NewMeetingActionsCurrentMeetings.tsx packages/client/components/NewMeetingActions.tsx packages/client/components/ActivityLibrary/ActivityDetailsSidebar.tsx packages/server/graphql/public/types/StartCheckInSuccess.ts packages/server/graphql/meetingTypePredicates.ts packages/server/graphql/mutations/helpers/notifications/getSummaryText.ts packages/server/graphql/mutations/helpers/notifications/MSTeamsNotifier.ts packages/server/graphql/mutations/helpers/addAgendaItemToActiveActionMeeting.ts packages/server/graphql/public/mutations/startCheckIn.ts packages/server/graphql/mutations/endCheckIn.ts packages/server/database/types/MeetingAction.ts # Step 2: Verify how the `name` field is handled in the usage of `MeetingAction`. rg --type ts 'name' packages/server/postgres/types/Meeting.d.ts packages/client/components/NewMeetingActionsCurrentMeetings.tsx packages/client/components/NewMeetingActions.tsx packages/client/components/ActivityLibrary/ActivityDetailsSidebar.tsx packages/server/graphql/public/types/StartCheckInSuccess.ts packages/server/graphql/meetingTypePredicates.ts packages/server/graphql/mutations/helpers/notifications/getSummaryText.ts packages/server/graphql/mutations/helpers/notifications/MSTeamsNotifier.ts packages/server/graphql/mutations/helpers/addAgendaItemToActiveActionMeeting.ts packages/server/graphql/public/mutations/startCheckIn.ts packages/server/graphql/mutations/endCheckIn.ts packages/server/database/types/MeetingAction.tsLength of output: 11481
packages/server/graphql/public/typeDefs/updateRecurrenceSettings.graphql (2)
15-18
: LGTM! The addition ofname
parameter is correct.The description and type for the
name
parameter are appropriate.
20-22
: LGTM! The addition ofrrule
parameter is correct.The description and type for the
rrule
parameter are appropriate.packages/server/database/types/MeetingPoker.ts (2)
11-11
: LGTM! Thename
field is now required.This change ensures that the meeting name is always provided, aligning with the PR objectives.
38-38
: LGTM! Thename
parameter is correctly included in the constructor.This change ensures that the meeting name is always provided and assigned to the superclass.
packages/client/mutations/UpdateRecurrenceSettingsMutation.ts (1)
24-25
: LGTM! Thename
andrrule
parameters are correctly included in the mutation.The parameters are correctly defined and included in the GraphQL query, aligning with the PR objectives.
packages/server/graphql/mutations/helpers/safeCreateRetrospective.ts (2)
50-50
: LGTM!The changes look good to me.
27-27
: Verify data loader keymeetingCount
.Ensure that the key
meetingCount
is correctly defined in the data loader.Verification successful
Data loader key
meetingCount
is correctly defined.The
meetingCount
key is defined in the data loader within thepackages/server/dataloader/customLoaderMakers.ts
file. The implementation usesDataLoader
to fetch the count of meetings byteamId
andmeetingType
, ensuring that the key is correctly set up for the intended functionality.
- File:
packages/server/dataloader/customLoaderMakers.ts
- Definition:
export const meetingCount = (parent: RootDataLoader) => { return new DataLoader<{teamId: string; meetingType: MeetingTypeEnum}, number, string>( async (keys) => { const r = await getRethink() const res = await Promise.all( keys.map(async ({teamId, meetingType}) => { return r .table('NewMeeting') .getAll(teamId, {index: 'teamId'}) .filter({meetingType: meetingType as any}) .count() .default(0) .run() }) ) return res }, { ...parent.dataLoaderOptions, cacheKeyFn: (key) => `${key.teamId}:${key.meetingType}` } ) }Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the presence of `meetingCount` key in the data loader. # Test: Search for the definition of `meetingCount` in the codebase. Expect: Key is correctly defined. rg --type js --type ts $'meetingCount'Length of output: 6162
Script:
#!/bin/bash # Display the content of the customLoaderMakers.ts file to verify the definition of `meetingCount`. # Find and display the content of `customLoaderMakers.ts` fd customLoaderMakers.ts --exec catLength of output: 29524
packages/client/mutations/StartTeamPromptMutation.ts (2)
26-26
: LGTM!The changes look good to me.
22-23
: Ensurename
andrrule
are correctly passed.The mutation now requires
name
andrrule
parameters. Verify that all calls toStartTeamPromptMutation
provide these parameters.Verification successful
All calls to
StartTeamPromptMutation
correctly provide thename
andrrule
parameters.
- In the file
packages/client/components/ActivityLibrary/ActivityDetailsSidebar.tsx
, theStartTeamPromptMutation
invocation includesname
andrrule
.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all calls to `StartTeamPromptMutation` provide the `name` and `rrule` parameters. # Test: Search for the function usage. Expect: All calls include the `name` and `rrule` parameters. rg --type js --type ts $'StartTeamPromptMutation'Length of output: 1942
Script:
#!/bin/bash # Description: Verify that all calls to `StartTeamPromptMutation` provide the `name` and `rrule` parameters. # Extract lines where StartTeamPromptMutation is invoked and check for parameters rg --type js --type ts 'StartTeamPromptMutation\(' -A 3Length of output: 737
Script:
#!/bin/bash # Description: Verify that all calls to `StartTeamPromptMutation` provide the `name` and `rrule` parameters. # Extract more lines around the StartTeamPromptMutation invocation in ActivityDetailsSidebar.tsx rg --type ts 'StartTeamPromptMutation\(' -A 10 -B 2 packages/client/components/ActivityLibrary/ActivityDetailsSidebar.tsxLength of output: 440
packages/server/__tests__/startRetrospective.test.ts (3)
37-37
: LGTM!The changes look good to me.
52-53
: Ensure test coverage forname
andrrule
.The mutation now requires
name
andrrule
parameters. Verify that the test cases cover these parameters adequately.
70-71
: LGTM!The changes look good to me.
packages/client/components/ActivityLibrary/ScheduleMeetingButton.tsx (2)
63-64
: LGTM!The changes look good to me.
4-4
: ImportRRule
fromrrule
.Ensure that the
RRule
import from therrule
library is correctly used.Verification successful
Import
RRule
fromrrule
.The import and usage of
RRule
from therrule
library in theScheduleMeetingButton.tsx
file are correct.
- The import statement is correctly placed.
- The
RRule
type is used appropriately in thehandleStartActivity
andonStartActivity
functions.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the usage of `RRule` from the `rrule` library. # Test: Search for the usage of `RRule`. Expect: Correct usage of `RRule`. rg --type js --type ts $'RRule'Length of output: 15097
Script:
#!/bin/bash # Extract the contents of the ScheduleMeetingButton.tsx file to verify the usage of RRule cat packages/client/components/ActivityLibrary/ScheduleMeetingButton.tsxLength of output: 2671
packages/server/graphql/public/mutations/startTeamPrompt.ts (4)
21-21
: Ensurename
andrrule
are validated.While the changes are correct, consider validating
name
andrrule
to ensure they meet expected formats.
49-50
: Consider handling missingname
andrrule
.The current logic defaults
name
to 'Standup' ifrrule
is present butname
is not. Ensure this behavior is intended and documented.
64-65
: EnsurestartNewMeetingSeries
handlesname
andrrule
correctly.Verify that
startNewMeetingSeries
correctly processes thename
andrrule
parameters.
72-79
: VerifycreateGcalEvent
handles new parameters.Ensure that
createGcalEvent
correctly processes thename
andgcalInput
parameters.packages/server/graphql/public/mutations/startCheckIn.ts (3)
20-20
: Ensurename
is validated.While the changes are correct, consider validating
name
to ensure it meets expected formats.
62-62
: Ensure defaultname
logic is intended.The current logic defaults
name
toCheck-in #${meetingCount + 1}
ifname
is not provided. Ensure this behavior is intended and documented.
96-103
: VerifycreateGcalEvent
handles new parameters.Ensure that
createGcalEvent
correctly processes thename
andgcalInput
parameters.packages/client/components/Recurrence/EndRecurringMeetingModal.tsx (1)
89-89
: Ensurename
andrrule
are validated.While the changes are correct, consider validating
name
andrrule
to ensure they meet expected formats.packages/server/graphql/public/mutations/startRetrospective.ts (4)
23-23
: Ensurename
andrrule
are validated.While the changes are correct, consider validating
name
andrrule
to ensure they meet expected formats.
57-61
: Consider handling missingname
andrrule
.The current logic sets a default
name
ifname
is not provided and handlesrrule
accordingly. Ensure this behavior is intended and documented.
100-100
: EnsurestartNewMeetingSeries
handlesname
andrrule
correctly.Verify that
startNewMeetingSeries
correctly processes thename
andrrule
parameters.
126-131
: VerifycreateGcalEvent
handles new parameters.Ensure that
createGcalEvent
correctly processes thename
,gcalInput
, andrrule
parameters.packages/server/graphql/mutations/helpers/createGcalEvent.ts (7)
27-27
: Review: Addedname
field to Input type.The
name
field is now required for creating Google Calendar events. Ensure this field is always provided when calling this function.
39-39
: Review: Destructuringname
from input.The function now correctly destructures
name
from the input object.
44-44
: Review: Usingname
in event input.The event summary is now set to the
name
field, ensuring the event has a meaningful title.
119-119
: Review: Addedname
field to UpdateGcalSeriesInput type.The
name
field is now optional for updating Google Calendar series events. Ensure this field is correctly managed when updating events.
126-126
: Review: Destructuringname
from input in updateGcalSeries.The function now correctly destructures
name
from the input object for updating Google Calendar series events.
Line range hint
163-163
:
Review: Updating Google Calendar series with newname
andrrule
.The update function now correctly uses the new
name
andrrule
values for updating the event series.
Line range hint
166-167
:
Review: Passingname
andrrule
to updateGcalSeries.The function now correctly passes the
name
andrrule
to theupdateGcalSeries
function.packages/server/graphql/public/mutations/updateRecurrenceSettings.ts (7)
127-127
: Review: UpdatedupdateRecurrenceSettings
mutation signature.The mutation now accepts
name
andrrule
parameters directly, ensuring consistency with the new approach.
155-155
: Review: Handling case whenrrule
is not provided.The function now stops the meeting series if
rrule
is not provided.
159-159
: Review: Updating meeting series with newrrule
.The function now updates the meeting series with the new
rrule
when provided.
163-167
: Review: Updating Google Calendar series with newname
andrrule
.The mutation now correctly updates the Google Calendar series with the new
name
andrrule
.
174-175
: Review: Updating meeting series title with newname
.The mutation now updates the meeting series title with the new
name
when provided.
180-180
: Review: Returning error whenrrule
is not provided for non-recurring meetings.The mutation now correctly returns an error if
rrule
is not provided for non-recurring meetings.
187-187
: Review: Starting new meeting series withname
andrrule
.The mutation now starts a new meeting series with the provided
name
andrrule
.packages/client/components/Recurrence/UpdateRecurrenceSettingsModal.tsx (2)
178-179
: Review: Passingname
andrrule
to UpdateRecurrenceSettingsMutation.The component now correctly passes the
name
andrrule
to theUpdateRecurrenceSettingsMutation
.
191-191
: Review: Stopping recurrence by passingnull
forrrule
.The component now correctly stops the recurrence by passing
null
forrrule
.packages/server/graphql/mutations/startSprintPoker.ts (5)
1-1
: Review: Addedname
argument to startSprintPoker mutation.The mutation now accepts a
name
argument for naming the sprint poker meeting.
77-80
: Review: Addedname
argument to mutation arguments.The mutation arguments now include
name
, allowing the client to specify the meeting name.
88-92
: Review: Destructuringname
from mutation arguments.The mutation now correctly destructures
name
from the arguments.
139-139
: Review: Usingname
for meeting name.The meeting name is now set to the provided
name
or a default value if not provided.
187-194
: Review: Passingname
to createGcalEvent.The mutation now correctly passes
name
to thecreateGcalEvent
function, ensuring the Google Calendar event is named appropriately.packages/client/components/ScheduleDialog.tsx (2)
34-34
: Ensure correct usage of updatedonStartActivity
prop.The
onStartActivity
prop now acceptsname
andrrule
parameters. Verify that all calls to this prop in the codebase are updated accordingly.Verification successful
Verified
onStartActivity
prop usage.The
onStartActivity
prop has been updated to acceptname
andrrule
parameters, and all relevant calls in the codebase have been updated accordingly.
packages/client/components/ScheduleDialog.tsx
packages/client/components/ActivityLibrary/ScheduleMeetingButton.tsx
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all calls to `onStartActivity` prop are updated to use `name` and `rrule`. # Test: Search for the prop usage. Expect: Only occurances of the new signature. rg --type tsx $'onStartActivity'Length of output: 59
Script:
#!/bin/bash # Description: Verify all calls to `onStartActivity` prop are updated to use `name` and `rrule`. # Test: Search for the prop usage. Expect: Only occurrences of the new signature. rg 'onStartActivity' --glob '**/*.tsx'Length of output: 570
201-205
: Ensure correct usage of updatedRecurrenceSettings
component.The
RecurrenceSettings
component now usestitle
andrrule
parameters. Verify that all instances of this component in the codebase are updated accordingly.packages/client/components/ActivityLibrary/ActivityDetailsSidebar.tsx (1)
125-126
: LGTM! But ensure the function calls are updated.The code changes are approved.
However, ensure that all function calls to
StartTeamPromptMutation
andStartRetrospectiveMutation
match the new signature.Also applies to: 149-150
Verification successful
Function calls match the new signature.
The function calls to
StartTeamPromptMutation
andStartRetrospectiveMutation
inActivityDetailsSidebar.tsx
have been verified and they match the new signature.
packages/client/components/ActivityLibrary/ActivityDetailsSidebar.tsx
StartTeamPromptMutation(atmosphere, { teamId: selectedTeam.id, name, rrule: rrule?.toString(), })
StartRetrospectiveMutation(atmosphere, { teamId: selectedTeam.id, name, rrule: rrule?.toString(), })
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all function calls to `StartTeamPromptMutation` and `StartRetrospectiveMutation` match the new signature. # Test: Search for the function usage. Expect: Only occurances of the new signature. rg --type ts $'StartTeamPromptMutation' rg --type ts $'StartRetrospectiveMutation'Length of output: 4377
Script:
#!/bin/bash # Extract function definitions and usages for StartTeamPromptMutation and StartRetrospectiveMutation in relevant files # Extract function definitions and usages for StartTeamPromptMutation echo "StartTeamPromptMutation Definitions and Usages:" rg 'StartTeamPromptMutation' packages/client/components/ActivityLibrary/ActivityDetailsSidebar.tsx rg 'StartTeamPromptMutation' packages/client/mutations/StartTeamPromptMutation.ts # Extract function definitions and usages for StartRetrospectiveMutation echo "StartRetrospectiveMutation Definitions and Usages:" rg 'StartRetrospectiveMutation' packages/client/components/ActivityLibrary/ActivityDetailsSidebar.tsx rg 'StartRetrospectiveMutation' packages/client/mutations/StartRetrospectiveMutation.tsLength of output: 1882
Script:
#!/bin/bash # Extract function call details for StartTeamPromptMutation and StartRetrospectiveMutation in ActivityDetailsSidebar.tsx # Extract function call details for StartTeamPromptMutation echo "StartTeamPromptMutation Call Details:" rg -A 5 'StartTeamPromptMutation\(' packages/client/components/ActivityLibrary/ActivityDetailsSidebar.tsx # Extract function call details for StartRetrospectiveMutation echo "StartRetrospectiveMutation Call Details:" rg -A 5 'StartRetrospectiveMutation\(' packages/client/components/ActivityLibrary/ActivityDetailsSidebar.tsxLength of output: 741
packages/server/graphql/public/typeDefs/_legacy.graphql (1)
5854-5856
: Ensure the newname
field is correctly integrated.The new
name
field in theStartSprintPoker
mutation should be consistent with the rest of the schema and correctly integrated into the resolver logic.Verification successful
The new
name
field is correctly integrated.The
name
field has been added to theStartSprintPoker
mutation's arguments and is used appropriately within the resolver logic and payload structures.
- The field is added to the arguments in
startSprintPoker.ts
.- The field is used in the
createGcalEvent
function call.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify that the new `name` field is integrated into the resolver logic for `StartSprintPoker`. # Test: Search for the usage and resolver implementation of `StartSprintPoker`. rg --type graphql --pattern 'StartSprintPoker'Length of output: 458
Script:
#!/bin/bash # Description: Verify that the new `name` field is integrated into the resolver logic for `StartSprintPoker`. # Test: Search for the usage and resolver implementation of `StartSprintPoker`. rg 'StartSprintPoker'Length of output: 2881
Script:
#!/bin/bash # Description: Extract and review the resolver logic for `StartSprintPoker` and its payload structures. # Extract the resolver logic for `StartSprintPoker`. echo "Resolver logic in startSprintPoker.ts:" rg --context 10 'StartSprintPoker' packages/server/graphql/mutations/startSprintPoker.ts # Extract the payload structures for `StartSprintPoker`. echo "Payload structures in StartSprintPokerPayload.ts:" rg --context 10 'StartSprintPoker' packages/server/graphql/types/StartSprintPokerPayload.tsLength of output: 3746
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Outside diff range, codebase verification and nitpick comments (1)
packages/server/graphql/mutations/helpers/safeCreateRetrospective.ts (1)
18-18
: Missingname
parameter in calls tosafeCreateRetrospective
The
name
parameter is required insafeCreateRetrospective
. However, it is missing in the following calls:
packages/server/graphql/public/mutations/startRetrospective.ts
packages/server/graphql/private/mutations/processRecurrence.ts
Please ensure that the
name
parameter is provided in these function calls.Analysis chain
Ensure
name
is provided in all calls tosafeCreateRetrospective
.The
name
parameter is now required inmeetingSettings
. Verify that all calls tosafeCreateRetrospective
provide this parameter.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all calls to `safeCreateRetrospective` provide the `name` parameter. # Test: Search for the function usage. Expect: All calls include the `name` parameter. rg --type js --type ts $'safeCreateRetrospective'Length of output: 791
Script:
#!/bin/bash # Description: Check if the `name` parameter is provided in all calls to `safeCreateRetrospective`. # Extract and display the lines where `safeCreateRetrospective` is called in the identified files rg --type js --type ts -A 5 $'safeCreateRetrospective(' packages/server/graphql/private/mutations/processRecurrence.ts packages/server/graphql/public/mutations/startRetrospective.tsLength of output: 281
Script:
#!/bin/bash # Description: Check if the `name` parameter is provided in all calls to `safeCreateRetrospective`. # Extract and display the lines where `safeCreateRetrospective` is called in the identified files rg --type js --type ts -A 5 'safeCreateRetrospective\(' packages/server/graphql/private/mutations/processRecurrence.ts packages/server/graphql/public/mutations/startRetrospective.tsLength of output: 1263
Fixes #9928
Make the meeting name a separate parameter so it can be used for the meeting, meeting series and gcal event. This makes everything more consistent.
Demo
https://www.loom.com/share/63b49765c63b4575a59838ab4fc8a402?sid=5fd46150-274e-4b70-baad-08d12521516a
Testing scenarios
[Please list all the testing scenarios a reviewer has to check before approving the PR]
Scenario A
Scenario B
Final checklist
Summary by CodeRabbit
New Features
name
andrrule
parameters for scheduling and starting activities.Refactor
RecurrenceSettingsInput
withname
andrrule
parameters across various components and mutations.Bug Fixes