Enhancement: stable IDs for custom field select values

This commit is contained in:
shamoon 2024-11-12 22:22:13 -08:00
parent 9c57915e9f
commit 5be432d135
16 changed files with 175 additions and 52 deletions

View File

@ -17,7 +17,11 @@ const customFields: CustomField[] = [
name: 'Field 4', name: 'Field 4',
data_type: CustomFieldDataType.Select, data_type: CustomFieldDataType.Select,
extra_data: { extra_data: {
select_options: ['Option 1', 'Option 2', 'Option 3'], select_options: [
{ label: 'Option 1', id: 'abc-123' },
{ label: 'Option 2', id: 'def-456' },
{ label: 'Option 3', id: 'ghi-789' },
],
}, },
}, },
{ {
@ -131,6 +135,8 @@ describe('CustomFieldDisplayComponent', () => {
}) })
it('should show select value', () => { it('should show select value', () => {
expect(component.getSelectValue(customFields[3], 2)).toEqual('Option 3') expect(component.getSelectValue(customFields[3], 'ghi-789')).toEqual(
'Option 3'
)
}) })
}) })

View File

@ -117,8 +117,8 @@ export class CustomFieldDisplayComponent implements OnInit, OnDestroy {
return this.docLinkDocuments?.find((d) => d.id === docId)?.title return this.docLinkDocuments?.find((d) => d.id === docId)?.title
} }
public getSelectValue(field: CustomField, index: number): string { public getSelectValue(field: CustomField, id: string): string {
return field.extra_data.select_options[index] return field.extra_data.select_options?.find((o) => o.id === id)?.label
} }
ngOnDestroy(): void { ngOnDestroy(): void {

View File

@ -39,7 +39,12 @@ const customFields = [
id: 2, id: 2,
name: 'Test Select Field', name: 'Test Select Field',
data_type: CustomFieldDataType.Select, data_type: CustomFieldDataType.Select,
extra_data: { select_options: ['Option 1', 'Option 2'] }, extra_data: {
select_options: [
{ label: 'Option 1', id: 'abc-123' },
{ label: 'Option 2', id: 'def-456' },
],
},
}, },
] ]
@ -128,11 +133,19 @@ describe('CustomFieldsQueryDropdownComponent', () => {
id: 1, id: 1,
name: 'Test Field', name: 'Test Field',
data_type: CustomFieldDataType.Select, data_type: CustomFieldDataType.Select,
extra_data: { select_options: ['Option 1', 'Option 2'] }, extra_data: {
select_options: [
{ label: 'Option 1', id: 'abc-123' },
{ label: 'Option 2', id: 'def-456' },
],
},
} }
component.customFields = [field] component.customFields = [field]
const options = component.getSelectOptionsForField(1) const options = component.getSelectOptionsForField(1)
expect(options).toEqual(['Option 1', 'Option 2']) expect(options).toEqual([
{ label: 'Option 1', id: 'abc-123' },
{ label: 'Option 2', id: 'def-456' },
])
// Fallback to empty array if field is not found // Fallback to empty array if field is not found
const options2 = component.getSelectOptionsForField(2) const options2 = component.getSelectOptionsForField(2)

View File

