Merge pull request #716 from xchem/m2ms-1552-cset-refactor

Refactor ComputedSet model to use numeric id rather than name as pk (Issue1552)

Author: kaliif

api/urls.py

@@ -124,11 +124,11 @@
 router.register(
     "metadata_upload", viewer_views.UploadMetadataView, basename='metadata_upload'
 )
-router.register(
-    "computedset_download",
-    viewer_views.DownloadComputedSetView,
-    basename='computedset_download',
-)
+# router.register(
+#     "computedset_download",
+#     viewer_views.DownloadComputedSetView,
+#     basename='computedset_download',
+# )
 
 # Squonk Jobs
 router.register(

viewer/cset_upload.py

@@ -175,7 +175,7 @@ def __init__(
         version,
         zfile,
         zfile_hashvals,
-        computed_set_name,
+        computed_set_id,
     ):
         self.user_id = user_id
         self.sdf_filename = sdf_filename
@@ -185,7 +185,7 @@ def __init__(
         self.version = version
         self.zfile = zfile
         self.zfile_hashvals = zfile_hashvals
-        self.computed_set_name = computed_set_name
+        self.computed_set_id = computed_set_id
 
         # using the same mechanism to pass messages as validation
         self.messages: dict[str, Any] = {
@@ -766,12 +766,13 @@ def task(self) -> tuple[ComputedSet, dict]:
 
                 # Do we have any existing ComputedSets?
                 try:
-                    computed_set = ComputedSet.objects.get(name=self.computed_set_name)
+                    computed_set = ComputedSet.objects.get(pk=self.computed_set_id)
                     # refresh some attributes
                     computed_set.md_ordinal = F("md_ordinal") + 1
                     computed_set.upload_date = datetime.date.today()
                     computed_set.save()
-                except ComputedSet.DoesNotExist:
+                except (ValueError, ComputedSet.DoesNotExist):
+                    # ValueError when pk is None
                     # no, create new
 
                     today: datetime.date = datetime.date.today()
@@ -790,16 +791,30 @@ def task(self) -> tuple[ComputedSet, dict]:
                         f"{truncated_submitter_method}-{str(today)}-"
                         + f"{get_column_letter(new_ordinal)}"
                     )
-                    logger.info('Creating new ComputedSet "%s"', cs_name)
-
-                    computed_set = ComputedSet(
-                        name=cs_name,
-                        md_ordinal=new_ordinal,
-                        upload_date=today,
-                        method=self.submitter_method[: ComputedSet.LENGTH_METHOD],
-                        target=target,
-                        spec_version=float(self.version.strip("ver_")),
-                    )
+
+                    # now that I have a name, I can check whether this
+                    # target already has this set
+                    try:
+                        # this feels wrong, I think it's better if the
+                        # object is resolved in the view.. or maybe in
+                        # validate task..
+                        computed_set = ComputedSet.objects.get(
+                            name=cs_name,
+                            target=target,
+                        )
+                    except ComputedSet.DoesNotExist:
+                        # and only now create new set
+                        logger.info('Creating new ComputedSet "%s"', cs_name)
+
+                        computed_set = ComputedSet(
+                            name=cs_name,
+                            md_ordinal=new_ordinal,
+                            upload_date=today,
+                            method=self.submitter_method[: ComputedSet.LENGTH_METHOD],
+                            target=target,
+                            spec_version=float(self.version.strip('ver_')),
+                        )
+
                     if self.user_id:
                         try:
                             computed_set.owner_user = User.objects.get(id=self.user_id)

viewer/filters.py

@@ -7,6 +7,7 @@
     CanonSite,
     CanonSiteConf,
     Compound,
+    ComputedSet,
     Experiment,
     ExperimentUpload,
     Pose,
@@ -128,6 +129,18 @@ class Meta:
         fields = ("target",)
 
 
+class ComputedSetFilter(filters.FilterSet):
+    project = django_filters.CharFilter(
+        field_name="project",
+        lookup_expr="icontains",
+        label="Project",
+    )
+
+    class Meta:
+        model = ComputedSet
+        fields = ("name", "target", "project")
+
+
 class ExperimentUploadFilter(filters.FilterSet):
     class Meta:
         model = ExperimentUpload

viewer/managers.py

@@ -415,6 +415,27 @@ def by_target(self, target):
         return self.get_queryset().filter_qs().filter(target=target.id)
 
 
+class ComputedSetQueryset(QuerySet):
+    def filter_qs(self):
+        ComputedSet = apps.get_model("viewer", "ComputedSet")
+        qs = ComputedSet.objects.annotate(
+            project=F("target__project__title"),
+        )
+
+        return qs
+
+
+class ComputedSetDataManager(Manager):
+    def get_queryset(self):
+        return ComputedSetQueryset(self.model, using=self._db)
+
+    def filter_qs(self):
+        return self.get_queryset().filter_qs()
+
+    def by_target(self, target):
+        return self.get_queryset().filter_qs().filter(target=target.id)
+
+
 class ExperimentUploadQueryset(QuerySet):
     def annotated_qs(self):
         ExperimentUpload = apps.get_model("viewer", "ExperimentUpload")

viewer/migrations/0085_auto_20241202_1157.py

@@ -0,0 +1,32 @@
+# Generated by Django 3.2.25 on 2024-12-02 11:57
+
+from django.db import migrations, models
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('viewer', '0084_auto_20241108_1119'),
+    ]
+
+    operations = [
+        migrations.RemoveField(
+            model_name='computedset',
+            name='computed_molecules',
+        ),
+        migrations.AlterField(
+            model_name='computedsetcomputedmolecule',
+            name='computed_set',
+            field=models.TextField(null=True),
+        ),
+        migrations.AlterField(
+            model_name='jobrequest',
+            name='computed_set',
+            field=models.TextField(null=True),
+        ),
+        migrations.AlterField(
+            model_name='scoredescription',
+            name='computed_set',
+            field=models.TextField(null=True),
+        ),
+    ]

viewer/migrations/0086_auto_20241202_1205.py

@@ -0,0 +1,23 @@
+# Generated by Django 3.2.25 on 2024-12-02 12:05
+
+from django.db import migrations, models
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('viewer', '0085_auto_20241202_1157'),
+    ]
+
+    operations = [
+        migrations.AddField(
+            model_name='computedset',
+            name='new_id',
+            field=models.IntegerField(default=0),
+        ),
+        migrations.AddField(
+            model_name='historicalcomputedset',
+            name='new_id',
+            field=models.IntegerField(default=0),
+        ),
+    ]

