From bfa1ab459b2ba8bb282fd393da00f07af249462b Mon Sep 17 00:00:00 2001
From: Eva Bardou <ebardou@teklia.com>
Date: Fri, 21 May 2021 13:32:16 +0200
Subject: [PATCH] Fix some review related code snippets

---
 app/apps/api/serializers.py                   |  7 +--
 app/apps/core/admin.py                        |  2 +-
 app/apps/core/forms.py                        | 57 ++++++++-----------
 ...518_1235.py => 0045_auto_20210521_1034.py} | 16 +++---
 app/apps/core/models.py                       |  8 +--
 app/apps/core/tests/factory.py                |  3 +-
 6 files changed, 43 insertions(+), 50 deletions(-)
 rename app/apps/core/migrations/{0045_auto_20210518_1235.py => 0045_auto_20210521_1034.py} (79%)

diff --git a/app/apps/api/serializers.py b/app/apps/api/serializers.py
index fc2d0296..f57fc5d0 100644
--- a/app/apps/api/serializers.py
+++ b/app/apps/api/serializers.py
@@ -4,6 +4,7 @@ import html
 
 from django.conf import settings
 from django.db.utils import IntegrityError
+from django.utils import timezone
 from django.utils.translation import gettext_lazy as _
 
 from rest_framework import serializers