@ -311,7 +311,9 @@ export class CustomFieldsQueryDropdownComponent implements OnDestroy {
})) }))
} }
getSelectOptionsForField(fieldID: number): string[] { getSelectOptionsForField(
fieldID: number
): Array<{ label: string; id: string }> {
const field = this.customFields.find((field) => field.id === fieldID) const field = this.customFields.find((field) => field.id === fieldID)
if (field) { if (field) {
return field.extra_data['select_options'] return field.extra_data['select_options']

View File

@ -21,15 +21,13 @@
</button> </button>
<div formArrayName="select_options"> <div formArrayName="select_options">
@for (option of objectForm.controls.extra_data.controls.select_options.controls; track option; let i = $index) { @for (option of objectForm.controls.extra_data.controls.select_options.controls; track option; let i = $index) {
<div class="input-group input-group-sm my-2"> <div class="input-group input-group-sm my-2" [formGroup]="objectForm.controls.extra_data.controls.select_options.controls[i]">
<input #selectOption type="text" class="form-control" [formControl]="option" autocomplete="off"> <input type="hidden" formControlName="id">
<input #selectOption type="text" class="form-control" formControlName="label" autocomplete="off">
<button type="button" class="btn btn-outline-danger" (click)="removeSelectOption(i)" i18n>Delete</button> <button type="button" class="btn btn-outline-danger" (click)="removeSelectOption(i)" i18n>Delete</button>
</div> </div>
} }
</div> </div>
@if (object?.id) {
<small class="d-block mt-2" i18n>Warning: existing instances of this field will retain their current value index (e.g. option #1, #2, #3) after editing the options here</small>
}
} }
@case (CustomFieldDataType.Monetary) { @case (CustomFieldDataType.Monetary) {
<div class="my-3"> <div class="my-3">

View File

@ -80,7 +80,11 @@ describe('CustomFieldEditDialogComponent', () => {
name: 'Field 1', name: 'Field 1',
data_type: CustomFieldDataType.Select, data_type: CustomFieldDataType.Select,
extra_data: { extra_data: {
select_options: ['Option 1', 'Option 2', 'Option 3'], select_options: [
{ label: 'Option 1', id: '123-xyz' },
{ label: 'Option 2', id: '456-abc' },
{ label: 'Option 3', id: '789-123' },
],
}, },
} }
fixture.detectChanges() fixture.detectChanges()
@ -94,6 +98,10 @@ describe('CustomFieldEditDialogComponent', () => {
component.dialogMode = EditDialogMode.CREATE component.dialogMode = EditDialogMode.CREATE
fixture.detectChanges() fixture.detectChanges()
component.ngOnInit() component.ngOnInit()
expect(
component.objectForm.get('extra_data').get('select_options').value.length
).toBe(0)
component.addSelectOption()
expect( expect(
component.objectForm.get('extra_data').get('select_options').value.length component.objectForm.get('extra_data').get('select_options').value.length
).toBe(1) ).toBe(1)
@ -101,14 +109,10 @@ describe('CustomFieldEditDialogComponent', () => {
expect( expect(
component.objectForm.get('extra_data').get('select_options').value.length component.objectForm.get('extra_data').get('select_options').value.length
).toBe(2) ).toBe(2)
component.addSelectOption()
expect(
component.objectForm.get('extra_data').get('select_options').value.length
).toBe(3)
component.removeSelectOption(0) component.removeSelectOption(0)
expect( expect(
component.objectForm.get('extra_data').get('select_options').value.length component.objectForm.get('extra_data').get('select_options').value.length
).toBe(2) ).toBe(1)
}) })
it('should focus on last select option input', () => { it('should focus on last select option input', () => {

View File

@ -57,9 +57,16 @@ export class CustomFieldEditDialogComponent
} }
if (this.object?.data_type === CustomFieldDataType.Select) { if (this.object?.data_type === CustomFieldDataType.Select) {
this.selectOptions.clear() this.selectOptions.clear()
this.object.extra_data.select_options.forEach((option) => this.object.extra_data.select_options
this.selectOptions.push(new FormControl(option)) .filter((option) => option)
) .forEach((option) =>
this.selectOptions.push(
new FormGroup({
label: new FormControl(option.label),
id: new FormControl(option.id),
})
)
)
} }
} }
@ -89,7 +96,7 @@ export class CustomFieldEditDialogComponent
name: new FormControl(null), name: new FormControl(null),
data_type: new FormControl(null), data_type: new FormControl(null),
extra_data: new FormGroup({ extra_data: new FormGroup({
select_options: new FormArray([new FormControl(null)]), select_options: new FormArray([]),
default_currency: new FormControl(null), default_currency: new FormControl(null),
}), }),
}) })
@ -104,7 +111,9 @@ export class CustomFieldEditDialogComponent
} }
public addSelectOption() { public addSelectOption() {
this.selectOptions.push(new FormControl('')) this.selectOptions.push(
new FormGroup({ label: new FormControl(null), id: new FormControl(null) })
)
} }
public removeSelectOption(index: number) { public removeSelectOption(index: number) {

View File

@ -132,12 +132,4 @@ describe('SelectComponent', () => {
const expectedTitle = `Filter documents with this ${component.title}` const expectedTitle = `Filter documents with this ${component.title}`
expect(component.filterButtonTitle).toEqual(expectedTitle) expect(component.filterButtonTitle).toEqual(expectedTitle)
}) })
it('should support setting items as a plain array', () => {
component.itemsArray = ['foo', 'bar']
expect(component.items).toEqual([
{ id: 0, name: 'foo' },
{ id: 1, name: 'bar' },
])
})
}) })

View File

