Skip to content

Add slot descriptions, matcher randomness #480

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

Merged
merged 5 commits into from
Mar 14, 2025

Conversation

smartspot2
Copy link
Member

Closes #466.

Matcher slots currently have no special metadata that coordinators can use to indicate special sections with---this information needs to be communicated to mentors separately. Adding a description to slots allows for this metadata to be embedded within the matcher, so mentors can see whether sections have any special tags (ex. NPE, affinity section, etc.) prior to filling availability.

Secondly, this PR also adds some randomness to the matcher algorithm, randomly shuffling the lists of mentors/slots/preferences so that tiebreaks happen approximately uniformly.

@smartspot2 smartspot2 added enhancement New feature or request python Pull requests that update Python code labels Aug 12, 2024
@smartspot2 smartspot2 self-assigned this Aug 12, 2024
Copy link

cypress bot commented Aug 12, 2024

csm_web    Run #417

Run Properties:  status check passed Passed #417  •  git commit 01fa6f4332: Add slot descriptions, matcher randomness (#480)
Project csm_web
Branch Review master
Run status status check passed Passed #417
Run duration 02m 18s
Commit git commit 01fa6f4332: Add slot descriptions, matcher randomness (#480)
Committer Alec Li
View all properties for this run ↗︎

Test results
Tests that failed  Failures 0
Tests that were flaky  Flaky 0
Tests that did not run due to a developer annotating a test with .skip  Pending 0
Tests that did not run due to a failure in a mocha hook  Skipped 0
Tests that passed  Passing 84
View all changes introduced in this branch ↗︎

@smartspot2 smartspot2 force-pushed the feat/matcher-improvements branch from 5afad51 to 9dd5d6a Compare August 13, 2024 06:41
@smartspot2 smartspot2 force-pushed the feat/matcher-improvements branch 2 times, most recently from f9629c9 to 65b3442 Compare September 11, 2024 05:12
@smartspot2 smartspot2 force-pushed the feat/matcher-improvements branch 2 times, most recently from c5df64c to 59a0884 Compare December 6, 2024 09:21
@smartspot2 smartspot2 force-pushed the feat/matcher-improvements branch from 59a0884 to 113ffe5 Compare February 12, 2025 00:13
Copy link
Contributor

@edwardneo edwardneo left a comment

Choose a reason for hiding this comment

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

Some minor things. I'm not sure which should be addressed in this PR or another PR.

  • The precommit check is failing.
  • It would be nice if the mentor side of the matcher had the descriptions on the tiles instead of only visible if you click on the tile -- otherwise, mentors have to click through the tiles to find the description. Or some other visual indicator of a description.
  • The save button on the section edit page is always gray -- I think it should be green.
  • The bulk editing feature on the section edit page only edits capacity and description, not section time. Should it edit section time?

Copy link

@isabellaalps isabellaalps left a comment

Choose a reason for hiding this comment

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

note: this PR should be prioritized over calendar as we need the description changes to be merged in before elaine and i make too much progress on the affinity section pop-up

+1 to edward's comments

@smartspot2
Copy link
Member Author

smartspot2 commented Feb 25, 2025

Newest changes should resolve the aforementioned comments:

  • Description is now displayed on the calendar event
    • I currently have it implemented where the description is displayed in parentheses on the same line as the event time; this is so that in most cases, the description will be visible, even if the event is 30 minutes long. If the schedule time or description text becomes too long, it'll overflow and get cut off, but that's just something we have to deal with regardless.
    • In the mentor view, this does mean that the preference number is more likely to get pushed out of the bounding box of the event, but again we'd have this issue regardless of the description if the event is 30 minutes long.
    • Not sure if there are any opinions on this; a more robust solution would be to have a larger bounding box on hover, though this would take more work, and I'm not sure if it's completely worth it at this point.
  • Save button is now styled with primary-btn
  • Bulk editing has now been fixed to also work with section times (this was probably missed in the initial implementation)

Regarding the pre-commit check, this is a more fundamental issue with the pylint configuration; it seems very weird that it's only failing for the fields in the DateTimeField models (and other similar time-related models), but after an update to pylint-django, it seems to have gone away? I've created #521 for that dependency update.

For now, we can just ignore the pre-commit failures in this PR, and #521 can be merged into master first if we want to maintain the passing CI on master.

@smartspot2 smartspot2 force-pushed the feat/matcher-improvements branch from 662cb4c to cc2569a Compare March 4, 2025 01:50
Copy link

@isabellaalps isabellaalps left a comment

Choose a reason for hiding this comment

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

looks good to me :) ty Alec

@smartspot2 smartspot2 force-pushed the feat/matcher-improvements branch from 4075c2a to 2e2efab Compare March 11, 2025 00:30
Copy link
Contributor

@ElaineShu0312 ElaineShu0312 left a comment

Choose a reason for hiding this comment

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

excellent work alec

Copy link
Contributor

@ElaineShu0312 ElaineShu0312 left a comment

Choose a reason for hiding this comment

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

excellent work alec

@edwardneo edwardneo self-requested a review March 14, 2025 09:03
Copy link
Contributor

@edwardneo edwardneo left a comment

Choose a reason for hiding this comment

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

Ok LGTM! I'll put more QOL feedback on another issue for someone to one day implement...

@smartspot2 smartspot2 merged commit 01fa6f4 into master Mar 14, 2025
17 of 24 checks passed
@smartspot2 smartspot2 deleted the feat/matcher-improvements branch March 14, 2025 09:05
faithdennis pushed a commit that referenced this pull request Mar 31, 2025
* Add matcher slot description, reformat match solver

* Add randomness to matcher assignment generation

* Set default description for matcher util

* Add description to event display, add ability to bulk edit times

* Add scrolling to matcher confirm modal content
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request python Pull requests that update Python code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add randomness to matcher algorithm
4 participants