User Choice and Conflict Resolution #795
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
* Rework modal to support CourseTaken type * Write function to convert course types to roster * Use courseID instead of uniqueId * Open modal from warning with correct data sent * Select pre-selected reqs when editing * Preserve unselected reqs in modal * Make selectable req order not matter, and refactor name * Fix bug where editing conflicts does not change fulfillment * Type fixes * Fix course name when editing conflicts * Fix icon styling bug on req bar * Fix modal not opening from course * Fix bug where fulfilled conflicts show up in sem plan * Toggle requirements only when modal saved Co-authored-by: Benjamin Shen <[email protected]>
…odel (#689) * optin optout functions * clean up * minor changes * flesh out functions * refactor selectable * deprecate getRelatedRequirementIdsForCourseOptOut * reorder
…nto user-choice
…716) * rename override-fulfillment-choices * fix overridden fulfillments race condition * remove toggle requirement choices function * address comments in course conflict modal
* Changed delete button to gear * Gear button * Button now turns blue when hovered over * Settings button opens the replace/remove modal - still need to change the icons to the correct ones * Minor changes to the remove/replace icons * correct icons for edit menu * Initial implementation of ReplaceCourseModal * Turned the DeleteCourseModal into removes * SlotMenu appears for incomplete subreqs * Add ReplaceSlotMenu * Replace Modal now opens * Replace slot menu completed * Added warning text * Added slot menu for adding new classes * Correct placeholder names * Ran format * Fixed delete modal * Added methods * fixing type errors * Changes to address spring comments * Address comments from fall * Revert "Address comments from fall" This reverts commit 5e4954f. * Add replacement editor * Fix bold text in delete course modal * Fix reqdesc * Run prettier * Fix yearRange export * Fix Cypress tests - changed SelectedRequirementEditor back - now need to fix ReplaceRequirementEditor to match design * Made ReplaceCourseModal match designs * Run prettier * Update ReplaceRequirementEditor.vue * Check if selected course is in schedule * 0 courses case covered * back button now shows search bar again * 2+ courses case handled * 1 course case handled * Fix dropdown menu length for duplicates * Fix arrow on drop down menu * Run prettier * Dropdown menu provides selected semesters * Run prettier * Clean up SelectSemesterDuplicate and fix bug bug involving searching for course multiple times will add the semesters again * Fix selection text * Add documentation * Add docstrings * Cleanup css * Changes to PR comments * Finish PR comments * Add feature flag names * Add cypress test - Add 2110 works * Fix cypress tests * Address review * Remove unused SelectSemesterReplace * change placeholder text to computed * Remove courseSelectorKey * Fix Placeholder Text * Fix requirement checker warning * Update ReplaceCourseModal.vue * Update ReplaceCourseModal.vue * Update ReplaceCourseModal.vue
* add transformers, refactor graph builder algorithm * small fixes, rework graph builder from user data * rename transformer to processor * turn initial graph building step into first pipeline step * clean up imports/exports * fix bad import * compute constraint violations from dangerous graph instead of safe graph * small improvements, fix dangerous graph bug * comments * break down add basic user fulfillment choices into multiple steps * address review comments * further abstract processor * rename processor to visitor, small fixes * change copy-graph visitor to be default class
* add readonly graph definition * format * lint * type check
Co-authored-by: zachary0kent <[email protected]>
noschiff
requested review from
benjamin-shen,
rohanmaheshwari430 and
zachary-kent
|
[diff-counting] Significant lines: 2477. This diff might be too big! Developer leads are invited to review the code. |
|
Visit the preview URL for this PR (updated for commit 52d2494): https://cornelldti-courseplan-dev--pr795-user-choice-kjte2db6.web.app (expires Thu, 06 Apr 2023 18:40:17 GMT) 🔥 via Firebase Hosting GitHub Action 🌎 Sign: 6d4e0437c4559ed895272bbd63991394f1e0e933 |
noschiff
changed the title
* Removed conflicts feature flag and its references * Only close add course modal on 'Add' click
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
FINALLY! Over two years in the making! This PR implements opt in/out, allowing the user to resolve conflicts when one course can fulfill one of multiple requirements. Thanks to all the hard work by @SamChou19815, @willespencer, @einc, @benjamin-shen, and @zachary0kent to make this happen! This PR also includes code for the remove and replace modal, but it's currently behind a feature flag.
Quick Background
We have an OPT OUT system that allows users to force an opt in.
Conflict resolution appears when we have (c,R) violation (one course to many potential reqs). The user opts out of requirements (removing edges in course-req graph) until it’s legal. We then store these choices inside “optOut”. We do not update arbitrary opt in through this. The underlying idea is, when there is a (c,R) violation, the user opts out of edges until the resulting graph is legal.
Users can force edges (force mapping from course to req) by arbitrary opt in to a requirement. Currently, this is done by the replace modal. Until we have replace modal,
arbitraryOptInis useless.Required Reading
Remaining TODOs:
Test Plan
Read required reading. Add a course that could count towards multiple requirements, creating a conflict. Then, manually resolve them using the "FIX NOW" prompt and ensure that your changes persist.