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

Course schema changes to support new gened format #89

Merged
merged 3 commits into from
Feb 20, 2023
Merged

Conversation

nsandler1
Copy link
Member

@nsandler1 nsandler1 commented Feb 7, 2023

Changes the course schema to include a geneds field that will be used to store the gened string from umdio.

NOTE: this pr will break anything else on the site that uses geneds. #92 updates the frontend where necessary.

@nsandler1 nsandler1 added the blocked Awaiting changes elsewhere label Feb 7, 2023
@nsandler1 nsandler1 removed the blocked Awaiting changes elsewhere label Feb 7, 2023
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.

I would prefer to rename geneds to geneds_json, but we can probably do that after this PR chain to reduce fussing with migrations, assuming you don't disagree. Otherwise the structure looks good.

Comment on lines +222 to +234
def gened_str(self):
gened_str = []
for item in self.geneds:
add_parens = len(item) > 1 and len(self.geneds) > 1
and_str = "(" if add_parens else ""
and_str += " and ".join(item)
and_str += ")" if add_parens else ""

# special umdio case. EX: https://api.umd.io/v1/courses/CHEM131
and_str = and_str.replace("|", " if taken with ")
gened_str.append(and_str)
return " or ".join(gened_str)

Copy link
Contributor

Choose a reason for hiding this comment

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

nit: this change would have fit better in #91.

@tybug tybug merged commit 4fe3fa0 into master Feb 20, 2023
@tybug tybug deleted the gened-schema branch February 20, 2023 02:01
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.

2 participants