From ac03527c20032840a7b1c0f2bd3f1eade399c250 Mon Sep 17 00:00:00 2001 From: David Vogt Date: Fri, 1 Jul 2022 14:13:16 +0200 Subject: [PATCH] feat(form): multiple files in file questions Change the model to allow multiple files per answer to be stored. Migrate the existing questions to the new question type, and migrate the existing answers to the new schema. Updated the GraphQL schema to allow saving and updating multiple files on an answer. The answer's value is a list of dicts, each containing a file name, and optionally an ID. When you're creating new files, add an entry without ID to the value. Caluma will then save the file, and return it back WITH and ID. BREAKING CHANGE: This renames the question type constant for file questions, and changes the semantics of the answer value for file questions as well: It is now a list of dicts instead of a single string. The response type for querying file(s) answers now also is a list instead of a single dict. --- .../test_run_analytics_cmdline.ambr | 52 +++--- caluma/caluma_core/mutation.py | 2 +- caluma/caluma_form/domain_logic.py | 88 ++++++--- caluma/caluma_form/factories.py | 46 +++-- caluma/caluma_form/filters.py | 2 +- caluma/caluma_form/historical_schema.py | 26 ++- .../0046_file_answer_reverse_keys.py | 158 ++++++++++++++++ caluma/caluma_form/models.py | 35 ++-- caluma/caluma_form/ordering.py | 2 +- caluma/caluma_form/schema.py | 45 +++-- caluma/caluma_form/serializers.py | 15 +- caluma/caluma_form/structure.py | 4 +- .../tests/__snapshots__/test_document.ambr | 176 ++++++++++-------- .../tests/__snapshots__/test_history.ambr | 6 +- .../tests/__snapshots__/test_question.ambr | 14 +- caluma/caluma_form/tests/test_answer.py | 144 +++++++++++++- caluma/caluma_form/tests/test_document.py | 87 ++++++--- .../tests/test_filter_by_answer.py | 2 +- caluma/caluma_form/tests/test_history.py | 30 +-- caluma/caluma_form/tests/test_jexl.py | 2 +- .../tests/test_migrate_to_multiple_files.py | 50 +++++ caluma/caluma_form/tests/test_model.py | 34 +++- caluma/caluma_form/tests/test_question.py | 6 +- caluma/caluma_form/tests/test_type.py | 4 +- caluma/caluma_form/tests/test_validators.py | 42 +++-- caluma/caluma_form/validators.py | 8 +- .../tests/__snapshots__/test_case.ambr | 100 ++++++---- caluma/caluma_workflow/tests/test_case.py | 23 +-- caluma/schema.py | 6 +- caluma/tests/__snapshots__/test_schema.ambr | 39 ++-- 30 files changed, 880 insertions(+), 368 deletions(-) create mode 100644 caluma/caluma_form/migrations/0046_file_answer_reverse_keys.py create mode 100644 caluma/caluma_form/tests/test_migrate_to_multiple_files.py diff --git a/caluma/caluma_analytics/tests/__snapshots__/test_run_analytics_cmdline.ambr b/caluma/caluma_analytics/tests/__snapshots__/test_run_analytics_cmdline.ambr index ebf20c09b..b602da1e5 100644 --- a/caluma/caluma_analytics/tests/__snapshots__/test_run_analytics_cmdline.ambr +++ b/caluma/caluma_analytics/tests/__snapshots__/test_run_analytics_cmdline.ambr @@ -65,7 +65,7 @@ FROM "caluma_form_document" INNER JOIN "caluma_form_form" ON ("caluma_form_document"."form_id" = "caluma_form_form"."slug") -- qs ref ), - answer_b4215 AS (SELECT "caluma_form_answer"."created_at", + answer_af542 AS (SELECT "caluma_form_answer"."created_at", "caluma_form_answer"."modified_at", "caluma_form_answer"."created_by_user", "caluma_form_answer"."created_by_group", @@ -76,8 +76,7 @@ "caluma_form_answer"."value", "caluma_form_answer"."meta", "caluma_form_answer"."document_id", - "caluma_form_answer"."date", - "caluma_form_answer"."file_id" + "caluma_form_answer"."date" FROM "caluma_form_answer" INNER JOIN "caluma_form_question" ON ("caluma_form_answer"."question_id" = "caluma_form_question"."slug") -- qs ref ) @@ -92,14 +91,14 @@ FROM document_2a07e AS "document_2a07e" LEFT JOIN ( SELECT DISTINCT ON (document_id) - (answer_b4215.value #>>'{}') AS "analytics_result_blablub", + (answer_af542.value #>>'{}') AS "analytics_result_blablub", "document_id" - FROM answer_b4215 AS "answer_b4215" + FROM answer_af542 AS "answer_af542" WHERE "question_id" = 'top_question' ORDER BY document_id - ) AS "answer_b4215_caea5" ON (document_2a07e.id = "answer_b4215_caea5".document_id) + ) AS "answer_af542_caea5" ON (document_2a07e.id = "answer_af542_caea5".document_id) WHERE form_id = 'top_form' ORDER BY id @@ -107,7 +106,7 @@ ) AS "document_2a07e_27f15" ON (case_ac50e.document_id = "document_2a07e_27f15".id) - ) AS analytics_247d1 + ) AS analytics_2a8de -- PARAMS: ''', @@ -156,7 +155,7 @@ FROM "caluma_form_document" INNER JOIN "caluma_form_form" ON ("caluma_form_document"."form_id" = "caluma_form_form"."slug") -- qs ref ), - answer_b4215 AS (SELECT "caluma_form_answer"."created_at", + answer_af542 AS (SELECT "caluma_form_answer"."created_at", "caluma_form_answer"."modified_at", "caluma_form_answer"."created_by_user", "caluma_form_answer"."created_by_group", @@ -167,8 +166,7 @@ "caluma_form_answer"."value", "caluma_form_answer"."meta", "caluma_form_answer"."document_id", - "caluma_form_answer"."date", - "caluma_form_answer"."file_id" + "caluma_form_answer"."date" FROM "caluma_form_answer" INNER JOIN "caluma_form_question" ON ("caluma_form_answer"."question_id" = "caluma_form_question"."slug") -- qs ref ) @@ -183,14 +181,14 @@ FROM document_2a07e AS "document_2a07e" LEFT JOIN ( SELECT DISTINCT ON (document_id) - (answer_b4215.value #>>'{}') AS "analytics_result_blablub", + (answer_af542.value #>>'{}') AS "analytics_result_blablub", "document_id" - FROM answer_b4215 AS "answer_b4215" + FROM answer_af542 AS "answer_af542" WHERE "question_id" = 'top_question' ORDER BY document_id - ) AS "answer_b4215_caea5" ON (document_2a07e.id = "answer_b4215_caea5".document_id) + ) AS "answer_af542_caea5" ON (document_2a07e.id = "answer_af542_caea5".document_id) WHERE form_id = 'top_form' ORDER BY id @@ -198,7 +196,7 @@ ) AS "document_2a07e_27f15" ON (case_ac50e.document_id = "document_2a07e_27f15".id) - ) AS analytics_247d1 + ) AS analytics_2a8de WHERE analytics_result_blablub IN (%(flt_analytics_result_blablub_8201d)s, %(flt_analytics_result_blablub_8e5e6)s) -- PARAMS: -- flt_analytics_result_blablub_8201d: Shelly Watson @@ -246,7 +244,7 @@ FROM "caluma_form_document" INNER JOIN "caluma_form_form" ON ("caluma_form_document"."form_id" = "caluma_form_form"."slug") -- qs ref ), - answer_b4215 AS (SELECT "caluma_form_answer"."created_at", + answer_af542 AS (SELECT "caluma_form_answer"."created_at", "caluma_form_answer"."modified_at", "caluma_form_answer"."created_by_user", "caluma_form_answer"."created_by_group", @@ -257,8 +255,7 @@ "caluma_form_answer"."value", "caluma_form_answer"."meta", "caluma_form_answer"."document_id", - "caluma_form_answer"."date", - "caluma_form_answer"."file_id" + "caluma_form_answer"."date" FROM "caluma_form_answer" INNER JOIN "caluma_form_question" ON ("caluma_form_answer"."question_id" = "caluma_form_question"."slug") -- qs ref ) @@ -273,14 +270,14 @@ FROM document_2a07e AS "document_2a07e" LEFT JOIN ( SELECT DISTINCT ON (document_id) - (answer_b4215.value #>>'{}') AS "analytics_result_blablub", + (answer_af542.value #>>'{}') AS "analytics_result_blablub", "document_id" - FROM answer_b4215 AS "answer_b4215" + FROM answer_af542 AS "answer_af542" WHERE "question_id" = 'top_question' ORDER BY document_id - ) AS "answer_b4215_caea5" ON (document_2a07e.id = "answer_b4215_caea5".document_id) + ) AS "answer_af542_caea5" ON (document_2a07e.id = "answer_af542_caea5".document_id) WHERE form_id = 'top_form' ORDER BY id @@ -288,7 +285,7 @@ ) AS "document_2a07e_27f15" ON (case_ac50e.document_id = "document_2a07e_27f15".id) - ) AS analytics_247d1 + ) AS analytics_2a8de -- PARAMS: ''', @@ -333,7 +330,7 @@ FROM "caluma_form_document" INNER JOIN "caluma_form_form" ON ("caluma_form_document"."form_id" = "caluma_form_form"."slug") -- qs ref ), - answer_b4215 AS (SELECT "caluma_form_answer"."created_at", + answer_af542 AS (SELECT "caluma_form_answer"."created_at", "caluma_form_answer"."modified_at", "caluma_form_answer"."created_by_user", "caluma_form_answer"."created_by_group", @@ -344,8 +341,7 @@ "caluma_form_answer"."value", "caluma_form_answer"."meta", "caluma_form_answer"."document_id", - "caluma_form_answer"."date", - "caluma_form_answer"."file_id" + "caluma_form_answer"."date" FROM "caluma_form_answer" INNER JOIN "caluma_form_question" ON ("caluma_form_answer"."question_id" = "caluma_form_question"."slug") -- qs ref ) @@ -360,14 +356,14 @@ FROM document_2a07e AS "document_2a07e" LEFT JOIN ( SELECT DISTINCT ON (document_id) - (answer_b4215.value #>>'{}') AS "analytics_result_blablub", + (answer_af542.value #>>'{}') AS "analytics_result_blablub", "document_id" - FROM answer_b4215 AS "answer_b4215" + FROM answer_af542 AS "answer_af542" WHERE "question_id" = 'top_question' ORDER BY document_id - ) AS "answer_b4215_caea5" ON (document_2a07e.id = "answer_b4215_caea5".document_id) + ) AS "answer_af542_caea5" ON (document_2a07e.id = "answer_af542_caea5".document_id) WHERE form_id = 'top_form' ORDER BY id @@ -375,7 +371,7 @@ ) AS "document_2a07e_27f15" ON (case_ac50e.document_id = "document_2a07e_27f15".id) - ) AS analytics_247d1 + ) AS analytics_2a8de WHERE analytics_result_blablub IN (%(flt_analytics_result_blablub_8201d)s, %(flt_analytics_result_blablub_8e5e6)s) -- PARAMS: -- flt_analytics_result_blablub_8201d: Shelly Watson diff --git a/caluma/caluma_core/mutation.py b/caluma/caluma_core/mutation.py index 85257a5da..69cfd6c12 100644 --- a/caluma/caluma_core/mutation.py +++ b/caluma/caluma_core/mutation.py @@ -119,7 +119,7 @@ class Meta: permission_classes = None @classmethod - def __init_subclass_with_meta__( + def __init_subclass_with_meta__( # noqa: C901 cls, lookup_field=None, lookup_input_kwarg=None, diff --git a/caluma/caluma_form/domain_logic.py b/caluma/caluma_form/domain_logic.py index 2b538a4db..485e1649f 100644 --- a/caluma/caluma_form/domain_logic.py +++ b/caluma/caluma_form/domain_logic.py @@ -5,6 +5,7 @@ from rest_framework.exceptions import ValidationError from caluma.caluma_core.models import BaseModel +from caluma.caluma_core.relay import extract_global_id from caluma.caluma_form import models, validators from caluma.caluma_user.models import BaseUser from caluma.utils import update_model @@ -42,13 +43,63 @@ def get_new_answer(cls, data, user, answer): cls.validate_for_save(data, user, answer=answer, origin=True) ) + files = validated_data.pop("files", None) if answer is None: answer = cls.create(validated_data, user) else: answer = cls.update(answer, validated_data, user) + # FILES type needs a bit more work due to the reverse + # relation of file -> answer + if answer.question.type == models.Question.TYPE_FILES: + if files is None: + raise ValidationError("Files input must be a list") + cls.update_answer_files(answer, files) + return cls.post_save(answer) + @classmethod + def update_answer_files(cls, answer: models.Answer, files: list): + """Update the files of a "FILES" answer. + + The files parameter is expected to be a list of dicts, where + each entry has a "name" (the file name), and optionally an "id" + for the case when the given file already exists. + """ + if not files: + files = [] + + updated = [] + + for file_ in files: + file_id = extract_global_id(file_["id"]) if "id" in file_ else None + file_name = file_["name"] + file_model = ( + answer.files.filter(pk=file_id).first() + if file_id + else models.File(answer=answer, name=file_name) + ) + if not file_model: + # Client wants to update a file that doesn't exist anymore. + # Reject call - this is an inconsistency as either + # the file was never created, or was deleted in the mean + # time. + raise ValidationError( + f"File with id={file_id} for given answer not found" + ) + + if file_model.name != file_name and file_id: + # Renaming existing file + file_model.rename(file_name) + file_model.save() + elif not file_id: + # New file + file_model.save() + updated.append(file_model.pk) + + for file in answer.files.exclude(pk__in=updated): + file.delete() + @staticmethod def validate_for_save( data: dict, user: BaseUser, answer: models.Answer = None, origin: bool = False @@ -68,8 +119,8 @@ def validate_for_save( else models.Document.objects.none() ) del data["value"] - elif question.type == models.Question.TYPE_FILE and not data.get("file"): - data["file"] = data["value"] + elif question.type == models.Question.TYPE_FILES and not data.get("files"): + data["files"] = data["value"] del data["value"] elif question.type == models.Question.TYPE_DATE and not data.get("date"): data["date"] = data["value"] @@ -91,35 +142,35 @@ def post_save(answer: models.Answer) -> models.Answer: # TODO emit events return answer - @staticmethod + @classmethod @transaction.atomic - def create(validated_data: dict, user: Optional[BaseUser] = None) -> models.Answer: - if validated_data["question"].type == models.Question.TYPE_FILE: - validated_data = __class__.set_file(validated_data) + def create( + cls, validated_data: dict, user: Optional[BaseUser] = None + ) -> models.Answer: if validated_data["question"].type == models.Question.TYPE_TABLE: documents = validated_data.pop("documents") + files = validated_data.pop("files", None) answer = BaseLogic.create(models.Answer, validated_data, user) + if validated_data["question"].type == models.Question.TYPE_FILES: + cls.update_answer_files(answer, files) + if answer.question.type == models.Question.TYPE_TABLE: answer.create_answer_documents(documents) return answer - @staticmethod + @classmethod @transaction.atomic - def update(answer, validated_data, user: Optional[BaseUser] = None): + def update(cls, answer, validated_data, user: Optional[BaseUser] = None): if answer.question.type == models.Question.TYPE_TABLE: documents = validated_data.pop("documents") answer.unlink_unused_rows(docs_to_keep=documents) - if ( - answer.question.type == models.Question.TYPE_FILE - and answer.file.name is not validated_data["file"] - ): - answer.file.delete() - validated_data = __class__.set_file(validated_data) + if answer.question.type == models.Question.TYPE_FILES: + cls.update_answer_files(answer, validated_data.pop("files", None)) BaseLogic.update(answer, validated_data, user) @@ -129,13 +180,6 @@ def update(answer, validated_data, user: Optional[BaseUser] = None): answer.refresh_from_db() return answer - @staticmethod - def set_file(validated_data): - file_name = validated_data.get("file") - file = models.File.objects.create(name=file_name) - validated_data["file"] = file - return validated_data - class SaveDefaultAnswerLogic(SaveAnswerLogic): @staticmethod @@ -143,7 +187,7 @@ def validate_for_save( data: dict, user: BaseUser, answer: models.Answer = None, origin: bool = False ) -> dict: if data["question"].type in [ - models.Question.TYPE_FILE, + models.Question.TYPE_FILES, models.Question.TYPE_STATIC, models.Question.TYPE_DYNAMIC_CHOICE, models.Question.TYPE_DYNAMIC_MULTIPLE_CHOICE, diff --git a/caluma/caluma_form/factories.py b/caluma/caluma_form/factories.py index cc604ae9a..8a5dfbda4 100644 --- a/caluma/caluma_form/factories.py +++ b/caluma/caluma_form/factories.py @@ -1,5 +1,14 @@ import faker -from factory import Faker, LazyAttribute, Maybe, SubFactory, lazy_attribute +from django.db.models.signals import post_save +from factory import ( + Faker, + LazyAttribute, + Maybe, + SubFactory, + django, + lazy_attribute, + post_generation, +) from ..caluma_core.factories import DjangoModelFactory from . import models @@ -141,13 +150,6 @@ class Meta: model = models.Document -class FileFactory(DjangoModelFactory): - name = Faker("file_name") - - class Meta: - model = models.File - - class AnswerFactory(DjangoModelFactory): question = SubFactory(QuestionFactory) document = SubFactory(DocumentFactory) @@ -166,26 +168,44 @@ def value(self): return faker.Faker().pyint() elif self.question.type not in [ models.Question.TYPE_TABLE, - models.Question.TYPE_FILE, + models.Question.TYPE_FILES, models.Question.TYPE_DATE, ]: return faker.Faker().name() return None - file = Maybe( - "is_file", yes_declaration=SubFactory(FileFactory), no_declaration=None - ) date = Maybe("is_date", yes_declaration=Faker("date"), no_declaration=None) + @post_generation + @django.mute_signals(post_save) + def files(self, create, extracted, **kwargs): + if not create: # pragma: no cover + return + if self.question.type == models.Question.TYPE_FILES: + if extracted is not None: + self.files.set(extracted) + else: + num_files = kwargs.pop("count", 1) + self.files.set( + FileFactory.create_batch(num_files, **kwargs, answer=self) + ) + class Meta: model = models.Answer class Params: - is_file = LazyAttribute(lambda a: a.question.type == models.Question.TYPE_FILE) is_date = LazyAttribute(lambda a: a.question.type == models.Question.TYPE_DATE) +class FileFactory(DjangoModelFactory): + name = Faker("file_name") + answer = SubFactory(AnswerFactory) + + class Meta: + model = models.File + + class AnswerDocumentFactory(DjangoModelFactory): answer = SubFactory(AnswerFactory) document = SubFactory(DocumentFactory) diff --git a/caluma/caluma_form/filters.py b/caluma/caluma_form/filters.py index ac384836e..68dcbcd89 100644 --- a/caluma/caluma_form/filters.py +++ b/caluma/caluma_form/filters.py @@ -449,7 +449,7 @@ class DocumentFilterSet(MetaFilterSet): "form__name", "form__description", "answers__value", - "answers__file__name", + "answers__files__name", ) ) root_document = GlobalIDFilter(field_name="family") diff --git a/caluma/caluma_form/historical_schema.py b/caluma/caluma_form/historical_schema.py index a9ae862b5..5db5ff17d 100644 --- a/caluma/caluma_form/historical_schema.py +++ b/caluma/caluma_form/historical_schema.py @@ -9,7 +9,7 @@ from .schema import ( Answer, DateAnswer, - FileAnswer, + FilesAnswer, FloatAnswer, FormDjangoObjectType, IntegerAnswer, @@ -55,7 +55,7 @@ class Meta: class HistoricalIntegerAnswer(IntegerAnswer): class Meta: model = models.Answer.history.model - exclude = ("document", "file", "date") + exclude = ("document", "date") use_connection = False interfaces = (HistoricalAnswer, graphene.Node) @@ -63,7 +63,7 @@ class Meta: class HistoricalFloatAnswer(FloatAnswer): class Meta: model = models.Answer.history.model - exclude = ("document", "file", "date") + exclude = ("document", "date") use_connection = False interfaces = (HistoricalAnswer, graphene.Node) @@ -71,7 +71,7 @@ class Meta: class HistoricalDateAnswer(DateAnswer): class Meta: model = models.Answer.history.model - exclude = ("document", "file") + exclude = ("document",) use_connection = False interfaces = (HistoricalAnswer, graphene.Node) @@ -79,7 +79,7 @@ class Meta: class HistoricalStringAnswer(StringAnswer): class Meta: model = models.Answer.history.model - exclude = ("document", "file", "date") + exclude = ("document", "date") use_connection = False interfaces = (HistoricalAnswer, graphene.Node) @@ -87,7 +87,7 @@ class Meta: class HistoricalListAnswer(ListAnswer): class Meta: model = models.Answer.history.model - exclude = ("document", "file", "date") + exclude = ("document", "date") use_connection = False interfaces = (HistoricalAnswer, graphene.Node) @@ -97,7 +97,7 @@ class HistoricalFile(ObjectType): download_url = graphene.String() metadata = generic.GenericScalar() historical_answer = graphene.Field( - "caluma.caluma_form.historical_schema.HistoricalFileAnswer" + "caluma.caluma_form.historical_schema.HistoricalFilesAnswer" ) history_date = graphene.types.datetime.DateTime(required=True) history_user_id = graphene.String() @@ -116,8 +116,8 @@ class Meta: interfaces = (relay.Node,) -class HistoricalFileAnswer(FileAnswer): - value = graphene.Field( +class HistoricalFilesAnswer(FilesAnswer): + value = graphene.List( HistoricalFile, required=False, as_of=graphene.types.datetime.DateTime(required=True), @@ -125,9 +125,7 @@ class HistoricalFileAnswer(FileAnswer): def resolve_value(self, info, as_of, **args): # we need to use the HistoricalFile of the correct revision - return models.File.history.filter( - id=self.file_id, history_date__lte=as_of - ).first() + return models.File.history.filter(answer_id=self.id, history_date__lte=as_of) class Meta: model = models.Answer.history.model @@ -196,7 +194,7 @@ def resolve_value(self, info, as_of, *args): class Meta: model = models.Answer.history.model - exclude = ("file", "date") + exclude = ("date",) use_connection = False interfaces = (HistoricalAnswer, graphene.Node) @@ -230,7 +228,7 @@ def resolve_document_as_of(self, info, id, as_of): models.Question.TYPE_TEXTAREA: HistoricalStringAnswer, models.Question.TYPE_TEXT: HistoricalStringAnswer, models.Question.TYPE_TABLE: HistoricalTableAnswer, - models.Question.TYPE_FILE: HistoricalFileAnswer, + models.Question.TYPE_FILES: HistoricalFilesAnswer, models.Question.TYPE_DYNAMIC_CHOICE: HistoricalStringAnswer, models.Question.TYPE_DYNAMIC_MULTIPLE_CHOICE: HistoricalListAnswer, models.Question.TYPE_CALCULATED_FLOAT: HistoricalFloatAnswer, diff --git a/caluma/caluma_form/migrations/0046_file_answer_reverse_keys.py b/caluma/caluma_form/migrations/0046_file_answer_reverse_keys.py new file mode 100644 index 000000000..8bec4098d --- /dev/null +++ b/caluma/caluma_form/migrations/0046_file_answer_reverse_keys.py @@ -0,0 +1,158 @@ +# Generated by Django 3.2.13 on 2022-06-28 08:16 + +import django.db.models.deletion +from django.db import migrations, models + + +def make_reverse_link(apps, schema_editor): + Answer = apps.get_model("caluma_form.Answer") + HistoricalAnswer = apps.get_model("caluma_form.HistoricalAnswer") + for model in [Answer, HistoricalAnswer]: + file_answers = model.objects.filter(question__type="file") + for ans in file_answers.iterator(): + file = ans.file + if file: + file.answer = ans + file.save() + + +def _rename_type(from_type, to_type, model): + file_questions = model.objects.filter(type=from_type) + file_questions.update(type=to_type) + + +def rename_file_type(apps, schema_editor): + _rename_type("file", "files", apps.get_model("caluma_form.Question")) + _rename_type("file", "files", apps.get_model("caluma_form.HistoricalQuestion")) + + +def rename_file_type_reverse(apps, schema_editor): + _rename_type("files", "file", apps.get_model("caluma_form.Question")) + _rename_type("files", "file", apps.get_model("caluma_form.HistoricalQuestion")) + + +class Migration(migrations.Migration): + + dependencies = [ + ("caluma_form", "0045_simple_history"), + ] + + operations = [ + migrations.AddField( + model_name="file", + name="answer", + field=models.ForeignKey( + default=None, + null=True, + on_delete=django.db.models.deletion.CASCADE, + related_name="files", + to="caluma_form.answer", + ), + ), + migrations.AddField( + model_name="historicalfile", + name="answer", + field=models.ForeignKey( + blank=True, + db_constraint=False, + default=None, + null=True, + on_delete=django.db.models.deletion.DO_NOTHING, + related_name="+", + to="caluma_form.answer", + ), + ), + migrations.AlterField( + model_name="answer", + name="file", + field=models.OneToOneField( + blank=True, + null=True, + on_delete=django.db.models.deletion.SET_NULL, + related_name="+", + to="caluma_form.file", + ), + ), + migrations.RunPython(make_reverse_link, migrations.RunPython.noop), + migrations.RemoveField( + model_name="answer", + name="file", + ), + migrations.RemoveField( + model_name="historicalanswer", + name="file", + ), + migrations.AlterField( + model_name="historicalanswer", + name="history_question_type", + field=models.CharField( + choices=[ + ("multiple_choice", "multiple_choice"), + ("integer", "integer"), + ("float", "float"), + ("date", "date"), + ("choice", "choice"), + ("textarea", "textarea"), + ("text", "text"), + ("table", "table"), + ("form", "form"), + ("files", "files"), + ("dynamic_choice", "dynamic_choice"), + ("dynamic_multiple_choice", "dynamic_multiple_choice"), + ("static", "static"), + ("calculated_float", "calculated_float"), + ("action_button", "action_button"), + ], + max_length=23, + ), + ), + migrations.AlterField( + model_name="historicalquestion", + name="type", + field=models.CharField( + choices=[ + ("multiple_choice", "multiple_choice"), + ("integer", "integer"), + ("float", "float"), + ("date", "date"), + ("choice", "choice"), + ("textarea", "textarea"), + ("text", "text"), + ("table", "table"), + ("form", "form"), + ("files", "files"), + ("dynamic_choice", "dynamic_choice"), + ("dynamic_multiple_choice", "dynamic_multiple_choice"), + ("static", "static"), + ("calculated_float", "calculated_float"), + ("action_button", "action_button"), + ], + max_length=23, + ), + ), + migrations.AlterField( + model_name="question", + name="type", + field=models.CharField( + choices=[ + ("multiple_choice", "multiple_choice"), + ("integer", "integer"), + ("float", "float"), + ("date", "date"), + ("choice", "choice"), + ("textarea", "textarea"), + ("text", "text"), + ("table", "table"), + ("form", "form"), + ("files", "files"), + ("dynamic_choice", "dynamic_choice"), + ("dynamic_multiple_choice", "dynamic_multiple_choice"), + ("static", "static"), + ("calculated_float", "calculated_float"), + ("action_button", "action_button"), + ], + max_length=23, + ), + ), + migrations.RunPython(rename_file_type, rename_file_type_reverse), + ] diff --git a/caluma/caluma_form/models.py b/caluma/caluma_form/models.py index ae6b2afcc..b5d6d1ac7 100644 --- a/caluma/caluma_form/models.py +++ b/caluma/caluma_form/models.py @@ -58,7 +58,7 @@ class Question(core_models.SlugModel): TYPE_TEXT = "text" TYPE_TABLE = "table" TYPE_FORM = "form" - TYPE_FILE = "file" + TYPE_FILES = "files" TYPE_DYNAMIC_CHOICE = "dynamic_choice" TYPE_DYNAMIC_MULTIPLE_CHOICE = "dynamic_multiple_choice" TYPE_STATIC = "static" @@ -75,7 +75,7 @@ class Question(core_models.SlugModel): TYPE_TEXT, TYPE_TABLE, TYPE_FORM, - TYPE_FILE, + TYPE_FILES, TYPE_DYNAMIC_CHOICE, TYPE_DYNAMIC_MULTIPLE_CHOICE, TYPE_STATIC, @@ -406,9 +406,6 @@ class Answer(core_models.BaseModel): Document, through="AnswerDocument", related_name="+" ) date = models.DateField(null=True, blank=True) - file = models.OneToOneField( - "File", on_delete=models.SET_NULL, null=True, blank=True - ) # override history to add extra fields on historical model history = HistoricalRecords( @@ -420,8 +417,10 @@ class Answer(core_models.BaseModel): ) def delete(self, *args, **kwargs): - if self.file: - self.file.delete() + # Files need to be deleted in sequence (not via on_delete) + # so the deletion code on the storage is properly triggered + for file in self.files.all(): + file.delete() super().delete(args, kwargs) def create_answer_documents(self, documents): @@ -465,9 +464,9 @@ def copy(self, document_family=None, to_document=None, user=None): }, ) - if self.question.type == Question.TYPE_FILE: - new_answer.file = self.file.copy() - new_answer.save() + if self.question.type == Question.TYPE_FILES: + for file in self.files.all(): + file.copy(to_answer=new_answer) if self.question.type in [ Question.TYPE_DYNAMIC_CHOICE, @@ -571,11 +570,18 @@ def wrapper(self, *args, **kwargs): class File(core_models.UUIDModel): name = models.CharField(max_length=255) + answer = models.ForeignKey( + Answer, on_delete=models.CASCADE, related_name="files", null=True, default=None + ) + @_ignore_missing_file def _move_blob(self): # move the file on update # this makes sure it stays available when querying the history old_file = self.history.first() + if not old_file: # pragma: no cover + # No history - cannot move object to preserve history. + return new_name = f"{old_file.pk}_{old_file.name}" client.move_object(self.object_name, new_name) @@ -583,11 +589,16 @@ def _move_blob(self): def _copy(self, new_object_name): client.copy_object(self.object_name, new_object_name) - def copy(self): - copy = File.objects.create(name=self.name) + def copy(self, to_answer): + copy = File.objects.create(name=self.name, answer=to_answer or self.answer) self._copy(copy.object_name) return copy + def rename(self, new_name): + from_object_name = self.object_name + self.name = new_name + client.move_object(from_object_name, self.object_name) + def delete(self, *args, **kwargs): self._move_blob() super().delete(*args, **kwargs) diff --git a/caluma/caluma_form/ordering.py b/caluma/caluma_form/ordering.py index 406330a20..28815c1ce 100644 --- a/caluma/caluma_form/ordering.py +++ b/caluma/caluma_form/ordering.py @@ -39,7 +39,7 @@ def get_ordering_value( Question.TYPE_CHOICE: "value", Question.TYPE_TEXTAREA: "value", Question.TYPE_TEXT: "value", - Question.TYPE_FILE: "file__name", + Question.TYPE_FILES: "files__name", Question.TYPE_DYNAMIC_CHOICE: "value", } diff --git a/caluma/caluma_form/schema.py b/caluma/caluma_form/schema.py index c14e0710e..2d8800408 100644 --- a/caluma/caluma_form/schema.py +++ b/caluma/caluma_form/schema.py @@ -495,7 +495,7 @@ class Meta: interfaces = (Question, graphene.Node) -class FileQuestion(QuestionQuerysetMixin, FormDjangoObjectType): +class FilesQuestion(QuestionQuerysetMixin, FormDjangoObjectType): hint_text = graphene.String() class Meta: @@ -735,7 +735,7 @@ class Meta: return_field_type = Question -class SaveFileQuestion(SaveQuestion): +class SaveFilesQuestion(SaveQuestion): class Meta: serializer_class = serializers.SaveFileQuestionSerializer return_field_type = Question @@ -799,7 +799,7 @@ class IntegerAnswer(AnswerQuerysetMixin, FormDjangoObjectType): class Meta: model = models.Answer - exclude = ("document", "documents", "file", "date") + exclude = ("document", "documents", "files", "date") use_connection = False interfaces = (Answer, graphene.Node) @@ -809,7 +809,7 @@ class FloatAnswer(AnswerQuerysetMixin, FormDjangoObjectType): class Meta: model = models.Answer - exclude = ("document", "documents", "file", "date") + exclude = ("document", "documents", "files", "date") use_connection = False interfaces = (Answer, graphene.Node) @@ -822,7 +822,7 @@ def resolve_value(self, info, **args): class Meta: model = models.Answer - exclude = ("document", "documents", "file") + exclude = ("document", "documents", "files") use_connection = False interfaces = (Answer, graphene.Node) @@ -847,7 +847,7 @@ def resolve_selected_option(self, info, **args): class Meta: model = models.Answer - exclude = ("document", "documents", "file", "date") + exclude = ("document", "documents", "files", "date") use_connection = False interfaces = (Answer, graphene.Node) @@ -858,7 +858,7 @@ class ListAnswer(AnswerQuerysetMixin, FormDjangoObjectType): class Meta: model = models.Answer - exclude = ("document", "documents", "file", "date") + exclude = ("document", "documents", "files", "date") use_connection = False interfaces = (Answer, graphene.Node) @@ -896,7 +896,7 @@ def resolve_value(self, info, **args): class Meta: model = models.Answer - exclude = ("documents", "file", "date") + exclude = ("documents", "files", "date") use_connection = False interfaces = (Answer, graphene.Node) @@ -906,7 +906,6 @@ class File(FormDjangoObjectType): upload_url = graphene.String() download_url = graphene.String() metadata = generic.GenericScalar() - answer = graphene.Field("caluma.caluma_form.schema.FileAnswer") class Meta: model = models.File @@ -914,15 +913,15 @@ class Meta: fields = "__all__" -class FileAnswer(AnswerQuerysetMixin, FormDjangoObjectType): - value = graphene.Field(File, required=True) +class FilesAnswer(AnswerQuerysetMixin, FormDjangoObjectType): + value = graphene.List(File, required=True) def resolve_value(self, info, **args): - return self.file + return self.files.all() class Meta: model = models.Answer - exclude = ("document", "documents", "date") + exclude = ("document", "documents", "date", "files") use_connection = False interfaces = (Answer, graphene.Node) @@ -992,9 +991,17 @@ class Meta: return_field_type = Answer -class SaveDocumentFileAnswer(SaveDocumentAnswer): +class SaveFile(graphene.InputObjectType): + id = graphene.String() + name = graphene.String() + + +class SaveDocumentFilesAnswer(SaveDocumentAnswer): + class Input: + value = graphene.List(SaveFile, required=False) + class Meta: - serializer_class = serializers.SaveDocumentFileAnswerSerializer + serializer_class = serializers.SaveDocumentFilesAnswerSerializer return_field_type = Answer @@ -1087,7 +1094,7 @@ class Mutation(object): save_integer_question = SaveIntegerQuestion().Field() save_table_question = SaveTableQuestion().Field() save_form_question = SaveFormQuestion().Field() - save_file_question = SaveFileQuestion().Field() + save_files_question = SaveFilesQuestion().Field() save_static_question = SaveStaticQuestion().Field() save_calculated_float_question = SaveCalculatedFloatQuestion().Field() save_action_button_question = SaveActionButtonQuestion().Field() @@ -1100,7 +1107,7 @@ class Mutation(object): save_document_date_answer = SaveDocumentDateAnswer().Field() save_document_list_answer = SaveDocumentListAnswer().Field() save_document_table_answer = SaveDocumentTableAnswer().Field() - save_document_file_answer = SaveDocumentFileAnswer().Field() + save_document_files_answer = SaveDocumentFilesAnswer().Field() save_default_string_answer = SaveDefaultStringAnswer().Field() save_default_integer_answer = SaveDefaultIntegerAnswer().Field() @@ -1200,7 +1207,7 @@ def resolve_document_validity(self, info, id, **kwargs): models.Question.TYPE_TEXTAREA: StringAnswer, models.Question.TYPE_TEXT: StringAnswer, models.Question.TYPE_TABLE: TableAnswer, - models.Question.TYPE_FILE: FileAnswer, + models.Question.TYPE_FILES: FilesAnswer, models.Question.TYPE_DYNAMIC_CHOICE: StringAnswer, models.Question.TYPE_DYNAMIC_MULTIPLE_CHOICE: ListAnswer, models.Question.TYPE_CALCULATED_FLOAT: FloatAnswer, @@ -1218,7 +1225,7 @@ def resolve_document_validity(self, info, id, **kwargs): models.Question.TYPE_DATE: DateQuestion, models.Question.TYPE_TABLE: TableQuestion, models.Question.TYPE_FORM: FormQuestion, - models.Question.TYPE_FILE: FileQuestion, + models.Question.TYPE_FILES: FilesQuestion, models.Question.TYPE_STATIC: StaticQuestion, models.Question.TYPE_CALCULATED_FLOAT: CalculatedFloatQuestion, models.Question.TYPE_ACTION_BUTTON: ActionButtonQuestion, diff --git a/caluma/caluma_form/serializers.py b/caluma/caluma_form/serializers.py index bfebcfa4b..b660d1061 100644 --- a/caluma/caluma_form/serializers.py +++ b/caluma/caluma_form/serializers.py @@ -423,7 +423,7 @@ class Meta(SaveQuestionSerializer.Meta): class SaveFileQuestionSerializer(SaveQuestionSerializer): def validate(self, data): - data["type"] = models.Question.TYPE_FILE + data["type"] = models.Question.TYPE_FILES return super().validate(data) class Meta(SaveQuestionSerializer.Meta): @@ -593,14 +593,15 @@ class Meta(SaveAnswerSerializer.Meta): pass -class SaveDocumentFileAnswerSerializer(SaveAnswerSerializer): - value = CharField(write_only=True, source="file", required=False) - value_id = PrimaryKeyRelatedField(read_only=True, source="file", required=False) +class SaveDocumentFilesAnswerSerializer(SaveAnswerSerializer): + value = ListField( + source="files", + required=False, + help_text="List of files for this answer", + ) class Meta(SaveAnswerSerializer.Meta): - fields = SaveAnswerSerializer.Meta.fields + [ - "value_id", - ] + fields = SaveAnswerSerializer.Meta.fields class RemoveAnswerSerializer(serializers.ModelSerializer): diff --git a/caluma/caluma_form/structure.py b/caluma/caluma_form/structure.py index 4eefabec7..80206bed2 100644 --- a/caluma/caluma_form/structure.py +++ b/caluma/caluma_form/structure.py @@ -91,8 +91,8 @@ def value(self): for row in self.children() ] - elif self.question.type == Question.TYPE_FILE: - return self.answer.file + elif self.question.type == Question.TYPE_FILES: + return [f.name for f in self.answer.files.all()] elif self.question.type == Question.TYPE_DATE: return self.answer.date diff --git a/caluma/caluma_form/tests/__snapshots__/test_document.ambr b/caluma/caluma_form/tests/__snapshots__/test_document.ambr index bb1949fa5..c2bb1daea 100644 --- a/caluma/caluma_form/tests/__snapshots__/test_document.ambr +++ b/caluma/caluma_form/tests/__snapshots__/test_document.ambr @@ -67,7 +67,7 @@ }), }) # --- -# name: test_query_all_documents[file-None-some-file.pdf-None] +# name: test_query_all_documents[files-None-answer__value6-None] dict({ 'allDocuments': dict({ 'edges': list([ @@ -77,31 +77,33 @@ 'edges': list([ dict({ 'node': dict({ - '__typename': 'FileAnswer', - 'fileValue': dict({ - 'downloadUrl': 'http://minio/download-url/09c697fb-fd0a-4345-bb9c-99df350b0cdb_some-file.pdf', - 'metadata': dict({ - 'bucket_name': 'caluma-media', - 'content_type': 'application/pdf', - 'etag': '5d41402abc4b2a76b9719d911017c592', - 'last_modified': '2021-03-05T15:24:33+00:00', + '__typename': 'FilesAnswer', + 'fileValue': list([ + dict({ + 'downloadUrl': "http://minio/download-url/09c697fb-fd0a-4345-bb9c-99df350b0cdb_[{'name': 'some-file.pdf'}]", 'metadata': dict({ - 'Accept-Ranges': 'bytes', - 'Content-Length': '5', - 'Content-Security-Policy': 'block-all-mixed-content', - 'Content-Type': 'binary/octet-stream', - 'Date': 'Fri, 05 Mar 2021 15:25:15 GMT', - 'ETag': '"5d41402abc4b2a76b9719d911017c592"', - 'Last-Modified': 'Fri, 05 Mar 2021 15:24:33 GMT', - 'Server': 'MinIO', - 'Vary': 'Origin', - 'X-Amz-Request-Id': '16697BAAD69D2214', - 'X-Xss-Protection': '1; mode=block', + 'bucket_name': 'caluma-media', + 'content_type': 'application/pdf', + 'etag': '5d41402abc4b2a76b9719d911017c592', + 'last_modified': '2021-03-05T15:24:33+00:00', + 'metadata': dict({ + 'Accept-Ranges': 'bytes', + 'Content-Length': '5', + 'Content-Security-Policy': 'block-all-mixed-content', + 'Content-Type': 'binary/octet-stream', + 'Date': 'Fri, 05 Mar 2021 15:25:15 GMT', + 'ETag': '"5d41402abc4b2a76b9719d911017c592"', + 'Last-Modified': 'Fri, 05 Mar 2021 15:24:33 GMT', + 'Server': 'MinIO', + 'Vary': 'Origin', + 'X-Amz-Request-Id': '16697BAAD69D2214', + 'X-Xss-Protection': '1; mode=block', + }), + 'size': 8200, }), - 'size': 8200, + 'name': "[{'name': 'some-file.pdf'}]", }), - 'name': 'some-file.pdf', - }), + ]), 'question': dict({ 'label': 'Taylor Williams', 'slug': 'suggest-adult-allow', @@ -119,7 +121,7 @@ }), }) # --- -# name: test_query_all_documents[file-None-some-other-file.pdf-None] +# name: test_query_all_documents[files-None-answer__value7-None] dict({ 'allDocuments': dict({ 'edges': list([ @@ -129,31 +131,33 @@ 'edges': list([ dict({ 'node': dict({ - '__typename': 'FileAnswer', - 'fileValue': dict({ - 'downloadUrl': 'http://minio/download-url/09c697fb-fd0a-4345-bb9c-99df350b0cdb_some-other-file.pdf', - 'metadata': dict({ - 'bucket_name': 'caluma-media', - 'content_type': 'application/pdf', - 'etag': '5d41402abc4b2a76b9719d911017c592', - 'last_modified': '2021-03-05T15:24:33+00:00', + '__typename': 'FilesAnswer', + 'fileValue': list([ + dict({ + 'downloadUrl': "http://minio/download-url/09c697fb-fd0a-4345-bb9c-99df350b0cdb_[{'name': 'some-other-file.pdf'}]", 'metadata': dict({ - 'Accept-Ranges': 'bytes', - 'Content-Length': '5', - 'Content-Security-Policy': 'block-all-mixed-content', - 'Content-Type': 'binary/octet-stream', - 'Date': 'Fri, 05 Mar 2021 15:25:15 GMT', - 'ETag': '"5d41402abc4b2a76b9719d911017c592"', - 'Last-Modified': 'Fri, 05 Mar 2021 15:24:33 GMT', - 'Server': 'MinIO', - 'Vary': 'Origin', - 'X-Amz-Request-Id': '16697BAAD69D2214', - 'X-Xss-Protection': '1; mode=block', + 'bucket_name': 'caluma-media', + 'content_type': 'application/pdf', + 'etag': '5d41402abc4b2a76b9719d911017c592', + 'last_modified': '2021-03-05T15:24:33+00:00', + 'metadata': dict({ + 'Accept-Ranges': 'bytes', + 'Content-Length': '5', + 'Content-Security-Policy': 'block-all-mixed-content', + 'Content-Type': 'binary/octet-stream', + 'Date': 'Fri, 05 Mar 2021 15:25:15 GMT', + 'ETag': '"5d41402abc4b2a76b9719d911017c592"', + 'Last-Modified': 'Fri, 05 Mar 2021 15:24:33 GMT', + 'Server': 'MinIO', + 'Vary': 'Origin', + 'X-Amz-Request-Id': '16697BAAD69D2214', + 'X-Xss-Protection': '1; mode=block', + }), + 'size': 8200, }), - 'size': 8200, + 'name': "[{'name': 'some-other-file.pdf'}]", }), - 'name': 'some-other-file.pdf', - }), + ]), 'question': dict({ 'label': 'Taylor Williams', 'slug': 'suggest-adult-allow', @@ -417,73 +421,81 @@ # name: test_save_document_answer[dynamic_multiple_choice-question__configuration19-MyDataSource-question__format_validators19-answer__value19-None-SaveDocumentListAnswer-True-option-slug-True-True] Answer(document=Document(form=Form(slug=eight-traditional)), question=Question(slug=environmental-ten, type=dynamic_multiple_choice), value=['5.5', '1']) # --- -# name: test_save_document_answer[file-question__configuration10-None-question__format_validators10-not-exist.pdf-None-SaveDocumentFileAnswer-True-option-slug-False-False] +# name: test_save_document_answer[files-question__configuration10-None-question__format_validators10-answer__value10-None-SaveDocumentFilesAnswer-True-option-slug-False-False] dict({ - 'saveDocumentFileAnswer': dict({ + 'saveDocumentFilesAnswer': dict({ 'answer': dict({ - '__typename': 'FileAnswer', - 'fileValue': dict({ - 'name': 'not-exist.pdf', - 'uploadUrl': 'http://minio/upload-url', - }), + '__typename': 'FilesAnswer', + 'fileValue': list([ + dict({ + 'name': 'not-exist.pdf', + 'uploadUrl': 'http://minio/upload-url', + }), + ]), }), 'clientMutationId': 'testid', }), }) # --- -# name: test_save_document_answer[file-question__configuration10-None-question__format_validators10-not-exist.pdf-None-SaveDocumentFileAnswer-True-option-slug-False-True] - Answer(document=Document(form=Form(slug=eight-traditional)), question=Question(slug=environmental-ten, type=file), value=None) +# name: test_save_document_answer[files-question__configuration10-None-question__format_validators10-answer__value10-None-SaveDocumentFilesAnswer-True-option-slug-False-True] + Answer(document=Document(form=Form(slug=eight-traditional)), question=Question(slug=environmental-ten, type=files), value=None) # --- -# name: test_save_document_answer[file-question__configuration10-None-question__format_validators10-not-exist.pdf-None-SaveDocumentFileAnswer-True-option-slug-True-False] +# name: test_save_document_answer[files-question__configuration10-None-question__format_validators10-answer__value10-None-SaveDocumentFilesAnswer-True-option-slug-True-False] dict({ - 'saveDocumentFileAnswer': dict({ + 'saveDocumentFilesAnswer': dict({ 'answer': dict({ - '__typename': 'FileAnswer', - 'fileValue': dict({ - 'name': 'not-exist.pdf', - 'uploadUrl': 'http://minio/upload-url', - }), + '__typename': 'FilesAnswer', + 'fileValue': list([ + dict({ + 'name': 'not-exist.pdf', + 'uploadUrl': 'http://minio/upload-url', + }), + ]), }), 'clientMutationId': 'testid', }), }) # --- -# name: test_save_document_answer[file-question__configuration10-None-question__format_validators10-not-exist.pdf-None-SaveDocumentFileAnswer-True-option-slug-True-True] - Answer(document=Document(form=Form(slug=eight-traditional)), question=Question(slug=environmental-ten, type=file), value=None) +# name: test_save_document_answer[files-question__configuration10-None-question__format_validators10-answer__value10-None-SaveDocumentFilesAnswer-True-option-slug-True-True] + Answer(document=Document(form=Form(slug=eight-traditional)), question=Question(slug=environmental-ten, type=files), value=None) # --- -# name: test_save_document_answer[file-question__configuration9-None-question__format_validators9-some-file.pdf-None-SaveDocumentFileAnswer-True-option-slug-False-False] +# name: test_save_document_answer[files-question__configuration9-None-question__format_validators9-answer__value9-None-SaveDocumentFilesAnswer-True-option-slug-False-False] dict({ - 'saveDocumentFileAnswer': dict({ + 'saveDocumentFilesAnswer': dict({ 'answer': dict({ - '__typename': 'FileAnswer', - 'fileValue': dict({ - 'name': 'some-file.pdf', - 'uploadUrl': 'http://minio/upload-url', - }), + '__typename': 'FilesAnswer', + 'fileValue': list([ + dict({ + 'name': 'some-file.pdf', + 'uploadUrl': 'http://minio/upload-url', + }), + ]), }), 'clientMutationId': 'testid', }), }) # --- -# name: test_save_document_answer[file-question__configuration9-None-question__format_validators9-some-file.pdf-None-SaveDocumentFileAnswer-True-option-slug-False-True] - Answer(document=Document(form=Form(slug=eight-traditional)), question=Question(slug=environmental-ten, type=file), value=None) +# name: test_save_document_answer[files-question__configuration9-None-question__format_validators9-answer__value9-None-SaveDocumentFilesAnswer-True-option-slug-False-True] + Answer(document=Document(form=Form(slug=eight-traditional)), question=Question(slug=environmental-ten, type=files), value=None) # --- -# name: test_save_document_answer[file-question__configuration9-None-question__format_validators9-some-file.pdf-None-SaveDocumentFileAnswer-True-option-slug-True-False] +# name: test_save_document_answer[files-question__configuration9-None-question__format_validators9-answer__value9-None-SaveDocumentFilesAnswer-True-option-slug-True-False] dict({ - 'saveDocumentFileAnswer': dict({ + 'saveDocumentFilesAnswer': dict({ 'answer': dict({ - '__typename': 'FileAnswer', - 'fileValue': dict({ - 'name': 'some-file.pdf', - 'uploadUrl': 'http://minio/upload-url', - }), + '__typename': 'FilesAnswer', + 'fileValue': list([ + dict({ + 'name': 'some-file.pdf', + 'uploadUrl': 'http://minio/upload-url', + }), + ]), }), 'clientMutationId': 'testid', }), }) # --- -# name: test_save_document_answer[file-question__configuration9-None-question__format_validators9-some-file.pdf-None-SaveDocumentFileAnswer-True-option-slug-True-True] - Answer(document=Document(form=Form(slug=eight-traditional)), question=Question(slug=environmental-ten, type=file), value=None) +# name: test_save_document_answer[files-question__configuration9-None-question__format_validators9-answer__value9-None-SaveDocumentFilesAnswer-True-option-slug-True-True] + Answer(document=Document(form=Form(slug=eight-traditional)), question=Question(slug=environmental-ten, type=files), value=None) # --- # name: test_save_document_answer[float-question__configuration2-None-question__format_validators2-2.1-None-SaveDocumentFloatAnswer-True-option-slug-False-False] dict({ diff --git a/caluma/caluma_form/tests/__snapshots__/test_history.ambr b/caluma/caluma_form/tests/__snapshots__/test_history.ambr index 88dc7720a..f4e563e57 100644 --- a/caluma/caluma_form/tests/__snapshots__/test_history.ambr +++ b/caluma/caluma_form/tests/__snapshots__/test_history.ambr @@ -72,7 +72,7 @@ 'edges': list([ dict({ 'node': dict({ - 'historyType': '+', + 'historyType': '~', 'value': 'first row value', }), }), @@ -84,7 +84,7 @@ 'edges': list([ dict({ 'node': dict({ - 'historyType': '+', + 'historyType': '~', 'value': 'second row value', }), }), @@ -109,7 +109,7 @@ 'edges': list([ dict({ 'node': dict({ - 'historyType': '+', + 'historyType': '~', 'value': 'first row value', }), }), diff --git a/caluma/caluma_form/tests/__snapshots__/test_question.ambr b/caluma/caluma_form/tests/__snapshots__/test_question.ambr index 43ab5cf5e..3ce9bd3f4 100644 --- a/caluma/caluma_form/tests/__snapshots__/test_question.ambr +++ b/caluma/caluma_form/tests/__snapshots__/test_question.ambr @@ -214,15 +214,15 @@ }), }) # --- -# name: test_query_all_questions[file-question__configuration9-None-question__format_validators9] +# name: test_query_all_questions[files-question__configuration9-None-question__format_validators9] dict({ 'allQuestions': dict({ 'edges': list([ dict({ 'node': dict({ - '__typename': 'FileQuestion', + '__typename': 'FilesQuestion', 'hintText': '', - 'id': 'RmlsZVF1ZXN0aW9uOmVudmlyb25tZW50YWwtdGVu', + 'id': 'RmlsZXNRdWVzdGlvbjplbnZpcm9ubWVudGFsLXRlbg==', 'infoText': '', 'label': 'Bonnie Moreno', 'meta': dict({ @@ -987,13 +987,13 @@ }), }) # --- -# name: test_save_question[true-True-SaveFileQuestion] +# name: test_save_question[true-True-SaveFilesQuestion] dict({ - 'saveFileQuestion': dict({ + 'saveFilesQuestion': dict({ 'clientMutationId': 'testid', 'question': dict({ - '__typename': 'FileQuestion', - 'id': 'RmlsZVF1ZXN0aW9uOmVudmlyb25tZW50YWwtdGVu', + '__typename': 'FilesQuestion', + 'id': 'RmlsZXNRdWVzdGlvbjplbnZpcm9ubWVudGFsLXRlbg==', 'label': 'Bonnie Moreno', 'meta': dict({ }), diff --git a/caluma/caluma_form/tests/test_answer.py b/caluma/caluma_form/tests/test_answer.py index bb2a59bf9..59ac2c67d 100644 --- a/caluma/caluma_form/tests/test_answer.py +++ b/caluma/caluma_form/tests/test_answer.py @@ -93,7 +93,7 @@ def test_remove_default_answer(db, snapshot, question, answer, schema_executor): False, ), (Question.TYPE_DATE, None, "2019-02-22", "SaveDefaultDateAnswer", True), - (Question.TYPE_FILE, None, None, "SaveDefaultFileAnswer", False), + (Question.TYPE_FILES, None, None, "SaveDefaultFilesAnswer", False), (Question.TYPE_TABLE, None, None, "SaveDefaultTableAnswer", True), ], ) @@ -200,7 +200,7 @@ def test_save_default_answer_graphql( (Question.TYPE_DYNAMIC_CHOICE, "5.5", None, False), (Question.TYPE_DYNAMIC_MULTIPLE_CHOICE, [], None, False), (Question.TYPE_DATE, None, "2019-02-22", True), - (Question.TYPE_FILE, None, None, False), + (Question.TYPE_FILES, None, None, False), (Question.TYPE_TABLE, None, None, True), ], ) @@ -286,7 +286,7 @@ def test_delete_question_with_default(db, question, answer): with pytest.raises(models.Answer.DoesNotExist): answer.refresh_from_db() - assert models.Answer.history.count() == 2 + assert models.Answer.history.count() == 3 assert all( h.history_question_type == question.type for h in models.Answer.history.all() ) @@ -334,12 +334,12 @@ def validate(self, mutation, data, info): ) -@pytest.mark.parametrize("question__type", ["file"]) +@pytest.mark.parametrize("question__type", ["files"]) def test_file_answer_metadata(db, answer, schema_executor, minio_mock): query = """ query ans($id: ID!) { node(id:$id) { - ... on FileAnswer { + ... on FilesAnswer { fileValue: value { name downloadUrl @@ -349,7 +349,7 @@ def test_file_answer_metadata(db, answer, schema_executor, minio_mock): } } """ - vars = {"id": to_global_id("FileAnswer", str(answer.pk))} + vars = {"id": to_global_id("FilesAnswer", str(answer.pk))} # before "upload" old_stat = minio_mock.stat_object.return_value @@ -365,7 +365,7 @@ def test_file_answer_metadata(db, answer, schema_executor, minio_mock): # Before upload, no metadata is available result = schema_executor(query, variable_values=vars) assert not result.errors - assert result.data["node"]["fileValue"]["metadata"] is None + assert result.data["node"]["fileValue"][0]["metadata"] is None # After "upload", metadata should contain some useful data minio_mock.stat_object.return_value = old_stat @@ -376,7 +376,7 @@ def test_file_answer_metadata(db, answer, schema_executor, minio_mock): # Make sure all the values (especially in metadata) are JSON serializable. # This is something the schema_executor doesn't test, but may bite us in prod - metadata = result.data["node"]["fileValue"]["metadata"] + metadata = result.data["node"]["fileValue"][0]["metadata"] assert json.dumps(metadata) # Ensure some of the important properties having the right types, @@ -385,3 +385,131 @@ def test_file_answer_metadata(db, answer, schema_executor, minio_mock): assert isinstance(metadata["size"], int) assert all(isinstance(m, str) for m in metadata["metadata"].values()) assert datetime.fromisoformat(metadata["last_modified"]) + + +SAVE_DOCUMENT_FILES_ANSWER_QUERY = """ + mutation save ($input: SaveDocumentFilesAnswerInput!) { + saveDocumentFilesAnswer (input: $input) { + answer { + ... on FilesAnswer { + value { + name + uploadUrl + id + } + } + } + } + } +""" + + +@pytest.mark.parametrize("answer__files", [[]]) +@pytest.mark.parametrize("question__type", ["files"]) +def test_file_answer_mutation_create(db, answer, document, schema_executor, minio_mock): + # Precondition check + assert answer.files.count() == 0 + + # Initial upload + result = schema_executor( + SAVE_DOCUMENT_FILES_ANSWER_QUERY, + variable_values={ + "input": { + "document": str(document.pk), + "question": answer.question.slug, + "value": [{"name": "some test file.txt"}], + } + }, + ) + assert not result.errors + assert answer.files.count() == 1 + first_file = answer.files.get() + assert first_file.name == "some test file.txt" + + +@pytest.mark.parametrize("question__type", ["files"]) +def test_file_answer_mutation_add_file( + db, answer, document, schema_executor, minio_mock +): + + first_file = answer.files.get() + history_count_before = first_file.history.count() + + # Second request: adding a file + result = schema_executor( + SAVE_DOCUMENT_FILES_ANSWER_QUERY, + variable_values={ + "input": { + "document": str(document.pk), + "question": answer.question.slug, + "value": [ + {"name": first_file.name, "id": str(first_file.pk)}, + {"name": "another file.txt"}, + ], + } + }, + ) + assert not result.errors + assert answer.files.count() == 2 + + second_file = answer.files.exclude(pk=first_file.pk).get() + assert second_file.name == "another file.txt" + # Check that previously-created file wasn't replaced or changed + assert first_file in answer.files.all() + assert history_count_before == first_file.history.count() + + +@pytest.mark.parametrize("question__type", ["files"]) +def test_file_answer_mutation_remove_file( + db, answer, document, schema_executor, minio_mock +): + # Add a file while removing another (replacing the full + # files set of an answer) + assert answer.files.count() == 1 + orig_file = answer.files.get() + result = schema_executor( + SAVE_DOCUMENT_FILES_ANSWER_QUERY, + variable_values={ + "input": { + "document": str(document.pk), + "question": answer.question.slug, + "value": [ + {"name": "another file.txt"}, + ], + } + }, + ) + assert not result.errors + assert answer.files.count() == 1 + with pytest.raises(type(orig_file).DoesNotExist): + # This file should be gone + orig_file.refresh_from_db() + + +@pytest.mark.parametrize("question__type", ["files"]) +def test_file_answer_mutation_update_missing_file( + db, answer, document, schema_executor, minio_mock +): + # Add a file while removing another (replacing the full + # files set of an answer) + orig_file = answer.files.get() + orig_file_id = str(orig_file.pk) + orig_file.delete() + + result = schema_executor( + SAVE_DOCUMENT_FILES_ANSWER_QUERY, + variable_values={ + "input": { + "document": str(document.pk), + "question": answer.question.slug, + "value": [ + {"name": "another file.txt", "id": orig_file_id}, + ], + } + }, + ) + assert result.errors + assert f"File with id={orig_file_id} for given answer not found" in str( + result.errors[0].args[0] + ) + assert answer.files.count() == 0 diff --git a/caluma/caluma_form/tests/test_document.py b/caluma/caluma_form/tests/test_document.py index 56a1e64e5..1e2eb682a 100644 --- a/caluma/caluma_form/tests/test_document.py +++ b/caluma/caluma_form/tests/test_document.py @@ -19,8 +19,8 @@ (Question.TYPE_MULTIPLE_CHOICE, None, ["somevalue", "anothervalue"], None), (Question.TYPE_TABLE, None, None, None), (Question.TYPE_DATE, None, None, "2019-02-22"), - (Question.TYPE_FILE, None, "some-file.pdf", None), - (Question.TYPE_FILE, None, "some-other-file.pdf", None), + (Question.TYPE_FILES, None, [{"name": "some-file.pdf"}], None), + (Question.TYPE_FILES, None, [{"name": "some-other-file.pdf"}], None), (Question.TYPE_DYNAMIC_CHOICE, "MyDataSource", "5.5", None), (Question.TYPE_DYNAMIC_MULTIPLE_CHOICE, "MyDataSource", ["5.5"], None), ], @@ -82,7 +82,7 @@ def test_query_all_documents( } } } - ... on FileAnswer { + ... on FilesAnswer { fileValue: value { name downloadUrl @@ -98,19 +98,33 @@ def test_query_all_documents( } """ - search = isinstance(answer.value, list) and " ".join(answer.value) or answer.value + def _value(val): + # Extract searchable value from given input if it exists + ret = { + list: lambda: _value(val[0]), + dict: lambda: _value(list(val.keys())[0]), + } + return ret.get(type(val), lambda: val)() + + search = _value(answer.value) - if question.type == Question.TYPE_FILE: - if answer.value == "some-other-file.pdf": + if question.type == Question.TYPE_FILES: + if answer.value[0]["name"] == "some-other-file.pdf": settings.MINIO_STORAGE_AUTO_CREATE_MEDIA_BUCKET = False minio_mock.bucket_exists.return_value = False # we need to set the pk here in order to match the snapshots - answer.file = file_factory( - name=answer.value, pk="09c697fb-fd0a-4345-bb9c-99df350b0cdb" + answer.files.set( + [ + file_factory( + name=answer.value, + pk="09c697fb-fd0a-4345-bb9c-99df350b0cdb", + answer=answer, + ) + ] ) answer.value = None answer.save() - search = answer.file.name + search = answer.files.get().name result = schema_executor(query, variable_values={"search": str(search)}) assert not result.errors @@ -138,10 +152,10 @@ def test_complex_document_query_performance( form_question_factory(question=multiple_choice_question, form=form) question_option_factory.create_batch(10, question=multiple_choice_question) answer_factory(question=multiple_choice_question) - file_question = question_factory(type=Question.TYPE_FILE) + file_question = question_factory(type=Question.TYPE_FILES) form_question_factory(question=file_question, form=form) answer_factory( - question=file_question, value=None, document=document, file=file_factory() + question=file_question, value=None, document=document, files=[file_factory()] ) form_question = question_factory(type=Question.TYPE_FORM) @@ -203,7 +217,7 @@ def test_complex_document_query_performance( ... on ListAnswer { listValue: value } - ... on FileAnswer { + ... on FilesAnswer { fileValue: value { name downloadUrl @@ -539,25 +553,34 @@ def test_save_document( "SaveDocumentDateAnswer", True, ), - (Question.TYPE_FILE, {}, None, [], None, None, "SaveDocumentFileAnswer", False), ( - Question.TYPE_FILE, + Question.TYPE_FILES, + {}, + None, + [], + None, + None, + "SaveDocumentFilesAnswer", + False, + ), + ( + Question.TYPE_FILES, {}, None, [], - "some-file.pdf", + [{"name": "some-file.pdf"}], None, - "SaveDocumentFileAnswer", + "SaveDocumentFilesAnswer", True, ), ( - Question.TYPE_FILE, + Question.TYPE_FILES, {}, None, [], - "not-exist.pdf", + [{"name": "not-exist.pdf"}], None, - "SaveDocumentFileAnswer", + "SaveDocumentFilesAnswer", True, ), ( @@ -773,7 +796,7 @@ def test_save_document_answer( # noqa:C901 }} }} }} - ... on FileAnswer {{ + ... on FilesAnswer {{ fileValue: value {{ name uploadUrl @@ -801,11 +824,16 @@ def test_save_document_answer( # noqa:C901 inp["input"]["value"] = [str(document.pk) for document in documents] - if question.type == Question.TYPE_FILE: - if answer.value == "some-file.pdf": - minio_mock.bucket_exists.return_value = False - answer.value = None - answer.save() + if question.type == Question.TYPE_FILES: + if answer.value: + file_name = answer.value[0]["name"] + if file_name == "some-file.pdf": + minio_mock.bucket_exists.return_value = False + answer.value = None + answer.files.all().delete() + inp["input"]["value"] = [{"name": file_name}] + + answer.save() if question.type == Question.TYPE_DATE: inp["input"]["value"] = answer.date @@ -1273,7 +1301,7 @@ def test_copy_document( question__slug="other_question", ) file_question = form_question_factory( - form=main_form, question__type=Question.TYPE_FILE + form=main_form, question__type=Question.TYPE_FILES ) dynamic_choice_question = form_question_factory( form=main_form, question__type=Question.TYPE_DYNAMIC_CHOICE @@ -1365,8 +1393,11 @@ def test_copy_document( assert new_file_answer.value == file_answer.value # file is copied minio_mock.copy_object.assert_called() - assert new_file_answer.file.name == file_answer.file.name - assert new_file_answer.file.object_name != file_answer.file.object_name + + old_file = file_answer.files.first() + new_file = new_file_answer.files.first() + assert new_file.name == old_file.name + assert new_file.object_name != old_file.object_name # dynamic options are copied for question in [ diff --git a/caluma/caluma_form/tests/test_filter_by_answer.py b/caluma/caluma_form/tests/test_filter_by_answer.py index 87b8cfdbd..f734ea339 100644 --- a/caluma/caluma_form/tests/test_filter_by_answer.py +++ b/caluma/caluma_form/tests/test_filter_by_answer.py @@ -76,7 +76,7 @@ def test_query_all_questions( if qtype not in ( # some question types are not searchable - models.Question.TYPE_FILE, + models.Question.TYPE_FILES, models.Question.TYPE_TABLE, models.Question.TYPE_FORM, models.Question.TYPE_DYNAMIC_CHOICE, diff --git a/caluma/caluma_form/tests/test_history.py b/caluma/caluma_form/tests/test_history.py index d18a74eae..37a890ed8 100644 --- a/caluma/caluma_form/tests/test_history.py +++ b/caluma/caluma_form/tests/test_history.py @@ -172,13 +172,13 @@ def test_historical_file_answer( document = document_factory(form=f) q1 = form_question_factory( - question__type=models.Question.TYPE_FILE, + question__type=models.Question.TYPE_FILES, question__slug="test_question1", form=f, ) save_answer_query = """ - mutation saveAnswer($input: SaveDocumentFileAnswerInput!) { - saveDocumentFileAnswer(input: $input) { + mutation saveAnswer($input: SaveDocumentFilesAnswerInput!) { + saveDocumentFilesAnswer(input: $input) { clientMutationId } } @@ -186,7 +186,7 @@ def test_historical_file_answer( input = { "document": str(document.pk), - "value": "my_file - rev 1", + "value": [{"name": "my_file - rev 1"}], "question": q1.question.slug, } @@ -194,17 +194,19 @@ def test_historical_file_answer( assert not result.errors hist_file_1 = models.File.history.first() timestamp1 = timezone.now() + file1 = document.answers.get(question=q1.question).files.get() + file_id = str(file1.pk) - input["value"] = "my_file - rev 2" + input["value"] = [{"name": "my_file - rev 2", "id": file_id}] result = schema_executor(save_answer_query, variable_values={"input": input}) assert not result.errors hist_file_2 = models.File.history.first() timestamp2 = timezone.now() - input["value"] = "my_file - rev 3" + input["value"] = [{"name": "my_file - rev 3", "id": file_id}] result = schema_executor(save_answer_query, variable_values={"input": input}) assert not result.errors - file3 = document.answers.get(question=q1.question).file + file3 = document.answers.get(question=q1.question).files.first() minio_mock.copy_object.assert_called() minio_mock.remove_object.assert_called() @@ -215,7 +217,7 @@ def test_historical_file_answer( historicalAnswers (asOf: $asOf) { edges { node { - ...on HistoricalFileAnswer { + ...on HistoricalFilesAnswer { __typename historyUserId value (asOf: $asOf) { @@ -240,8 +242,8 @@ def test_historical_file_answer( assert not result.errors assert ( result.data["documentAsOf"]["historicalAnswers"]["edges"][0]["node"]["value"][ - "downloadUrl" - ] + 0 + ]["downloadUrl"] == f"http://minio/download-url/{hist_file_1.pk}_{hist_file_1.name}" ) @@ -250,8 +252,8 @@ def test_historical_file_answer( assert not result.errors assert ( result.data["documentAsOf"]["historicalAnswers"]["edges"][0]["node"]["value"][ - "downloadUrl" - ] + 0 + ]["downloadUrl"] == f"http://minio/download-url/{hist_file_2.pk}_{hist_file_2.name}" ) @@ -262,8 +264,8 @@ def test_historical_file_answer( assert not result.errors assert ( result.data["documentAsOf"]["historicalAnswers"]["edges"][0]["node"]["value"][ - "downloadUrl" - ] + 0 + ]["downloadUrl"] == f"http://minio/download-url/{file3.pk}_{file3.name}" ) diff --git a/caluma/caluma_form/tests/test_jexl.py b/caluma/caluma_form/tests/test_jexl.py index 7178693cf..cb0529076 100644 --- a/caluma/caluma_form/tests/test_jexl.py +++ b/caluma/caluma_form/tests/test_jexl.py @@ -308,7 +308,7 @@ def test_answer_transform_on_hidden_question(info, form_and_document): (Question.TYPE_TEXTAREA, None), (Question.TYPE_TEXT, None), (Question.TYPE_TABLE, []), - (Question.TYPE_FILE, None), + (Question.TYPE_FILES, None), (Question.TYPE_DYNAMIC_CHOICE, None), (Question.TYPE_DYNAMIC_MULTIPLE_CHOICE, []), # Those should not appear in a JEXL answer transform diff --git a/caluma/caluma_form/tests/test_migrate_to_multiple_files.py b/caluma/caluma_form/tests/test_migrate_to_multiple_files.py new file mode 100644 index 000000000..2e682ffd4 --- /dev/null +++ b/caluma/caluma_form/tests/test_migrate_to_multiple_files.py @@ -0,0 +1,50 @@ +from django.db import connection +from django.db.migrations.executor import MigrationExecutor + + +def migrate_and_get_apps(state): + executor = MigrationExecutor(connection) + executor.migrate(state) + apps = executor.loader.project_state(state).apps + executor.loader.build_graph() + return apps + + +def test_migrate_to_multiple_files(transactional_db): + app = "caluma_form" + migrate_from = [(app, "0045_simple_history")] + migrate_to = [(app, "0046_file_answer_reverse_keys")] + + old_apps = migrate_and_get_apps(migrate_from) + + # Create some old data. Can't use factories here + + Document = old_apps.get_model(app, "Document") + Form = old_apps.get_model(app, "Form") + Answer = old_apps.get_model(app, "Answer") + File = old_apps.get_model(app, "File") + Question = old_apps.get_model(app, "Question") + FormQuestion = old_apps.get_model(app, "FormQuestion") + + the_form = Form.objects.create(slug="main-form") + + file_question = Question.objects.create(type="file", slug="file-question") + FormQuestion.objects.create(form=the_form, question=file_question) + + doc = Document.objects.create(form=the_form) + + the_file = File.objects.create(name="foo.txt") + file_ans = Answer.objects.create( + question=file_question, document=doc, file=the_file + ) + + new_apps = migrate_and_get_apps(migrate_to) + + # Test the new data: `answer.file` should be gone, `answer.files` + # must contain the old file data + Answer = new_apps.get_model(app, "Answer") + + file_ans_after = Answer.objects.get(pk=file_ans.pk) + + assert file_ans_after.files.count() == 1 + assert not hasattr(file_ans_after, "file") diff --git a/caluma/caluma_form/tests/test_model.py b/caluma/caluma_form/tests/test_model.py index 56dadc974..fd8db897e 100644 --- a/caluma/caluma_form/tests/test_model.py +++ b/caluma/caluma_form/tests/test_model.py @@ -4,6 +4,7 @@ from ...caluma_form.models import File, Question +@pytest.mark.parametrize("have_history", [True, False]) def test_delete_file_answer( db, question_factory, @@ -13,17 +14,28 @@ def test_delete_file_answer( form, document, mocker, + have_history, ): - file_question = question_factory(type=Question.TYPE_FILE) + copy = mocker.patch.object(Minio, "copy_object") + remove = mocker.patch.object(Minio, "remove_object") + + file_question = question_factory(type=Question.TYPE_FILES) form_question_factory(question=file_question, form=form) - answer = answer_factory( - question=file_question, value=None, document=document, file=file_factory() - ) - mocker.patch.object(Minio, "copy_object") - mocker.patch.object(Minio, "remove_object") + answer = answer_factory(question=file_question, value=None, document=document) + + if have_history: + file_ = answer.files.get() + file_.rename("new name") + file_.save() + assert answer.files.count() # just checking preconditions answer.delete() + + if have_history: + remove.assert_called() + copy.assert_called() + with pytest.raises(File.DoesNotExist): - answer.file.refresh_from_db() + answer.files.get() def test_update_file(db, file_factory, mocker): @@ -37,7 +49,7 @@ def test_update_file(db, file_factory, mocker): @pytest.mark.parametrize("should_raise", [False, True]) -def test_missing_file(db, file, mocker, should_raise): +def test_missing_file(db, file, mocker, should_raise, answer_factory): mocker.patch.object( Minio, "copy_object", @@ -51,8 +63,10 @@ def test_missing_file(db, file, mocker, should_raise): ), ) + other_answer = answer_factory() + if should_raise: with pytest.raises(S3Error): - file.copy() + file.copy(to_answer=other_answer) else: - file.copy() + file.copy(to_answer=other_answer) diff --git a/caluma/caluma_form/tests/test_question.py b/caluma/caluma_form/tests/test_question.py index 8b534e68a..3d1037a66 100644 --- a/caluma/caluma_form/tests/test_question.py +++ b/caluma/caluma_form/tests/test_question.py @@ -21,7 +21,7 @@ (models.Question.TYPE_CHOICE, {}, None, []), (models.Question.TYPE_MULTIPLE_CHOICE, {}, None, []), (models.Question.TYPE_FORM, {}, None, []), - (models.Question.TYPE_FILE, {}, None, []), + (models.Question.TYPE_FILES, {}, None, []), (models.Question.TYPE_DYNAMIC_CHOICE, {}, "MyDataSource", []), (models.Question.TYPE_DYNAMIC_MULTIPLE_CHOICE, {}, "MyDataSource", []), (models.Question.TYPE_STATIC, {}, None, []), @@ -156,7 +156,7 @@ def test_query_all_questions( hintText calcExpression } - ... on FileQuestion { + ... on FilesQuestion { hintText } } @@ -233,7 +233,7 @@ def sorted_option_list(q): "SaveIntegerQuestion", "SaveFloatQuestion", "SaveDateQuestion", - "SaveFileQuestion", + "SaveFilesQuestion", "SaveCalculatedFloatQuestion", ], ) diff --git a/caluma/caluma_form/tests/test_type.py b/caluma/caluma_form/tests/test_type.py index ceea57556..3387a2da6 100644 --- a/caluma/caluma_form/tests/test_type.py +++ b/caluma/caluma_form/tests/test_type.py @@ -7,7 +7,7 @@ @pytest.mark.parametrize( "question__type,expected_typename,success", [ - (models.Question.TYPE_FILE, "FileAnswer", True), + (models.Question.TYPE_FILES, "FilesAnswer", True), (models.Question.TYPE_TEXT, "StringAnswer", True), (models.Question.TYPE_TEXTAREA, "StringAnswer", True), (models.Question.TYPE_FLOAT, "FloatAnswer", True), @@ -56,7 +56,7 @@ def test_answer_types( (models.Question.TYPE_DATE, "DateQuestion"), (models.Question.TYPE_TABLE, "TableQuestion"), (models.Question.TYPE_FORM, "FormQuestion"), - (models.Question.TYPE_FILE, "FileQuestion"), + (models.Question.TYPE_FILES, "FilesQuestion"), (models.Question.TYPE_STATIC, "StaticQuestion"), ], ) diff --git a/caluma/caluma_form/tests/test_validators.py b/caluma/caluma_form/tests/test_validators.py index 8fdfa2e57..4dc12b762 100644 --- a/caluma/caluma_form/tests/test_validators.py +++ b/caluma/caluma_form/tests/test_validators.py @@ -1,3 +1,4 @@ +import minio import pytest from rest_framework.exceptions import ValidationError @@ -42,7 +43,7 @@ def test_validate_hidden_required_field( @pytest.mark.parametrize( "question__type,question__is_required", - [(Question.TYPE_FILE, "false"), (Question.TYPE_DATE, "false")], + [(Question.TYPE_FILES, "false"), (Question.TYPE_DATE, "false")], ) def test_validate_special_fields( db, form_question, question, document_factory, answer_factory, admin_user @@ -288,6 +289,7 @@ def test_validate_data_source( QuestionValidator().validate(data) +@pytest.mark.parametrize("answer__files", [[]]) @pytest.mark.parametrize( "question__type,answer__value,expected_value", [ @@ -298,11 +300,11 @@ def test_validate_data_source( (Question.TYPE_INTEGER, None, None), (Question.TYPE_FLOAT, None, None), (Question.TYPE_DATE, None, None), - (Question.TYPE_FILE, None, None), + (Question.TYPE_FILES, None, []), (Question.TYPE_TEXTAREA, None, None), ], ) -@pytest.mark.parametrize("answer__date,answer__file", [(None, None)]) +@pytest.mark.parametrize("answer__date", [None]) def test_validate_empty_answers( db, form_question, @@ -374,6 +376,7 @@ def test_validate_required_integer_0( DocumentValidator().validate(document, admin_user) +@pytest.mark.parametrize("answer__files", [[]]) @pytest.mark.parametrize( "question__type,answer__value", [ @@ -386,7 +389,7 @@ def test_validate_required_integer_0( (Question.TYPE_INTEGER, None), (Question.TYPE_FLOAT, None), (Question.TYPE_DATE, None), - (Question.TYPE_FILE, None), + (Question.TYPE_FILES, None), (Question.TYPE_TEXTAREA, None), (Question.TYPE_TABLE, None), (Question.TYPE_TABLE, []), @@ -394,9 +397,7 @@ def test_validate_required_integer_0( (Question.TYPE_DYNAMIC_CHOICE, None), ], ) -@pytest.mark.parametrize( - "answer__date,answer__file,question__is_required", [(None, None, "true")] -) +@pytest.mark.parametrize("answer__date,question__is_required", [(None, "true")]) def test_validate_required_empty_answers( db, admin_user, form_question, document, answer, question ): @@ -541,28 +542,37 @@ def test_dependent_question_is_hidden( DocumentValidator().validate(document, admin_user) -@pytest.mark.parametrize("question__type", ["file"]) +@pytest.mark.parametrize("question__type", ["files"]) @pytest.mark.parametrize("question__is_required", ["true"]) @pytest.mark.parametrize("question__is_hidden", ["false"]) -def test_required_file(db, question, form, document, answer, admin_user, form_question): +def test_required_file( + db, question, form, document, answer, admin_user, form_question, minio_mock +): # verify some assumptions assert document.form == form assert answer.document == document assert form in question.forms.all() - assert answer.file - + assert answer.files.count() + + minio_mock.stat_object.side_effect = minio.error.S3Error( + "NoSuchKey", + "object does not exist", + resource="bla", + request_id=134, + host_id="minio", + response="bla", + ) # remove the file - the_file = answer.file - answer.file = None - answer.save() + the_file = answer.files.first() + answer.files.all().delete() # ensure validation fails with no `file` value with pytest.raises(ValidationError): DocumentValidator().validate(document, admin_user) # put the file back - answer.file = the_file - answer.save() + the_file.save() + answer.files.set([the_file]) # then, validation must pass assert DocumentValidator().validate(document, admin_user) is None diff --git a/caluma/caluma_form/validators.py b/caluma/caluma_form/validators.py index 0e1fb9e91..ab50776c0 100644 --- a/caluma/caluma_form/validators.py +++ b/caluma/caluma_form/validators.py @@ -182,8 +182,10 @@ def _validate_question_table( f"Document {row_doc.pk} is not of form type {question.row_form.pk}." ) - def _validate_question_file(self, question, value, **kwargs): - pass + def _validate_question_files(self, question, value, **kwargs): + if not isinstance(value, list) and value is not None: # pragma: no cover + # should already have been rejected by schema / gql level + raise exceptions.ValidationError("Input files must be a list") def _validate_question_calculated_float(self, question, value, **kwargs): pass @@ -201,7 +203,7 @@ def validate( ): # Check all possible fields for value value = None - for i in ["documents", "file", "date", "value"]: + for i in ["documents", "files", "date", "value"]: value = kwargs.get(i, value) if value: break diff --git a/caluma/caluma_workflow/tests/__snapshots__/test_case.ambr b/caluma/caluma_workflow/tests/__snapshots__/test_case.ambr index 78158633a..2471a29d1 100644 --- a/caluma/caluma_workflow/tests/__snapshots__/test_case.ambr +++ b/caluma/caluma_workflow/tests/__snapshots__/test_case.ambr @@ -190,7 +190,7 @@ }), }) # --- -# name: test_order_by_question_answer_value[file-True-False] +# name: test_order_by_question_answer_value[files-True-False] dict({ 'allCases': dict({ 'edges': list([ @@ -201,16 +201,20 @@ 'edges': list([ dict({ 'node': dict({ - 'fileValue': dict({ - 'name': 'f', - }), + 'fileValue': list([ + dict({ + 'name': 'f', + }), + ]), }), }), dict({ 'node': dict({ - 'fileValue': dict({ - 'name': 'a', - }), + 'fileValue': list([ + dict({ + 'name': 'a', + }), + ]), }), }), ]), @@ -226,16 +230,20 @@ 'edges': list([ dict({ 'node': dict({ - 'fileValue': dict({ - 'name': 'd', - }), + 'fileValue': list([ + dict({ + 'name': 'd', + }), + ]), }), }), dict({ 'node': dict({ - 'fileValue': dict({ - 'name': 'c', - }), + 'fileValue': list([ + dict({ + 'name': 'c', + }), + ]), }), }), ]), @@ -251,16 +259,20 @@ 'edges': list([ dict({ 'node': dict({ - 'fileValue': dict({ - 'name': 'b', - }), + 'fileValue': list([ + dict({ + 'name': 'b', + }), + ]), }), }), dict({ 'node': dict({ - 'fileValue': dict({ - 'name': 'e', - }), + 'fileValue': list([ + dict({ + 'name': 'e', + }), + ]), }), }), ]), @@ -274,7 +286,7 @@ }), }) # --- -# name: test_order_by_question_answer_value[file-True-True] +# name: test_order_by_question_answer_value[files-True-True] dict({ 'allCases': dict({ 'edges': list([ @@ -285,16 +297,20 @@ 'edges': list([ dict({ 'node': dict({ - 'fileValue': dict({ - 'name': 'b', - }), + 'fileValue': list([ + dict({ + 'name': 'b', + }), + ]), }), }), dict({ 'node': dict({ - 'fileValue': dict({ - 'name': 'e', - }), + 'fileValue': list([ + dict({ + 'name': 'e', + }), + ]), }), }), ]), @@ -310,16 +326,20 @@ 'edges': list([ dict({ 'node': dict({ - 'fileValue': dict({ - 'name': 'd', - }), + 'fileValue': list([ + dict({ + 'name': 'd', + }), + ]), }), }), dict({ 'node': dict({ - 'fileValue': dict({ - 'name': 'c', - }), + 'fileValue': list([ + dict({ + 'name': 'c', + }), + ]), }), }), ]), @@ -335,16 +355,20 @@ 'edges': list([ dict({ 'node': dict({ - 'fileValue': dict({ - 'name': 'f', - }), + 'fileValue': list([ + dict({ + 'name': 'f', + }), + ]), }), }), dict({ 'node': dict({ - 'fileValue': dict({ - 'name': 'a', - }), + 'fileValue': list([ + dict({ + 'name': 'a', + }), + ]), }), }), ]), diff --git a/caluma/caluma_workflow/tests/test_case.py b/caluma/caluma_workflow/tests/test_case.py index 577a08760..1b291186f 100644 --- a/caluma/caluma_workflow/tests/test_case.py +++ b/caluma/caluma_workflow/tests/test_case.py @@ -330,7 +330,7 @@ def test_family_workitems(schema_executor, db, case_factory, work_item_factory): (Question.TYPE_TEXT, True), (Question.TYPE_FORM, True), (Question.TYPE_DATE, True), - (Question.TYPE_FILE, True), + (Question.TYPE_FILES, True), (Question.TYPE_TABLE, False), ], ) @@ -343,6 +343,7 @@ def test_order_by_question_answer_value( success, case_factory, document_factory, + file_factory, question_factory, form_question_factory, form_factory, @@ -422,20 +423,20 @@ def test_order_by_question_answer_value( case_factory(document=d2) case_factory(document=d3) - elif type == Question.TYPE_FILE: + elif type == Question.TYPE_FILES: d1 = document_factory() d2 = document_factory() d3 = document_factory() - q1 = question_factory(type=Question.TYPE_FILE, slug="test_question1") - answer_factory(question=q1, file__name="d", document=d1) - answer_factory(question=q1, file__name="b", document=d2) - answer_factory(question=q1, file__name="f", document=d3) + q1 = question_factory(type=Question.TYPE_FILES, slug="test_question1") + answer_factory(question=q1, files=[file_factory(name="d")], document=d1) + answer_factory(question=q1, files=[file_factory(name="b")], document=d2) + answer_factory(question=q1, files=[file_factory(name="f")], document=d3) - q2 = question_factory(type=Question.TYPE_FILE, slug="test_question2") - answer_factory(question=q2, file__name="c", document=d1) - answer_factory(question=q2, file__name="e", document=d2) - answer_factory(question=q2, file__name="a", document=d3) + q2 = question_factory(type=Question.TYPE_FILES, slug="test_question2") + answer_factory(question=q2, files=[file_factory(name="c")], document=d1) + answer_factory(question=q2, files=[file_factory(name="e")], document=d2) + answer_factory(question=q2, files=[file_factory(name="a")], document=d3) case_factory(document=d1) case_factory(document=d2) @@ -465,7 +466,7 @@ def test_order_by_question_answer_value( ... on DateAnswer { dateValue: value } - ... on FileAnswer { + ... on FilesAnswer { fileValue: value { name } diff --git a/caluma/schema.py b/caluma/schema.py index 68306f47d..0fb6becd2 100644 --- a/caluma/schema.py +++ b/caluma/schema.py @@ -48,7 +48,7 @@ form_schema.DateQuestion, form_schema.TableQuestion, form_schema.FormQuestion, - form_schema.FileQuestion, + form_schema.FilesQuestion, form_schema.StaticQuestion, form_schema.StringAnswer, form_schema.ListAnswer, @@ -56,7 +56,7 @@ form_schema.FloatAnswer, form_schema.DateAnswer, form_schema.TableAnswer, - form_schema.FileAnswer, + form_schema.FilesAnswer, form_schema.CalculatedFloatQuestion, form_schema.ActionButtonQuestion, workflow_schema.SimpleTask, @@ -73,7 +73,7 @@ form_historical_schema.HistoricalFloatAnswer, form_historical_schema.HistoricalDateAnswer, form_historical_schema.HistoricalTableAnswer, - form_historical_schema.HistoricalFileAnswer, + form_historical_schema.HistoricalFilesAnswer, ] if settings.ENABLE_HISTORICAL_API: diff --git a/caluma/tests/__snapshots__/test_schema.ambr b/caluma/tests/__snapshots__/test_schema.ambr index e9f3499de..fb401ba01 100644 --- a/caluma/tests/__snapshots__/test_schema.ambr +++ b/caluma/tests/__snapshots__/test_schema.ambr @@ -409,8 +409,8 @@ """form""" FORM - """file""" - FILE + """files""" + FILES """dynamic_choice""" DYNAMIC_CHOICE @@ -1065,13 +1065,13 @@ """The ID of the object""" id: ID! name: String! - answer: FileAnswer + answer: FilesAnswer uploadUrl: String downloadUrl: String metadata: GenericScalar } - type FileAnswer implements Answer & Node { + type FilesAnswer implements Answer & Node { createdAt: DateTime! modifiedAt: DateTime! createdByUser: String @@ -1082,12 +1082,11 @@ """The ID of the object""" id: ID! question: Question! - value: File! + value: [File]! meta: GenericScalar! - file: File } - type FileQuestion implements Question & Node { + type FilesQuestion implements Question & Node { createdAt: DateTime! modifiedAt: DateTime! createdByUser: String @@ -1446,13 +1445,13 @@ name: String! downloadUrl: String metadata: GenericScalar - historicalAnswer: HistoricalFileAnswer + historicalAnswer: HistoricalFilesAnswer historyDate: DateTime! historyUserId: String historyType: String } - type HistoricalFileAnswer implements HistoricalAnswer & Node { + type HistoricalFilesAnswer implements HistoricalAnswer & Node { createdAt: DateTime! modifiedAt: DateTime! createdByUser: String @@ -1463,7 +1462,7 @@ """The ID of the object""" id: ID! - value(asOf: DateTime!): HistoricalFile + value(asOf: DateTime!): [HistoricalFile] meta: GenericScalar! historyUserId: String question: Question! @@ -1471,7 +1470,6 @@ historyDate: DateTime! historyChangeReason: String historyType: String - file: File } type HistoricalFloatAnswer implements HistoricalAnswer & Node { @@ -1740,7 +1738,7 @@ saveIntegerQuestion(input: SaveIntegerQuestionInput!): SaveIntegerQuestionPayload saveTableQuestion(input: SaveTableQuestionInput!): SaveTableQuestionPayload saveFormQuestion(input: SaveFormQuestionInput!): SaveFormQuestionPayload - saveFileQuestion(input: SaveFileQuestionInput!): SaveFileQuestionPayload + saveFilesQuestion(input: SaveFilesQuestionInput!): SaveFilesQuestionPayload saveStaticQuestion(input: SaveStaticQuestionInput!): SaveStaticQuestionPayload saveCalculatedFloatQuestion(input: SaveCalculatedFloatQuestionInput!): SaveCalculatedFloatQuestionPayload saveActionButtonQuestion(input: SaveActionButtonQuestionInput!): SaveActionButtonQuestionPayload @@ -1752,7 +1750,7 @@ saveDocumentDateAnswer(input: SaveDocumentDateAnswerInput!): SaveDocumentDateAnswerPayload saveDocumentListAnswer(input: SaveDocumentListAnswerInput!): SaveDocumentListAnswerPayload saveDocumentTableAnswer(input: SaveDocumentTableAnswerInput!): SaveDocumentTableAnswerPayload - saveDocumentFileAnswer(input: SaveDocumentFileAnswerInput!): SaveDocumentFileAnswerPayload + saveDocumentFilesAnswer(input: SaveDocumentFilesAnswerInput!): SaveDocumentFilesAnswerPayload saveDefaultStringAnswer(input: SaveDefaultStringAnswerInput!): SaveDefaultStringAnswerPayload saveDefaultIntegerAnswer(input: SaveDefaultIntegerAnswerInput!): SaveDefaultIntegerAnswerPayload saveDefaultFloatAnswer(input: SaveDefaultFloatAnswerInput!): SaveDefaultFloatAnswerPayload @@ -2379,15 +2377,15 @@ clientMutationId: String } - input SaveDocumentFileAnswerInput { + input SaveDocumentFilesAnswerInput { + value: [SaveFile] question: ID! document: ID! meta: JSONString - value: String clientMutationId: String } - type SaveDocumentFileAnswerPayload { + type SaveDocumentFilesAnswerPayload { answer: Answer clientMutationId: String } @@ -2507,7 +2505,12 @@ clientMutationId: String } - input SaveFileQuestionInput { + input SaveFile { + id: String + name: String + } + + input SaveFilesQuestionInput { slug: String! label: String! infoText: String @@ -2519,7 +2522,7 @@ clientMutationId: String } - type SaveFileQuestionPayload { + type SaveFilesQuestionPayload { question: Question clientMutationId: String }