@@ -395,8 +396,7 @@ class SegTrainSerializer(ProcessSerializerMixin, serializers.Serializer):
             OcrModelDocument.objects.create(
                 document=self.document,
                 ocr_model=model,
-                trained_on=False,
-                executed_on=True,
+                executed_on=timezone.now(),
             )
 
         segtrain.delay(model.pk if model else None, self.document.pk,
@@ -437,8 +437,7 @@ class TrainSerializer(ProcessSerializerMixin, serializers.Serializer):
             OcrModelDocument.objects.create(
                 document=self.document,
                 ocr_model=model,
-                trained_on=False,
-                executed_on=True,
+                executed_on=timezone.now(),
             )
 
         train.delay([part.pk for part in self.validated_data.get('parts')],
diff --git a/app/apps/core/admin.py b/app/apps/core/admin.py
index c19039b5..fc4881ac 100644
--- a/app/apps/core/admin.py
+++ b/app/apps/core/admin.py
@@ -51,7 +51,7 @@ class OcrModelAdmin(admin.ModelAdmin):
 
 
 class OcrModelDocumentAdmin(admin.ModelAdmin):
-    list_display = ['document', 'ocr_model', 'trained_on', 'executed_on', 'created']
+    list_display = ['document', 'ocr_model', 'trained_on', 'executed_on', 'created_at']
 
 
 admin.site.register(Document, DocumentAdmin)
diff --git a/app/apps/core/forms.py b/app/apps/core/forms.py
index d9811a51..dbddbe18 100644
--- a/app/apps/core/forms.py
+++ b/app/apps/core/forms.py
@@ -7,6 +7,7 @@ from django.conf import settings
 from django.core.validators import FileExtensionValidator, MinValueValidator, MaxValueValidator
 from django.db.models import Q
 from django.forms.models import inlineformset_factory
+from django.utils import timezone
 from django.utils.functional import cached_property
 from django.utils.translation import gettext_lazy as _
 
@@ -178,8 +179,7 @@ class DocumentSegmentForm(DocumentProcessForm1):
             OcrModelDocument.objects.create(
                 document=self.parts[0].document,
                 ocr_model=model,
-                trained_on=False,
-                executed_on=True,
+                executed_on=timezone.now(),
             )
             # Note: needs to save the file in a second step because the path needs the db PK
             model.file = data['upload_model']
@@ -190,10 +190,10 @@ class DocumentSegmentForm(DocumentProcessForm1):
             ocr_model_document, created = OcrModelDocument.objects.get_or_create(
                 ocr_model=model,
                 document=self.parts[0].document,
-                defaults={'trained_on': False, 'executed_on': True}
+                defaults={'executed_on': timezone.now()}
             )
             if not created:
-                ocr_model_document.executed_on = True
+                ocr_model_document.executed_on = timezone.now()
                 ocr_model_document.save()
         else:
             model = None
@@ -240,10 +240,10 @@ class DocumentTrainForm(DocumentProcessForm1):
             ocr_model_document, created = OcrModelDocument.objects.get_or_create(
                 ocr_model=model,
                 document=self.parts[0].document,
-                defaults={'trained_on': True, 'executed_on': False}
+                defaults={'trained_on': timezone.now()}
             )
             if not created:
-                ocr_model_document.trained_on = True
+                ocr_model_document.trained_on = timezone.now()
                 ocr_model_document.save()
 
         elif data.get('upload_model'):
@@ -254,8 +254,7 @@ class DocumentTrainForm(DocumentProcessForm1):
             OcrModelDocument.objects.create(
                 document=self.parts[0].document,
                 ocr_model=model,
-                trained_on=True,
-                executed_on=False,
+                trained_on=timezone.now(),
             )
             # Note: needs to save the file in a second step because the path needs the db PK
             model.file = data['upload_model']
@@ -270,8 +269,7 @@ class DocumentTrainForm(DocumentProcessForm1):
             OcrModelDocument.objects.create(
                 document=self.parts[0].document,
                 ocr_model=model,
-                trained_on=True,
-                executed_on=False,
+                trained_on=timezone.now(),
             )
 
         else:
@@ -313,10 +311,10 @@ class DocumentSegtrainForm(DocumentProcessForm1):
             ocr_model_document, created = OcrModelDocument.objects.get_or_create(
                 ocr_model=model,
                 document=self.parts[0].document,
-                defaults={'trained_on': True, 'executed_on': False}
+                defaults={'trained_on': timezone.now()}
             )
             if not created:
-                ocr_model_document.trained_on = True
+                ocr_model_document.trained_on = timezone.now()
                 ocr_model_document.save()
         elif data.get('upload_model'):
             model = OcrModel.objects.create(
@@ -326,8 +324,7 @@ class DocumentSegtrainForm(DocumentProcessForm1):
             OcrModelDocument.objects.create(
                 document=self.parts[0].document,
                 ocr_model=model,
-                trained_on=True,
-                executed_on=False,
+                trained_on=timezone.now(),
             )
             # Note: needs to save the file in a second step because the path needs the db PK
             model.file = data['upload_model']
@@ -342,8 +339,7 @@ class DocumentSegtrainForm(DocumentProcessForm1):
             OcrModelDocument.objects.create(
                 document=self.parts[0].document,
                 ocr_model=model,
-                trained_on=True,
-                executed_on=False,
+                trained_on=timezone.now(),
             )
 
         else:
@@ -383,8 +379,7 @@ class DocumentTranscribeForm(DocumentProcessForm1):
             OcrModelDocument.objects.create(
                 document=self.parts[0].document,
                 ocr_model=model,
-                trained_on=False,
-                executed_on=True,
+                executed_on=timezone.now(),
             )
             # Note: needs to save the file in a second step because the path needs the db PK
             model.file = data['upload_model']
@@ -395,10 +390,10 @@ class DocumentTranscribeForm(DocumentProcessForm1):
             ocr_model_document, created = OcrModelDocument.objects.get_or_create(
                 ocr_model=model,
                 document=self.parts[0].document,
-                defaults={'trained_on': False, 'executed_on': True}
+                defaults={'executed_on': timezone.now()}
             )
             if not created:
-                ocr_model_document.executed_on = True
+                ocr_model_document.executed_on = timezone.now()
                 ocr_model_document.save()
         else:
             raise forms.ValidationError(
@@ -552,20 +547,20 @@ class DocumentProcessForm(BootstrapFormMixin, forms.Form):
             ocr_model_document, created = OcrModelDocument.objects.get_or_create(
                 ocr_model=model,
                 document=self.parts[0].document,
-                defaults={'trained_on': True, 'executed_on': False}
+                defaults={'trained_on': timezone.now()}
             )
             if not created:
-                ocr_model_document.trained_on = True
+                ocr_model_document.trained_on = timezone.now()
                 ocr_model_document.save()
         elif task == self.TASK_SEGTRAIN and data.get('segtrain_model'):
             model = data.get('segtrain_model')
             ocr_model_document, created = OcrModelDocument.objects.get_or_create(
                 ocr_model=model,
                 document=self.parts[0].document,
-                defaults={'trained_on': True, 'executed_on': False}
+                defaults={'trained_on': timezone.now()}
             )
             if not created:
-                ocr_model_document.trained_on = True
+                ocr_model_document.trained_on = timezone.now()
                 ocr_model_document.save()
         elif data.get('upload_model'):
             model = OcrModel.objects.create(
@@ -575,8 +570,7 @@ class DocumentProcessForm(BootstrapFormMixin, forms.Form):
             OcrModelDocument.objects.create(
                 document=self.parts[0].document,
                 ocr_model=model,
-                trained_on=False,
-                executed_on=True,
+                executed_on=timezone.now(),
             )
             # Note: needs to save the file in a second step because the path needs the db PK
             model.file = data['upload_model']
@@ -591,28 +585,27 @@ class DocumentProcessForm(BootstrapFormMixin, forms.Form):
             OcrModelDocument.objects.create(
                 document=self.parts[0].document,
                 ocr_model=model,
-                trained_on=True,
-                executed_on=False,
+                trained_on=timezone.now(),
             )
         elif data.get('ocr_model'):
             model = data.get('ocr_model')
             ocr_model_document, created = OcrModelDocument.objects.get_or_create(
                 ocr_model=model,
                 document=self.parts[0].document,
-                defaults={'trained_on': False, 'executed_on': True}
+                defaults={'executed_on': timezone.now()}
             )
             if not created:
-                ocr_model_document.executed_on = True
+                ocr_model_document.executed_on = timezone.now()
                 ocr_model_document.save()
         elif data.get('seg_model'):
             model = data.get('seg_model')
             ocr_model_document, created = OcrModelDocument.objects.get_or_create(
                 ocr_model=model,
                 document=self.parts[0].document,
-                defaults={'trained_on': False, 'executed_on': True}
+                defaults={'executed_on': timezone.now()}
             )
             if not created:
-                ocr_model_document.executed_on = True
+                ocr_model_document.executed_on = timezone.now()
                 ocr_model_document.save()
         else:
             if task in (self.TASK_TRAIN, self.TASK_SEGTRAIN):
diff --git a/app/apps/core/migrations/0045_auto_20210518_1235.py b/app/apps/core/migrations/0045_auto_20210521_1034.py
similarity index 79%
rename from app/apps/core/migrations/0045_auto_20210518_1235.py
rename to app/apps/core/migrations/0045_auto_20210521_1034.py
index a31d6819..1deb56b1 100644
--- a/app/apps/core/migrations/0045_auto_20210518_1235.py
+++ b/app/apps/core/migrations/0045_auto_20210521_1034.py
@@ -1,7 +1,8 @@
-# Generated by Django 2.2.19 on 2021-05-18 12:35
+# Generated by Django 2.2.19 on 2021-05-21 10:34
 
 from django.db import migrations, models
 import django.db.models.deletion
+from django.utils import timezone
 
 
 def populate_m2m(apps, schema_editor):
@@ -10,10 +11,9 @@ def populate_m2m(apps, schema_editor):
 
     OcrModelDocument.objects.bulk_create([
         OcrModelDocument(
-            document_id = model.document_id,
-            ocr_model_id = model.id,
-            trained_on = False,
-            executed_on = True,
+            document_id=model.document_id,
+            ocr_model_id=model.id,
+            executed_on=timezone.now(),
         ) for model in OcrModel.objects.exclude(document__isnull=True)
     ])
 
@@ -29,9 +29,9 @@ class Migration(migrations.Migration):
             name='OcrModelDocument',
             fields=[
                 ('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')),
-                ('created', models.DateTimeField(auto_now_add=True)),
-                ('trained_on', models.BooleanField()),
-                ('executed_on', models.BooleanField()),
+                ('created_at', models.DateTimeField(auto_now_add=True)),
+                ('trained_on', models.DateTimeField(null=True)),
+                ('executed_on', models.DateTimeField(null=True)),
                 ('document', models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, related_name='ocr_model_documents', to='core.Document')),
                 ('ocr_model', models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, related_name='ocr_model_documents', to='core.OcrModel')),
             ],
diff --git a/app/apps/core/models.py b/app/apps/core/models.py
index 20b352fd..edf76b24 100644
--- a/app/apps/core/models.py
+++ b/app/apps/core/models.py
@@ -1083,7 +1083,7 @@ class LineTranscription(Versioned, models.Model):
 
 def models_path(instance, filename):
     fn, ext = os.path.splitext(filename)
-    return 'models/%s%s' % (slugify(fn), ext)
+    return 'models/%d/%s%s' % (instance.owner.pk, slugify(fn), ext)
 
 
 class OcrModel(Versioned, models.Model):
@@ -1182,9 +1182,9 @@ class OcrModel(Versioned, models.Model):
 class OcrModelDocument(models.Model):
     document = models.ForeignKey(Document, on_delete=models.CASCADE, related_name='ocr_model_documents')
     ocr_model = models.ForeignKey(OcrModel, on_delete=models.CASCADE, related_name='ocr_model_documents')
-    created = models.DateTimeField(auto_now_add=True)
-    trained_on = models.BooleanField()
-    executed_on = models.BooleanField()
+    created_at = models.DateTimeField(auto_now_add=True)
+    trained_on = models.DateTimeField(null=True)
+    executed_on = models.DateTimeField(null=True)
 
     class Meta:
         unique_together = (('document', 'ocr_model'),)
diff --git a/app/apps/core/tests/factory.py b/app/apps/core/tests/factory.py
index 9c5655cc..2c4028a2 100644
--- a/app/apps/core/tests/factory.py
+++ b/app/apps/core/tests/factory.py
@@ -6,6 +6,7 @@ import os.path
 from django.conf import settings
 from django.core.files.uploadedfile import SimpleUploadedFile
 from django.test import TestCase
+from django.utils import timezone
 
 from django_redis import get_redis_connection
 from kraken.lib import vgsl
@@ -93,7 +94,7 @@ class CoreFactory():
         model = document.ocr_models.add(
             name=model_name,
             job=job,
-            through_defaults={'trained_on': False, 'executed_on': True}
+            through_defaults={'executed_on': timezone.now()}
         )
         modeldir = os.path.join(settings.MEDIA_ROOT, os.path.split(
             model.file.field.upload_to(model, 'test-model.mlmodel'))[0])
-- 
GitLab