From b2bba1e053f036f8dfad4293a017be30ae66824d Mon Sep 17 00:00:00 2001 From: aadalal <57609353+AaDalal@users.noreply.github.com> Date: Mon, 5 Feb 2024 20:01:16 -0500 Subject: [PATCH] Allow multiple degrees per degreeplan and multiple degrees per rule (for deduplication) --- .../management/commands/deduplicate_rules.py | 45 +++++++++++++++++++ .../management/commands/fetch_degrees.py | 7 +-- .../management/commands/load_degrees.py | 12 ++--- .../migrations/0006_auto_20240205_1950.py | 37 +++++++++++++++ backend/degree/models.py | 32 ++++++------- backend/degree/serializers.py | 11 +++-- backend/degree/utils/parse_degreeworks.py | 20 +++++++-- backend/degree/views.py | 4 +- 8 files changed, 124 insertions(+), 44 deletions(-) create mode 100644 backend/degree/management/commands/deduplicate_rules.py create mode 100644 backend/degree/migrations/0006_auto_20240205_1950.py diff --git a/backend/degree/management/commands/deduplicate_rules.py b/backend/degree/management/commands/deduplicate_rules.py new file mode 100644 index 000000000..5a1679f59 --- /dev/null +++ b/backend/degree/management/commands/deduplicate_rules.py @@ -0,0 +1,45 @@ +from textwrap import dedent + +from django.core.management.base import BaseCommand +from django.db import transaction +from degree.models import Rule +from degree.serializers import RuleSerializer +import json +from collections import defaultdict + +class Command(BaseCommand): + help = dedent( + """ + Removes rules that are identical (based on content hash except for rule ids) + """ + ) + + @transaction.atomic + def handle(self, *args, **kwargs): + # get toposort of rules + rules = list(Rule.objects.filter(parent=None).order_by('id')) + + # serialize rules to fixed format + rules = { + rule.id: hash(json.dumps( + RuleSerializer(rule).data, + sort_keys=True, + ensure_ascii=True + )) + for rule in rules + } + + # invert rules + inverted_rules = defaultdict(list) + for rule, hash in rules.items(): + inverted_rules[hash].append(rule) + + # fold the rules + for hash, rule_ids in inverted_rules.items(): + if len(rule_ids) > 1: + print(f"Removing rules {rule_ids[1:]}") + Rule.objects.filter(id__in=rule_ids[1:]).values_list("parent_id", flat=True).update(parent_id=rule_ids[0] + + + + diff --git a/backend/degree/management/commands/fetch_degrees.py b/backend/degree/management/commands/fetch_degrees.py index 821ef3a1d..b74457968 100644 --- a/backend/degree/management/commands/fetch_degrees.py +++ b/backend/degree/management/commands/fetch_degrees.py @@ -7,7 +7,7 @@ from courses.util import get_current_semester from degree.models import Degree, program_code_to_name from degree.utils.degreeworks_client import DegreeworksClient -from degree.utils.parse_degreeworks import parse_degreeworks +from degree.utils.parse_degreeworks import parse_and_save_degreeworks class Command(BaseCommand): @@ -93,7 +93,4 @@ def handle(self, *args, **kwargs): degree.save() print(f"Saving degree {degree}...") - rules = parse_degreeworks(client.audit(degree), degree) - for rule in rules: - rule.degree = degree - rule.save() + parse_and_save_degreeworks(client.audit(degree), degree) \ No newline at end of file diff --git a/backend/degree/management/commands/load_degrees.py b/backend/degree/management/commands/load_degrees.py index 481207a5d..446b508f0 100644 --- a/backend/degree/management/commands/load_degrees.py +++ b/backend/degree/management/commands/load_degrees.py @@ -2,13 +2,12 @@ import re from os import listdir, path from textwrap import dedent -import logging from django.core.management.base import BaseCommand from django.db import transaction from degree.models import Degree, program_code_to_name -from degree.utils.parse_degreeworks import parse_degreeworks +from degree.utils.parse_degreeworks import parse_and_save_degreeworks class Command(BaseCommand): @@ -61,14 +60,11 @@ def handle(self, *args, **kwargs): concentration=concentration, year=year, ) - degree.save() with open(path.join(directory, degree_file)) as f: degree_json = json.load(f) - rules = parse_degreeworks(degree_json, degree) - print(f"Saving degree {degree}...") - for rule in rules: - rule.degree = degree - rule.save() + print(f"Parsing and saving degree {degree}...") + parse_and_save_degreeworks(degree_json, degree) + \ No newline at end of file diff --git a/backend/degree/migrations/0006_auto_20240205_1950.py b/backend/degree/migrations/0006_auto_20240205_1950.py new file mode 100644 index 000000000..048ddf55a --- /dev/null +++ b/backend/degree/migrations/0006_auto_20240205_1950.py @@ -0,0 +1,37 @@ +# Generated by Django 3.2.23 on 2024-02-06 00:50 + +from django.db import migrations, models +import django.db.models.deletion + + +class Migration(migrations.Migration): + + dependencies = [ + ('degree', '0005_auto_20240205_0150'), + ] + + operations = [ + migrations.RemoveField( + model_name='degreeplan', + name='degree', + ), + migrations.RemoveField( + model_name='rule', + name='degree', + ), + migrations.AddField( + model_name='degree', + name='rules', + field=models.ManyToManyField(blank=True, help_text='\nThe rules for this degree. Blank if this degree has no rules.\n', related_name='degrees', to='degree.Rule'), + ), + migrations.AddField( + model_name='degreeplan', + name='degrees', + field=models.ManyToManyField(help_text='The degree this is associated with.', to='degree.Degree'), + ), + migrations.AlterField( + model_name='rule', + name='parent', + field=models.ForeignKey(help_text="\nThis rule's parent Rule if it has one. Null if this is a top level rule\n(i.e., this rule belongs to some Degree's `.rules` set).\n", null=True, on_delete=django.db.models.deletion.CASCADE, related_name='children', to='degree.rule'), + ), + ] diff --git a/backend/degree/models.py b/backend/degree/models.py index 6765eba50..f3d0722fd 100644 --- a/backend/degree/models.py +++ b/backend/degree/models.py @@ -71,6 +71,16 @@ class Degree(models.Model): """ ) ) + rules = models.ManyToManyField( + "Rule", + related_name="degrees", + blank=True, + help_text=dedent( + """ + The rules for this degree. Blank if this degree has no rules. + """ + ), + ) class Meta: constraints = [ @@ -120,18 +130,6 @@ class Rule(models.Model): ), ) - degree = models.ForeignKey( - Degree, - null=True, - on_delete=models.CASCADE, - related_name="rules", - help_text=dedent( - """ - The degree that has this rule. Null if this rule has a parent. - """ - ), - ) - q = models.TextField( max_length=1000, blank=True, @@ -152,7 +150,7 @@ class Rule(models.Model): help_text=dedent( """ This rule's parent Rule if it has one. Null if this is a top level rule - (i.e., degree is not null). + (i.e., this rule belongs to some Degree's `.rules` set). """ ), related_name="children", @@ -217,9 +215,8 @@ class DegreePlan(models.Model): name = models.CharField(max_length=255, help_text="The user's nickname for the degree plan.") - degree = models.ForeignKey( + degrees = models.ManyToManyField( Degree, - on_delete=models.CASCADE, help_text="The degree this is associated with.", ) @@ -259,8 +256,7 @@ def check_satisfactions(self) -> bool: Returns True if for each Rule in this DegreePlan's Degree, there is a SatisfactionStatus for this DegreePlan/Rule combination is satisfied. """ - - top_level_rules = Rule.objects.filter(degree=self.degree) + top_level_rules = Rule.objects.filter(degrees__in=self.degrees) for rule in top_level_rules: status = SatisfactionStatus.objects.filter(degree_plan=self, rule=rule).first() if not status.satisfied: @@ -387,8 +383,6 @@ def update_satisfaction_statuses(sender, instance, action, pk_set, **kwargs): [fulfillment.full_code for fulfillment in degree_plan.fulfillments.all()] ) status.save() - - m2m_changed.connect(update_satisfaction_statuses, sender=Fulfillment.rules.through) diff --git a/backend/degree/serializers.py b/backend/degree/serializers.py index 06a81f7b3..e799f2e32 100644 --- a/backend/degree/serializers.py +++ b/backend/degree/serializers.py @@ -66,7 +66,7 @@ def validate(self, data): full_code = data.get("full_code") degree_plan = data.get("degree_plan") - if rules is None and full_code is None: + if rules is None and full_code is None and degree_plan is None: return data # Nothing to validate if rules is None: rules = self.instance.rules.all() @@ -96,12 +96,12 @@ def validate(self, data): class DegreePlanListSerializer(serializers.ModelSerializer): - degree = DegreeListSerializer(read_only=True) + degrees = DegreeListSerializer(read_only=True, many=True) id = serializers.ReadOnlyField(help_text="The id of the DegreePlan.") class Meta: model = DegreePlan - fields = ["id", "name", "degree"] + fields = ["id", "name", "degrees"] class DegreePlanDetailSerializer(serializers.ModelSerializer): @@ -110,14 +110,13 @@ class DegreePlanDetailSerializer(serializers.ModelSerializer): read_only=True, help_text="The courses used to fulfill degree plan.", ) - degree = DegreeDetailSerializer(read_only=True) - degree_id = serializers.PrimaryKeyRelatedField( + degrees = DegreeDetailSerializer(read_only=True, many=True) + degree_ids = serializers.PrimaryKeyRelatedField( write_only=True, source="degree", queryset=Degree.objects.all(), help_text="The degree_id this degree plan belongs to.", ) - id = serializers.ReadOnlyField(help_text="The id of the degree plan.") person = serializers.HiddenField(default=serializers.CurrentUserDefault()) class Meta: diff --git a/backend/degree/utils/parse_degreeworks.py b/backend/degree/utils/parse_degreeworks.py index b2f378f8b..c49861d0e 100644 --- a/backend/degree/utils/parse_degreeworks.py +++ b/backend/degree/utils/parse_degreeworks.py @@ -165,7 +165,7 @@ def parse_rulearray( A ruleArray consists of a list of rule objects that contain a requirement object. """ for rule_json in ruleArray: - this_rule = Rule(parent=parent, degree=None, title=rule_json["label"]) + this_rule = Rule(parent=parent, title=rule_json["label"]) rules.append(this_rule) rule_req = rule_json["requirement"] @@ -264,9 +264,21 @@ def parse_degreeworks(json: dict, degree: Degree) -> list[Rule]: # TODO: use requirement code? credits=None, num=None, - degree=degree, ) - rules.append(degree_req) parse_rulearray(requirement["ruleArray"], degree, rules, parent=degree_req) - return rules \ No newline at end of file + return rules + +def parse_and_save_degreeworks(json: dict, degree: Degree) -> None: + """ + Parses a DegreeWorks JSON audit and saves the rules to the database. + """ + degree.save() + rules = parse_degreeworks(json, degree) + for rule in rules: + rule.save() + top_level_rules = [rule for rule in rules if rule.parent is None] + for rule in top_level_rules: + rule.refresh_from_db() + degree.rules.add(rule) + \ No newline at end of file diff --git a/backend/degree/views.py b/backend/degree/views.py index 8c59a0553..c028942e1 100644 --- a/backend/degree/views.py +++ b/backend/degree/views.py @@ -48,8 +48,8 @@ def get_queryset(self): queryset = DegreePlan.objects.filter(person=self.request.user) queryset = queryset.prefetch_related( "fulfillments", - "degree", - "degree__rules", + "degrees", + "degrees__rules", ) return queryset