viewer/migrations/0087_auto_20241202_1206.py

@@ -0,0 +1,24 @@
+# Generated by Django 3.2.25 on 2024-12-02 12:06
+
+from django.db import migrations
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('viewer', '0086_auto_20241202_1205'),
+    ]
+
+    def populate_ids(apps, schema_editor):
+        ComputedSet = apps.get_model('viewer', 'ComputedSet')
+
+        for i, obj in enumerate(ComputedSet.objects.all()):
+            obj.new_id = i + 1
+            obj.save()
+
+    def reverse_populate_ids(apps, schema_editor):
+        pass
+
+    operations = [
+        migrations.RunPython(populate_ids, reverse_populate_ids),
+    ]

viewer/migrations/0088_auto_20241202_1224.py

@@ -0,0 +1,33 @@
+# Generated by Django 3.2.25 on 2024-12-02 12:24
+
+from django.db import migrations, models
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('viewer', '0087_auto_20241202_1206'),
+    ]
+
+    operations = [
+        migrations.AlterField(
+            model_name='computedset',
+            name='name',
+            field=models.TextField(),
+        ),
+        migrations.AlterField(
+            model_name='computedset',
+            name='new_id',
+            field=models.IntegerField(primary_key=True, serialize=False),
+        ),
+        migrations.AlterField(
+            model_name='historicalcomputedset',
+            name='name',
+            field=models.TextField(),
+        ),
+        migrations.AlterField(
+            model_name='historicalcomputedset',
+            name='new_id',
+            field=models.IntegerField(db_index=True),
+        ),
+    ]

viewer/migrations/0089_auto_20241202_1230.py

@@ -0,0 +1,23 @@
+# Generated by Django 3.2.25 on 2024-12-02 12:30
+
+from django.db import migrations
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('viewer', '0088_auto_20241202_1224'),
+    ]
+
+    operations = [
+        migrations.RenameField(
+            model_name='computedset',
+            old_name='new_id',
+            new_name='id',
+        ),
+        migrations.RenameField(
+            model_name='historicalcomputedset',
+            old_name='new_id',
+            new_name='id',
+        ),
+    ]

viewer/migrations/0090_auto_20241202_1231.py

@@ -0,0 +1,23 @@
+# Generated by Django 3.2.25 on 2024-12-02 12:31
+
+from django.db import migrations, models
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('viewer', '0089_auto_20241202_1230'),
+    ]
+
+    operations = [
+        migrations.AlterField(
+            model_name='computedset',
+            name='id',
+            field=models.BigAutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID'),
+        ),
+        migrations.AlterField(
+            model_name='historicalcomputedset',
+            name='id',
+            field=models.BigIntegerField(auto_created=True, blank=True, db_index=True, verbose_name='ID'),
+        ),
+    ]

viewer/migrations/0091_auto_20241202_1234.py

@@ -0,0 +1,32 @@
+# Generated by Django 3.2.25 on 2024-12-02 12:34
+
+from django.db import migrations
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('viewer', '0090_auto_20241202_1231'),
+    ]
+
+    operations = [
+        migrations.RemoveConstraint(
+            model_name='computedsetcomputedmolecule',
+            name='unique_computedsetcomputedmolecule',
+        ),
+        migrations.RenameField(
+            model_name='computedsetcomputedmolecule',
+            old_name='computed_set',
+            new_name='computed_set_txt',
+        ),
+        migrations.RenameField(
+            model_name='jobrequest',
+            old_name='computed_set',
+            new_name='computed_set_txt',
+        ),
+        migrations.RenameField(
+            model_name='scoredescription',
+            old_name='computed_set',
+            new_name='computed_set_txt',
+        ),
+    ]

viewer/migrations/0092_auto_20241202_1322.py

@@ -0,0 +1,29 @@
+# Generated by Django 3.2.25 on 2024-12-02 13:22
+
+from django.db import migrations, models
+import django.db.models.deletion
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('viewer', '0091_auto_20241202_1234'),
+    ]
+
+    operations = [
+        migrations.AddField(
+            model_name='computedsetcomputedmolecule',
+            name='computed_set',
+            field=models.ForeignKey(null=True, on_delete=django.db.models.deletion.CASCADE, to='viewer.computedset'),
+        ),
+        migrations.AddField(
+            model_name='jobrequest',
+            name='computed_set',
+            field=models.ForeignKey(null=True, on_delete=django.db.models.deletion.CASCADE, to='viewer.computedset'),
+        ),
+        migrations.AddField(
+            model_name='scoredescription',
+            name='computed_set',
+            field=models.ForeignKey(null=True, on_delete=django.db.models.deletion.CASCADE, to='viewer.computedset'),
+        ),
+    ]