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 acda63756..bde292cd4 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..227e35d8d 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,61 @@ 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("File 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 +117,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 +140,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 +178,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 +185,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 ec7104c2f..7c54dca82 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..27bc6d935 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,84 @@ 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"]) + + +@pytest.mark.parametrize("answer__files", [[]]) +@pytest.mark.parametrize("question__type", ["files"]) +def test_file_answer_mutation(db, answer, document, schema_executor, minio_mock): + query = """ + mutation save ($input: SaveDocumentFilesAnswerInput!) { + saveDocumentFilesAnswer (input: $input) { + answer { + ... on FilesAnswer { + value { + name + uploadUrl + id + } + } + } + } + } + """ + # Precondition check + assert answer.files.count() == 0 + + # Initial upload + result = schema_executor( + 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" + history_count_before = first_file.history.count() + + # Second request: adding a file + result = schema_executor( + query, + variable_values={ + "input": { + "document": str(document.pk), + "question": answer.question.slug, + "value": [ + {"name": "some test file.txt", "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() + + # Third: remove a file + result = schema_executor( + query, + variable_values={ + "input": { + "document": str(document.pk), + "question": answer.question.slug, + "value": [ + {"name": "another file.txt", "id": str(second_file.pk)}, + ], + } + }, + ) + assert not result.errors + assert answer.files.count() == 1 + with pytest.raises(type(first_file).DoesNotExist): + # This file should be gone + first_file.refresh_from_db() 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..b8e1b2c96 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 39d96712c..436e990d5 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 @@ -2381,15 +2379,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 } @@ -2509,7 +2507,12 @@ clientMutationId: String } - input SaveFileQuestionInput { + input SaveFile { + id: String + name: String + } + + input SaveFilesQuestionInput { slug: String! label: String! infoText: String @@ -2521,7 +2524,7 @@ clientMutationId: String } - type SaveFileQuestionPayload { + type SaveFilesQuestionPayload { question: Question clientMutationId: String }