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

New admin schedule #61

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

New admin schedule #61

wants to merge 9 commits into from

Conversation

rryanhan
Copy link
Collaborator

@rryanhan rryanhan commented Feb 19, 2025

DRAFT PR - NOT COMPLETE

Changes Made (Previous)

  • For now I've chosen to keep filtering based on shift_type but use shift_absence_request.status for both admin and volunteer accounts to determine the shift type to minimize changes made in code
Screenshot 2025-03-08 at 4 55 17 PM
  • Unsure of how to handle logic when there is more than one volunteer assigned to a shift – initial idea was to filter using class_id, start_time, end_time and display one class, but what if there is an instance where more than one volunteer for a class has requested a coverage request?
  • Made some changes to the getShifts() query – shifts to sh, date to shift_date
  • Admin and Volunteer schedules are under /pages/Schedule. isAdmin is used to determine the filter and display logic
  • getButtonConfig handles display and filter logic for Admin UI
Screenshot 2025-03-08 at 4 45 05 PM

case statements made into one, used getShifts updated PR

  • Created test classes for Needs Coverage, Request for Shift Coverage, Request to Fulfill Coverage (Called Ryan's Test Class!!!)
  • For classes that have more than one volunteer assigned, the shift has been "clumped" into one:
Screenshot 2025-03-10 at 1 52 11 AM - Added a small text indicating how many volunteers are assigned to the shift Screenshot 2025-03-10 at 1 56 36 AM - Volunteer shift display should work the same as it has, I've essentially mapped the shifts for admin using`${shift.class_id}-${shiftDay}-${shift.start_time}` as the key to ensure that the classes are the same

@rryanhan rryanhan requested a review from jjessieshang March 1, 2025 07:40
@theosiemensrhodes
Copy link
Contributor

Really good start, Im getting some errors currently but I think that is due to Jan's changes to moving instructor from a class field to schedule field. I think after he merges his changes in soon you can rebase to fix that.

As for the issue of duplicate shifts for each volunteer, I think the best way to do this would be to combine the shifts based on schedule id. So if you had

...
{
   shift_id: 10,
   schedule_id: 12,
   volunteer_id: "some volunteer"
   ...
},
{
   shift_id: 11,
   schedule_id: 12,
   volunteer_id: "other volunteer"
   ...
}
...

You could combine on schedule id to get

{
   schedule_id: 12,
   ...other fields dependent on schedule (i.e. class fields etc...)
   [
       {
           shift_id: 10,
           volunteer_id: "some volunteer"
           ...fields dependent on shift
       },
       {
           shift_id: 11,
           volunteer_id: "other volunteer"
           ...fields dependent on shift
       }
   ]
}

For now we should combine on the frontend I think.

@jjessieshang
Copy link
Collaborator

Good work so far. I agree with Theo that on the frontend before displaying the list of shifts we can group by shift.fk_schedule_id to remove shift duplicates . Your logic for toggling between shift statuses also makes sense to me, but when logged in as a volunteer I wasn't able to see open coverage requests, aka the shifts with status SHIFT_TYPES.COVERAGE.

@jjessieshang
Copy link
Collaborator

jjessieshang commented Mar 9, 2025

Really good start, Im getting some errors currently but I think that is due to Jan's changes to moving instructor from a class field to schedule field. I think after he merges his changes in soon you can rebase to fix that.

As for the issue of duplicate shifts for each volunteer, I think the best way to do this would be to combine the shifts based on schedule id. So if you had

...
{
   shift_id: 10,
   schedule_id: 12,
   volunteer_id: "some volunteer"
   ...
},
{
   shift_id: 11,
   schedule_id: 12,
   volunteer_id: "other volunteer"
   ...
}
...

You could combine on schedule id to get

{
   schedule_id: 12,
   ...other fields dependent on schedule (i.e. class fields etc...)
   [
       {
           shift_id: 10,
           volunteer_id: "some volunteer"
           ...fields dependent on shift
       },
       {
           shift_id: 11,
           volunteer_id: "other volunteer"
           ...fields dependent on shift
       }
   ]
}

For now we should combine on the frontend I think.

It seems like the DetailsPanel is not rendering properly due to an undefined property "schedules" it's trying to read. I'm thinking this is due to some difference in the values returned upon switching to the new shifts api.

Update: nvm seeing the bad field instructors error now...

@rryanhan
Copy link
Collaborator Author

I believe that the Details panel just isn't working rn, I've made changes based on the reviews!!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants