Enhancement: use stable unique IDs for custom field select options (#8299)

This commit is contained in:
shamoon
2024-12-01 20:15:38 -08:00
committed by GitHub
parent 00485138f9
commit 0fc1860d4c
24 changed files with 494 additions and 101 deletions

View File

@@ -1,5 +1,6 @@
import json
from datetime import date
from unittest.mock import ANY
from django.contrib.auth.models import Permission
from django.contrib.auth.models import User
@@ -61,7 +62,10 @@ class TestCustomFieldsAPI(DirectoriesMixin, APITestCase):
"data_type": "select",
"name": "Select Field",
"extra_data": {
"select_options": ["Option 1", "Option 2"],
"select_options": [
{"label": "Option 1", "id": "abc-123"},
{"label": "Option 2", "id": "def-456"},
],
},
},
),
@@ -73,7 +77,10 @@ class TestCustomFieldsAPI(DirectoriesMixin, APITestCase):
self.assertCountEqual(
data["extra_data"]["select_options"],
["Option 1", "Option 2"],
[
{"label": "Option 1", "id": "abc-123"},
{"label": "Option 2", "id": "def-456"},
],
)
def test_create_custom_field_nonunique_name(self):
@@ -138,6 +145,133 @@ class TestCustomFieldsAPI(DirectoriesMixin, APITestCase):
)
self.assertEqual(resp.status_code, status.HTTP_400_BAD_REQUEST)
def test_custom_field_select_unique_ids(self):
"""
GIVEN:
- Nothing
- Existing custom field
WHEN:
- API request to create custom field with select options without id
THEN:
- Unique ids are generated for each option
"""
resp = self.client.post(
self.ENDPOINT,
json.dumps(
{
"data_type": "select",
"name": "Select Field",
"extra_data": {
"select_options": [
{"label": "Option 1"},
{"label": "Option 2"},
],
},
},
),
content_type="application/json",
)
self.assertEqual(resp.status_code, status.HTTP_201_CREATED)
data = resp.json()
self.assertCountEqual(
data["extra_data"]["select_options"],
[
{"label": "Option 1", "id": ANY},
{"label": "Option 2", "id": ANY},
],
)
# Add a new option
resp = self.client.patch(
f"{self.ENDPOINT}{data['id']}/",
json.dumps(
{
"extra_data": {
"select_options": data["extra_data"]["select_options"]
+ [{"label": "Option 3"}],
},
},
),
content_type="application/json",
)
self.assertEqual(resp.status_code, status.HTTP_200_OK)
data = resp.json()
self.assertCountEqual(
data["extra_data"]["select_options"],
[
{"label": "Option 1", "id": ANY},
{"label": "Option 2", "id": ANY},
{"label": "Option 3", "id": ANY},
],
)
def test_custom_field_select_options_pruned(self):
"""
GIVEN:
- Select custom field exists and document instance with one of the options
WHEN:
- API request to remove an option from the select field
THEN:
- The option is removed from the field
- The option is removed from the document instance
"""
custom_field_select = CustomField.objects.create(
name="Select Field",
data_type=CustomField.FieldDataType.SELECT,
extra_data={
"select_options": [
{"label": "Option 1", "id": "abc-123"},
{"label": "Option 2", "id": "def-456"},
{"label": "Option 3", "id": "ghi-789"},
],
},
)
doc = Document.objects.create(
title="WOW",
content="the content",
checksum="123",
mime_type="application/pdf",
)
CustomFieldInstance.objects.create(
document=doc,
field=custom_field_select,
value_text="abc-123",
)
resp = self.client.patch(
f"{self.ENDPOINT}{custom_field_select.id}/",
json.dumps(
{
"extra_data": {
"select_options": [
{"label": "Option 1", "id": "abc-123"},
{"label": "Option 3", "id": "ghi-789"},
],
},
},
),
content_type="application/json",
)
self.assertEqual(resp.status_code, status.HTTP_200_OK)
data = resp.json()
self.assertCountEqual(
data["extra_data"]["select_options"],
[
{"label": "Option 1", "id": "abc-123"},
{"label": "Option 3", "id": "ghi-789"},
],
)
doc.refresh_from_db()
self.assertEqual(doc.custom_fields.first().value, None)
def test_create_custom_field_monetary_validation(self):
"""
GIVEN:
@@ -261,7 +395,10 @@ class TestCustomFieldsAPI(DirectoriesMixin, APITestCase):
name="Test Custom Field Select",
data_type=CustomField.FieldDataType.SELECT,
extra_data={
"select_options": ["Option 1", "Option 2"],
"select_options": [
{"label": "Option 1", "id": "abc-123"},
{"label": "Option 2", "id": "def-456"},
],
},
)
@@ -309,7 +446,7 @@ class TestCustomFieldsAPI(DirectoriesMixin, APITestCase):
},
{
"field": custom_field_select.id,
"value": 0,
"value": "abc-123",
},
],
},
@@ -332,7 +469,7 @@ class TestCustomFieldsAPI(DirectoriesMixin, APITestCase):
{"field": custom_field_monetary.id, "value": "EUR11.10"},
{"field": custom_field_monetary2.id, "value": "11.1"},
{"field": custom_field_documentlink.id, "value": [doc2.id]},
{"field": custom_field_select.id, "value": 0},
{"field": custom_field_select.id, "value": "abc-123"},
],
)
@@ -722,7 +859,10 @@ class TestCustomFieldsAPI(DirectoriesMixin, APITestCase):
name="Test Custom Field SELECT",
data_type=CustomField.FieldDataType.SELECT,
extra_data={
"select_options": ["Option 1", "Option 2"],
"select_options": [
{"label": "Option 1", "id": "abc-123"},
{"label": "Option 2", "id": "def-456"},
],
},
)
@@ -730,7 +870,7 @@ class TestCustomFieldsAPI(DirectoriesMixin, APITestCase):
f"/api/documents/{doc.id}/",
data={
"custom_fields": [
{"field": custom_field_select.id, "value": 3},
{"field": custom_field_select.id, "value": "not an option"},
],
},
format="json",

View File

@@ -657,13 +657,16 @@ class TestDocumentApi(DirectoriesMixin, DocumentConsumeDelayMixin, APITestCase):
name="Test Custom Field Select",
data_type=CustomField.FieldDataType.SELECT,
extra_data={
"select_options": ["Option 1", "Choice 2"],
"select_options": [
{"label": "Option 1", "id": "abc123"},
{"label": "Choice 2", "id": "def456"},
],
},
)
CustomFieldInstance.objects.create(
document=doc1,
field=custom_field_select,
value_select=1,
value_select="def456",
)
r = self.client.get("/api/documents/?custom_fields__icontains=choice")

View File

@@ -46,7 +46,13 @@ class TestCustomFieldsSearch(DirectoriesMixin, APITestCase):
# Add some options to the select_field
select = self.custom_fields["select_field"]
select.extra_data = {"select_options": ["A", "B", "C"]}
select.extra_data = {
"select_options": [
{"label": "A", "id": "abc-123"},
{"label": "B", "id": "def-456"},
{"label": "C", "id": "ghi-789"},
],
}
select.save()
# Now we will create some test documents
@@ -122,9 +128,9 @@ class TestCustomFieldsSearch(DirectoriesMixin, APITestCase):
# CustomField.FieldDataType.SELECT
self._create_document(select_field=None)
self._create_document(select_field=0)
self._create_document(select_field=1)
self._create_document(select_field=2)
self._create_document(select_field="abc-123")
self._create_document(select_field="def-456")
self._create_document(select_field="ghi-789")
def _create_document(self, **kwargs):
title = str(kwargs)
@@ -296,18 +302,18 @@ class TestCustomFieldsSearch(DirectoriesMixin, APITestCase):
)
def test_select(self):
# For select fields, you can either specify the index
# For select fields, you can either specify the id of the option
# or the name of the option. They function exactly the same.
self._assert_query_match_predicate(
["select_field", "exact", 1],
["select_field", "exact", "def-456"],
lambda document: "select_field" in document
and document["select_field"] == 1,
and document["select_field"] == "def-456",
)
# This is the same as:
self._assert_query_match_predicate(
["select_field", "exact", "B"],
lambda document: "select_field" in document
and document["select_field"] == 1,
and document["select_field"] == "def-456",
)
# ==========================================================#
@@ -522,9 +528,9 @@ class TestCustomFieldsSearch(DirectoriesMixin, APITestCase):
def test_invalid_value(self):
self._assert_validation_error(
json.dumps(["select_field", "exact", "not an option"]),
json.dumps(["select_field", "exact", []]),
["custom_field_query", "2"],
"integer",
"string",
)
def test_invalid_logical_operator(self):

View File

@@ -544,7 +544,11 @@ class TestFileHandling(DirectoriesMixin, FileSystemAssertsMixin, TestCase):
name="test",
data_type=CustomField.FieldDataType.SELECT,
extra_data={
"select_options": ["apple", "banana", "cherry"],
"select_options": [
{"label": "apple", "id": "abc123"},
{"label": "banana", "id": "def456"},
{"label": "cherry", "id": "ghi789"},
],
},
)
doc = Document.objects.create(
@@ -555,14 +559,22 @@ class TestFileHandling(DirectoriesMixin, FileSystemAssertsMixin, TestCase):
archive_checksum="B",
mime_type="application/pdf",
)
CustomFieldInstance.objects.create(field=cf, document=doc, value_select=0)
CustomFieldInstance.objects.create(
field=cf,
document=doc,
value_select="abc123",
)
self.assertEqual(generate_filename(doc), "document_apple.pdf")
# handler should not have been called
self.assertEqual(m.call_count, 0)
cf.extra_data = {
"select_options": ["aubergine", "banana", "cherry"],
"select_options": [
{"label": "aubergine", "id": "abc123"},
{"label": "banana", "id": "def456"},
{"label": "cherry", "id": "ghi789"},
],
}
cf.save()
self.assertEqual(generate_filename(doc), "document_aubergine.pdf")
@@ -1373,13 +1385,18 @@ class TestFilenameGeneration(DirectoriesMixin, TestCase):
cf2 = CustomField.objects.create(
name="Select Field",
data_type=CustomField.FieldDataType.SELECT,
extra_data={"select_options": ["ChoiceOne", "ChoiceTwo"]},
extra_data={
"select_options": [
{"label": "ChoiceOne", "id": "abc=123"},
{"label": "ChoiceTwo", "id": "def-456"},
],
},
)
cfi1 = CustomFieldInstance.objects.create(
document=doc_a,
field=cf2,
value_select=0,
value_select="abc=123",
)
cfi = CustomFieldInstance.objects.create(

View File

@@ -0,0 +1,87 @@
from unittest.mock import ANY
from documents.tests.utils import TestMigrations
class TestMigrateCustomFieldSelects(TestMigrations):
migrate_from = "1058_workflowtrigger_schedule_date_custom_field_and_more"
migrate_to = "1059_alter_customfieldinstance_value_select"
def setUpBeforeMigration(self, apps):
CustomField = apps.get_model("documents.CustomField")
self.old_format = CustomField.objects.create(
name="cf1",
data_type="select",
extra_data={"select_options": ["Option 1", "Option 2", "Option 3"]},
)
Document = apps.get_model("documents.Document")
doc = Document.objects.create(title="doc1")
CustomFieldInstance = apps.get_model("documents.CustomFieldInstance")
self.old_instance = CustomFieldInstance.objects.create(
field=self.old_format,
value_select=0,
document=doc,
)
def test_migrate_old_to_new_select_fields(self):
self.old_format.refresh_from_db()
self.old_instance.refresh_from_db()
self.assertEqual(
self.old_format.extra_data["select_options"],
[
{"label": "Option 1", "id": ANY},
{"label": "Option 2", "id": ANY},
{"label": "Option 3", "id": ANY},
],
)
self.assertEqual(
self.old_instance.value_select,
self.old_format.extra_data["select_options"][0]["id"],
)
class TestMigrationCustomFieldSelectsReverse(TestMigrations):
migrate_from = "1059_alter_customfieldinstance_value_select"
migrate_to = "1058_workflowtrigger_schedule_date_custom_field_and_more"
def setUpBeforeMigration(self, apps):
CustomField = apps.get_model("documents.CustomField")
self.new_format = CustomField.objects.create(
name="cf1",
data_type="select",
extra_data={
"select_options": [
{"label": "Option 1", "id": "id1"},
{"label": "Option 2", "id": "id2"},
{"label": "Option 3", "id": "id3"},
],
},
)
Document = apps.get_model("documents.Document")
doc = Document.objects.create(title="doc1")
CustomFieldInstance = apps.get_model("documents.CustomFieldInstance")
self.new_instance = CustomFieldInstance.objects.create(
field=self.new_format,
value_select="id1",
document=doc,
)
def test_migrate_new_to_old_select_fields(self):
self.new_format.refresh_from_db()
self.new_instance.refresh_from_db()
self.assertEqual(
self.new_format.extra_data["select_options"],
[
"Option 1",
"Option 2",
"Option 3",
],
)
self.assertEqual(
self.new_instance.value_select,
0,
)