Check for duplicate field names in the form generator

[leofeyer]

Q	A
Fixed issues	Fixes #1440
Docs PR or issue	-

[fritzmg]

Hm, what if you do the following:

1. Create a Text field with field name text, save the field.
2. Switch the type of the form field to a fieldsetStart, save the field.
3. Create a Text field with field name text, save the field.

This will probably throw the exception, but it won't be clear to the user why this happens, as there is no form field with an editable field name of the same name anymore.

May be the field name of any form field, that does not contain the field name in its palette, should be cleared on save.

[leofeyer]

That's a valid point. 🤔

[fritzmg]

I mean, it's still better with the exception now, because then you at least know that something is wrong. Before, the new text field simply would not be shown in the front end (or the fieldset, I cannot remember), because there would be two fields with the same field name (even though the field name of the fieldset would not matter).

[leofeyer]

I have adjusted the SQL query in aae17493e3661bff489bbee6bf8c190689917606 so it only considers field types with a "name" field in their palette.

[leofeyer]

I am not sure whether this is overkill though. There are still ways to circumvent the check, e.g.

1. Create a text field with the name foo.
2. Switch the type to "explanation field".
3. Create another text field with the name foo.
4. Switch back the type of the first field to "text field".

Now both fields will have the name foo. 🤷‍♂ And the same is possible by restoring a previous version of the first field.

Therefore I am not sure whether the more complicated check makes sense?

[fritzmg]

What about my idea, simply removing the field name in an onsubmit_callback, if the new field does not have a field name in its palette?

[leofeyer]

Not sure about this, because it would alter the original record. If a user switches back, they will probably expect the name to be still present, won't they?

[aschempp]

I‘m not sure whether this check is more harm than good. Maybe it would be better to show a „duplicate“ warning in the field list than prevent duplicate names?

[leofeyer]

A warning in the field list would not make it any more obvious that the duplicate field name is "hidden" in an explanation or fieldset field, would it?

[m-vo]

I think removing the value is the only correct solution from a data integrity point of view (you could add a composite unique key [pid, name] then to ensure this).

But this feature/problem exists everywhere within dca tables not only here, so I don't think there is a easy solution to cater for both sharing/restoring field values and data integrity. :man_shrugging:

[asaage]

Honestly i think one of these light-blue tl_info - messages for elements with a field-name would be more than sufficient. Removing the value is a pattern that almost nowhere else is applied. Why are those field's only rendered once in the first place if it's a valid use-case? https://github.com/contao/contao/issues/1440#issuecomment-597006160 I remember that once i struggled trying that with a conditional-formfields.

[aschempp]

Honestly i think one of these light-blue tl_info - messages for elements with a field-name would be more than sufficient. Removing the value is a pattern that almost nowhere else is applied. Why are those field's only rendered once in the first place if it's a valid use-case? #1440 (comment) I remember that once i struggled trying that with a conditional-formfields.

absolutely agree

[Toflar]

I also think that info box would be enough.

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[fritzmg]

But this feature/problem exists everywhere within dca tables not only here, so I don't think there is a easy solution to cater for both sharing/restoring field values and data integrity.

I consider the form fields a special case though. You cannot reproduce the same problem with, say, a news article for example, because you'll never be able to create another news article with the same alias. Whereas with form fields you currently can create form fields that use the same field name - which then leads to issues in the front end. Form fields are further unique because there is a DCA field, that is technically unique, but does not exist in every palette for each type of form field. Whereas in other places in Contao where there is a DCA field that is unique, that field is always there.

So implementing a unique solution is warranted imho, since it is a unique situation.

[aschempp]

I think we discussed this at a public call. There are (rare) cases where using the same input name makes sense, therefore we must not limit it but just show a blue box. My client had this issue last week and he actually came up with "it should tell me that I have the same field name" as well 😂

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.