@ -34,11 +34,6 @@ export class SelectComponent extends AbstractInputComponent<number> {
if (items && this.value) this.checkForPrivateItems(this.value) if (items && this.value) this.checkForPrivateItems(this.value)
} }
@Input()
set itemsArray(items: any[]) {
this._items = items.map((item, index) => ({ id: index, name: item }))
}
writeValue(newValue: any): void { writeValue(newValue: any): void {
if (newValue && this._items) { if (newValue && this._items) {
this.checkForPrivateItems(newValue) this.checkForPrivateItems(newValue)

View File

@ -190,7 +190,8 @@
@case (CustomFieldDataType.Select) { @case (CustomFieldDataType.Select) {
<pngx-input-select formControlName="value" <pngx-input-select formControlName="value"
[title]="getCustomFieldFromInstance(fieldInstance)?.name" [title]="getCustomFieldFromInstance(fieldInstance)?.name"
[itemsArray]="getCustomFieldFromInstance(fieldInstance)?.extra_data.select_options" [items]="getCustomFieldFromInstance(fieldInstance)?.extra_data.select_options"
bindLabel="label"
[allowNull]="true" [allowNull]="true"
[horizontal]="true" [horizontal]="true"
[removable]="userIsOwner" [removable]="userIsOwner"

View File

@ -56,7 +56,7 @@ export interface CustomField extends ObjectWithId {
name: string name: string
created?: Date created?: Date
extra_data?: { extra_data?: {
select_options?: string[] select_options?: Array<{ label: string; id: string }>
default_currency?: string default_currency?: string
} }
document_count?: number document_count?: number

View File

@ -0,0 +1,79 @@
# Generated by Django 5.1.1 on 2024-11-13 05:14
import uuid
from django.db import migrations
from django.db import models
from django.db import transaction
def migrate_customfield_selects(apps, schema_editor):
"""
Migrate the custom field selects from a simple list of strings to a list of dictionaries with
label and id. Then update all instances of the custom field to use the new format.
"""
CustomFieldInstance = apps.get_model("documents", "CustomFieldInstance")
CustomField = apps.get_model("documents", "CustomField")
with transaction.atomic():
for custom_field in CustomField.objects.all():
if custom_field.data_type == "select": # CustomField.FieldDataType.SELECT
old_select_options = custom_field.extra_data["select_options"]
custom_field.extra_data["select_options"] = [
{"id": str(uuid.uuid4()), "label": value}
for value in old_select_options
]
custom_field.save()
for instance in CustomFieldInstance.objects.filter(field=custom_field):
if instance.value_select:
instance.value_select = custom_field.extra_data[
"select_options"
][int(instance.value_select)]["id"]
instance.save()
def reverse_migrate_customfield_selects(apps, schema_editor):
"""
Reverse the migration of the custom field selects from a list of dictionaries with label and id
to a simple list of strings. Then update all instances of the custom field to use the old format,
which is just the index of the selected option.
"""
CustomFieldInstance = apps.get_model("documents", "CustomFieldInstance")
CustomField = apps.get_model("documents", "CustomField")
with transaction.atomic():
for custom_field in CustomField.objects.all():
if custom_field.data_type == "select": # CustomField.FieldDataType.SELECT
old_select_options = custom_field.extra_data["select_options"]
custom_field.extra_data["select_options"] = [
option["label"]
for option in custom_field.extra_data["select_options"]
]
custom_field.save()
for instance in CustomFieldInstance.objects.filter(field=custom_field):
instance.value_select = next(
index
for index, option in enumerate(old_select_options)
if option.id == instance.value_select
)
instance.save()
class Migration(migrations.Migration):
dependencies = [
("documents", "1056_customfieldinstance_deleted_at_and_more"),
]
operations = [
migrations.AlterField(
model_name="customfieldinstance",
name="value_select",
field=models.CharField(max_length=128, null=True),
),
migrations.RunPython(
migrate_customfield_selects,
reverse_migrate_customfield_selects,
),
]

View File

@ -947,7 +947,7 @@ class CustomFieldInstance(SoftDeleteModel):
value_document_ids = models.JSONField(null=True) value_document_ids = models.JSONField(null=True)
value_select = models.PositiveSmallIntegerField(null=True) value_select = models.CharField(null=True, max_length=128)
class Meta: class Meta:
ordering = ("created",) ordering = ("created",)
@ -962,7 +962,11 @@ class CustomFieldInstance(SoftDeleteModel):
def __str__(self) -> str: def __str__(self) -> str:
value = ( value = (
self.field.extra_data["select_options"][self.value_select] next(
option.get("label")
for option in self.field.extra_data["select_options"]
if option.get("id") == self.value_select
)
if ( if (
self.field.data_type == CustomField.FieldDataType.SELECT self.field.data_type == CustomField.FieldDataType.SELECT
and self.value_select is not None and self.value_select is not None

View File

@ -539,7 +539,7 @@ class CustomFieldSerializer(serializers.ModelSerializer):
or not isinstance(attrs["extra_data"]["select_options"], list) or not isinstance(attrs["extra_data"]["select_options"], list)
or len(attrs["extra_data"]["select_options"]) == 0 or len(attrs["extra_data"]["select_options"]) == 0
or not all( or not all(
isinstance(option, str) and len(option) > 0 len(option.get("label", "")) > 0
for option in attrs["extra_data"]["select_options"] for option in attrs["extra_data"]["select_options"]
) )
) )
@ -646,10 +646,14 @@ class CustomFieldInstanceSerializer(serializers.ModelSerializer):
elif field.data_type == CustomField.FieldDataType.SELECT: elif field.data_type == CustomField.FieldDataType.SELECT:
select_options = field.extra_data["select_options"] select_options = field.extra_data["select_options"]
try: try:
select_options[data["value"]] next(
option
for option in select_options
if option["id"] == data["value"]
)
except Exception: except Exception:
raise serializers.ValidationError( raise serializers.ValidationError(
f"Value must be index of an element in {select_options}", f"Value must be an id of an element in {select_options}",
) )
elif field.data_type == CustomField.FieldDataType.DOCUMENTLINK: elif field.data_type == CustomField.FieldDataType.DOCUMENTLINK:
doc_ids = data["value"] doc_ids = data["value"]

View File

@ -253,7 +253,11 @@ def get_custom_fields_context(
): ):
options = field_instance.field.extra_data["select_options"] options = field_instance.field.extra_data["select_options"]
value = pathvalidate.sanitize_filename( value = pathvalidate.sanitize_filename(
options[int(field_instance.value)], next(
option["label"]
for option in options
if option["id"] == field_instance.value
),
replacement_text="-", replacement_text="-",
) )
else: else:

View File

@ -61,7 +61,10 @@ class TestCustomFieldsAPI(DirectoriesMixin, APITestCase):
"data_type": "select", "data_type": "select",
"name": "Select Field", "name": "Select Field",
"extra_data": { "extra_data": {
"select_options": ["Option 1", "Option 2"], "select_options": [
{"label": "Option 1", "id": "abc-123"},
{"label": "Option 2", "id": "def-456"},
],
}, },
}, },
), ),
@ -73,7 +76,10 @@ class TestCustomFieldsAPI(DirectoriesMixin, APITestCase):
self.assertCountEqual( self.assertCountEqual(
data["extra_data"]["select_options"], 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): def test_create_custom_field_nonunique_name(self):
@ -261,7 +267,10 @@ class TestCustomFieldsAPI(DirectoriesMixin, APITestCase):
name="Test Custom Field Select", name="Test Custom Field Select",
data_type=CustomField.FieldDataType.SELECT, data_type=CustomField.FieldDataType.SELECT,
extra_data={ extra_data={
"select_options": ["Option 1", "Option 2"], "select_options": [
{"label": "Option 1", "id": "abc-123"},
{"label": "Option 2", "id": "def-456"},
],
}, },
) )
@ -309,7 +318,7 @@ class TestCustomFieldsAPI(DirectoriesMixin, APITestCase):
}, },
{ {
"field": custom_field_select.id, "field": custom_field_select.id,
"value": 0, "value": "abc-123",
}, },
], ],
}, },
@ -332,7 +341,7 @@ class TestCustomFieldsAPI(DirectoriesMixin, APITestCase):
{"field": custom_field_monetary.id, "value": "EUR11.10"}, {"field": custom_field_monetary.id, "value": "EUR11.10"},
{"field": custom_field_monetary2.id, "value": "11.1"}, {"field": custom_field_monetary2.id, "value": "11.1"},
{"field": custom_field_documentlink.id, "value": [doc2.id]}, {"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 +731,10 @@ class TestCustomFieldsAPI(DirectoriesMixin, APITestCase):
name="Test Custom Field SELECT", name="Test Custom Field SELECT",
data_type=CustomField.FieldDataType.SELECT, data_type=CustomField.FieldDataType.SELECT,
extra_data={ extra_data={
"select_options": ["Option 1", "Option 2"], "select_options": [
{"label": "Option 1", "id": "abc-123"},
{"label": "Option 2", "id": "def-456"},
],
}, },
) )
@ -730,7 +742,7 @@ class TestCustomFieldsAPI(DirectoriesMixin, APITestCase):
f"/api/documents/{doc.id}/", f"/api/documents/{doc.id}/",
data={ data={
"custom_fields": [ "custom_fields": [
{"field": custom_field_select.id, "value": 3}, {"field": custom_field_select.id, "value": "not an option"},
], ],
}, },
format="json", format="json",