Schedule Builder Feature #867
Conversation
… options when closing the schedule select modal
|
@kmd2zjw can you resolve merge conflicts? |
|
Ok, I went ahead and merged dev into this branch. Let me know what else you think. |
package-lock.json
Outdated
There was a problem hiding this comment.
Why are there changes to this file? We currently have a finnicky node module setup - are these changes necessary?
There was a problem hiding this comment.
I don't think they are necessary, I don't remember importing any dependencies.
|
|
||
| return scheduled_courses | ||
|
|
||
| def calculate_total_rating(self, rating): |
There was a problem hiding this comment.
Add python type annotations to this function and everywhere. This will make sure you handle cases where this function returns None, for example.
| return ret; | ||
| } | ||
|
|
||
| function parseTime(time_string) { |
There was a problem hiding this comment.
For all functions in this file, I would DEFINITELY comment an example input and output. It is very difficulty to see what's actually happening here.
|
|
||
| function consolidateTimes(times) { | ||
| selected_classes_meeting_times = [[],[],[],[],[]] | ||
| for (var selected_class = 0; selected_class < times.length; selected_class++) { |
There was a problem hiding this comment.
Also, once again, the organization of this data makes think function very odd. Wouldn't it make more sense, for example, to have a list of selected class identifiers that map to meeting times explicitly, rather than implicityl through an array structure?
| @@ -0,0 +1,89 @@ | |||
| /* eslint-disable */ | |||
There was a problem hiding this comment.
Don't disable - LMK if you are having trouble configuring your IDE. I recommend the eslint plugin and configuring auto-formatting. Artie added VSCode setup docs
| <script> | ||
|
|
||
| function buttonDoubleSubmit() { | ||
| // this function is for preventing a double submit |
There was a problem hiding this comment.
What do you mean by this - "double-submit" on forms is usually handled via a sort of threshold/debounce time.
There was a problem hiding this comment.
This code basically does that.
| if(isConflict){ | ||
| // if there is a conflict, alert the user and return | ||
| event.preventDefault(); | ||
| alert("Error: new course has a time conflict with this schedule. Select another section time and try again.") |
There was a problem hiding this comment.
Can you specify more specifically about the specific course with a conflict?
There was a problem hiding this comment.
It's possible, but I think it would be very complicated with the current code. For now it might be better to save this for a future ticket, I think.
|
Some of the bugs when testing around with the feature so far: |
I put some comments in the document. |
Schedule builder feature with edit, create, and delete capabilities.
NOTE: add to schedule from search page is not implemented on this branch and is left for future work