fix: field appending on duplicate should ignore non string values #11621
Conversation
| const localizedUniqueRequired: FieldHook = ({ req, value }) => | ||
| `${value} - ${req?.t('general:copy') ?? 'Copy'}` | ||
| hasValue(value) ? `${value} - ${req?.t('general:copy') ?? 'Copy'}` : undefined |
There was a problem hiding this comment.
Aren't localizedUniqueRequired and localizedUnique always the same value? I know it is unrelated, but it seems odd. We should clean that up.
On another note, I do wonder what happens when you have a number field that you set to unique on duplicate. Should we set that to undefined if it is marked as unique?
There was a problem hiding this comment.
Aren't localizedUniqueRequired and localizedUnique always the same value? I know it is unrelated, but it seems odd. We should clean that up.
Sure is, I will consolidate this
On another note, I do wonder what happens when you have a number field that you set to unique on duplicate. Should we set that to undefined if it is marked as unique?
I reverted these changes to check what is currently happening when you duplicate a unique number field - it borks and throws Something went wrong.. In the console the real error is that it tried to append - Copy and then fails field validation.
I think setting it to undefined would be best - or we could append it with a 1, but this could potentially be more confusing.
There was a problem hiding this comment.
Hmm we actually don't seem to be handling all field types that allow the unique option. This includes:
- Json
- Code
- Number
- Point
- Relationship
- Select
- Text
- Textarea
- Upload
There was a problem hiding this comment.
@JarrodMFlesch I took a stab at cleaning up that file and handling the rest of fields which can have unique: true.
I appended the number field by adding + 1 but honestly not sure how much value this provides, I could return undefined instead - what do you think?
What?
When duplicating a document with
uniquefields, we append- Copyto the field value.The issue is that this is happening when the field is empty resulting in values being added that look like:
undefined - Copyornull - Copy.Why?
We are not checking the incoming value in all cases.
How?
Checks the value exists, is a string, and is not just an empty space before appending
- Copy.At first glance it looks incorrect to return required fields with
undefined- however when duplicating a document, the new document is always created as adraftso it is not an issue to returnundefined.Closes #11373