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

gql: add field "assignedSchedules" to type "User" #3231

Merged
merged 22 commits into from
Oct 27, 2023
Merged

Conversation

Forfold
Copy link
Contributor

@Forfold Forfold commented Aug 16, 2023

Description:
Adds the field assignedSchedules to type User so that each schedule's shifts can be fetched for a given user. This will be especially important for adding a calendar to a user's profile, so they know the context of each shift they are on-call for.

Which issue(s) this PR fixes:
This PR is precedent to #3229 and is a part of #3164

Screenshots:

Screenshot 2023-10-11 at 11 47 02 AM

Other changes:
Also adds the argument user on query shifts to use for filtering:

type Schedule {
  // ...
  shifts(start: ISOTimestamp!, end: ISOTimestamp!, userIDs: [ID!]): [OnCallShift!]!
  // ...
}

@Forfold Forfold marked this pull request as ready for review August 16, 2023 16:02
oncall/state.go Outdated Show resolved Hide resolved
allending313
allending313 previously approved these changes Aug 16, 2023
Copy link
Contributor

@allending313 allending313 left a comment

Choose a reason for hiding this comment

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

lgtm!

@mastercactapus
Copy link
Member

I propose we expand the existing shifts field on Schedule by adding a userIDs argument that will filter the returned shifts to the provided list.

Secondly, an assignedSchedules field on User that will return the list of schedules the user is associated with (i.e., assigned directly, on a future add_override /w end_time in the future, or participant of a rotation assigned to the schedule)

type Schedule {
  // ...
  shifts(start: ISOTimestamp!, end: ISOTimestamp!, userIDs: [ID!]): [OnCallShift!]!
  // ...
}

type User {
  // ...
  assignedSchedules: [Schedule!]
  // ...
} 

@Forfold Forfold dismissed stale reviews from allending313 and andrewbenington via b21f911 August 23, 2023 17:37
@github-actions github-actions bot added size/m and removed size/s labels Aug 23, 2023
@Forfold
Copy link
Contributor Author

Forfold commented Aug 24, 2023

When attempting to find any temporary schedules, the application gets stuck in an infinite timeout. No idea why. It happens on this line:

err := s.grp.Get(ctx, timeKey("userIDs", time.Minute), groupcache.AllocatingByteSliceSink(&data))

@Forfold
Copy link
Contributor Author

Forfold commented Oct 2, 2023

Blocked by #3286

@github-actions github-actions bot added size/l and removed size/m labels Oct 10, 2023
@github-actions github-actions bot added size/m and removed size/l labels Oct 10, 2023
oncall/store.go Outdated Show resolved Hide resolved
schedule/rotation/store.go Outdated Show resolved Hide resolved
graphql2/graphqlapp/schedule.go Outdated Show resolved Hide resolved
graphql2/schema.graphql Outdated Show resolved Hide resolved
schedule/rotation/store.go Outdated Show resolved Hide resolved
schedule/store.go Outdated Show resolved Hide resolved
@Forfold Forfold changed the title gql: add field "target" to type "OnCallShift" gql: add field "assignedSchedules" to type "User" Oct 11, 2023
@github-actions github-actions bot added size/l and removed size/m labels Oct 11, 2023
@github-actions github-actions bot added size/m and removed size/l labels Oct 11, 2023
oncall/state.go Outdated Show resolved Hide resolved
oncall/store.go Outdated Show resolved Hide resolved
schedule/queries.sql Outdated Show resolved Hide resolved
schedule/rotation/store.go Outdated Show resolved Hide resolved
schedule/store.go Outdated Show resolved Hide resolved
schedule/store.go Outdated Show resolved Hide resolved
schedule/store.go Outdated Show resolved Hide resolved
schedule/store.go Outdated Show resolved Hide resolved
Copy link
Collaborator

@ethan-haynes ethan-haynes left a comment

Choose a reason for hiding this comment

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

LGTM 👍

return nil, err
}

shifts, err := s.OnCallStore.HistoryBySchedule(ctx, raw.ID, start, end)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Small comment, but should we avoid "magic numbers" like the 50 used here and above in end.After(start.AddDate(0, 0, 50?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Looks like it's an issue with most of our calls to validate.ManyUUID, so I'm going to document that for its own issue, if possible

@Forfold Forfold merged commit 8480382 into master Oct 27, 2023
@Forfold Forfold deleted the oncallshift-target branch October 27, 2023 14:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants