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.

Sign up for GitHub

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

Jump to bottom

add script to fetch new professors/courses #38

Merged
merged 55 commits into from
Jan 30, 2023
Merged

add script to fetch new professors/courses #38

merged 55 commits into from
Jan 30, 2023

Conversation

nsandler1
Copy link
Member

@nsandler1 nsandler1 commented Nov 3, 2022 •
edited by tybug
Loading

@nsandler1 nsandler1 mentioned this pull request Nov 3, 2022
Closed
1 task
@nsandler1 nsandler1 requested a review from tybug November 3, 2022 03:27
tybug
tybug requested changes Nov 3, 2022
View reviewed changes
home/management/commands/updatecourses.py Outdated Show resolved Hide resolved
home/management/commands/updatecourses.py Outdated Show resolved Hide resolved
home/management/commands/updatecourses.py Outdated Show resolved Hide resolved
home/management/commands/updatecourses.py Outdated Show resolved Hide resolved
tybug
tybug requested changes Nov 10, 2022
View reviewed changes
home/management/commands/updatecourses.py Outdated Show resolved Hide resolved
tybug
tybug requested changes Nov 10, 2022
View reviewed changes
home/management/commands/updatecourses.py Outdated Show resolved Hide resolved
@nsandler1 nsandler1 added the blocked Awaiting changes elsewhere label Nov 10, 2022
@nsandler1
Merge branch 'fuzzy-professors' into update-courses-no-descriptions
76b27b1
@nsandler1
Merge branch 'professor-alias' into update-courses-no-descriptions
1253635
@nsandler1
Revert "Merge branch 'professor-alias' into update-courses-no-descrip…
0f607d0 
…tions"

This reverts commit 1253635, reversing
changes made to 76b27b1.
@nsandler1
Revert "Merge branch 'fuzzy-professors' into update-courses-no-descri…
8a216f8 
…ptions"

This reverts commit 76b27b1, reversing
changes made to 6483460.
@nsandler1
reorder statements
e4b4d6b
@nsandler1
This reverts commit 0f607d0.
d3a40bb
@nsandler1
Merge branch 'fuzzy-professors' into update-courses-no-descriptions
c909d54
@nsandler1
Merge branch 'professor-alias' into update-courses-no-descriptions
23021f0
@nsandler1
make migrations
cfcee1c
@nsandler1
Merge branch 'professor-alias' into update-courses-no-descriptions
b7658c3
@nsandler1
update Professor.find_similar() to match fuzzy-professors
a88ddf5
@nsandler1
use new method to find similar professors
cc1a118
@nsandler1 nsandler1 removed the blocked Awaiting changes elsewhere label Nov 12, 2022
tybug
tybug requested changes Nov 13, 2022
View reviewed changes
api/serializers.py Outdated Show resolved Hide resolved
home/utils.py Outdated Show resolved Hide resolved
home/forms/professor_forms.py Outdated Show resolved Hide resolved
tybug
tybug requested changes Jan 8, 2023
View reviewed changes
home/management/commands/updatecourses.py Outdated Show resolved Hide resolved
home/management/commands/updatecourses.py Outdated Show resolved Hide resolved
@nsandler1
Copy link
Member Author

nsandler1 commented Jan 10, 2023 •
edited
Loading

This PR now depends on #71. Please be sure to run migrations before updating the courses!

This was linked to issues Jan 12, 2023
Create new script for django for getting professors #1
Closed
Create new script for django for getting courses #2
Closed
tybug
tybug approved these changes Jan 30, 2023
View reviewed changes
Copy link
Contributor

@tybug tybug left a comment

Choose a reason for hiding this comment

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

one side comment, looks ok otherwise. Tested with no immediate issues but didn't look very deep on the admin panel or validate against umdio responses.

home/management/commands/updatecourses.py
Comment on lines +112 to +116
# if there's more than one matching professor but
# we have an alias that narrows the query down to one,
# use the professor associated with that alias.
if professor.count() > 1 and alias.count() == 1:
professor = alias.first()
Copy link
Contributor

Choose a reason for hiding this comment

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

I would hope that alias.count() is always one. If we have two aliases for the same name, that's a problem. We should add a unique=True requirement to the alias' name field.

@tybug tybug merged commit 36e303f into master Jan 30, 2023
@tybug tybug deleted the update-courses-no-descriptions branch January 30, 2023 05:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tybug tybug tybug approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Create new script for django for getting courses Create new script for django for getting professors
2 participants
@nsandler1 @tybug