Remove default/fallback entity form operation

I think this is a good improvement and makes $operation to work inline with the usual edit|add|delete, .. use - however, we miss implementation of deletion forms here as well. Maybe we could do that as a follow-up?

Despite being the one that introduced the default operation, I'm definitely +1 on this :)

#11: entity-form-2006348-11.patch queued for re-testing.

#21 is in, let's get this in now

Can we please take #2014821: Introduce form modes UI and their configuration entity into consideration before rushing this in?

Because "reasonably sure" != "enforced policy/requirement"? :)

Another option would be to make the 'default_operation' property introduced by translation_entity.module an official property of the entity annotation, and default it to 'edit' instead of 'default'. We can either do it here (what I'd prefer) or open a new issue for it.

I like the fact that we remove some magic, which doesn't really actually help as you want to have a custom form controller anyway.

This is my idea on how to marry the 'default' form mode/form display with the default form operation. Basically, I'm bringing #2022873: Turn default_operation into an actual thing into scope here because it's kinda late to have dependent issues.

Now with less broken logic.

+++ b/core/includes/entity.incundefined
@@ -442,45 +442,17 @@ function entity_access_controller($entity_type) {
-function entity_form_state_defaults(EntityInterface $entity, $operation = 'default') {
+function entity_form_state_defaults(EntityInterface $entity, $operation) {

Is there a follow up already to move this function as well to some kind of object?

I'm assigning to catch since he has expressed a feeling that this change is not necessary...

Also this needs a reroll...

git applyc https://drupal.org/files/entity-2006348-40.patch
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
100 52421  100 52421    0     0  29488      0  0:00:01  0:00:01 --:--:-- 32824
error: patch failed: core/modules/comment/comment.module:686
error: core/modules/comment/comment.module: patch does not apply

Yeah I'm not sure the API change is necessary, can we clean up the existing forms doing double duty without it?

I think it in this case it makes sense to do the API change, as otherwise contrib will be lazy and not care about about $operation at all. There's two implications of that:
1. Contrib simply doing all forms (e.g. add and edit) in one controller and putting conditions such as $entity->isNew() there. This is not optimal, but I don't consider this particularly problematic. It's a small amount of code smell and it's contained to the specific module.

2. Contrib modules that interact with entity forms might have the expectation that a 'default' $operation always exist and thus break modules that follow best practices. This point is arguable, as core modules already follow this best practice (assuming this patch goes in), but nevertheless this point is why I would advocate making the API change.

Just wondering : if we remove the notion of a "default" form mode, what's the impact on the "create new field" UI ?
Does it mean newly created fields do not appear in any forms until the admin explicitly visits the "Manage form" tabs and configures the field he just created ?
That would be a net UX regression.

On the display side, the fact that we have a "default" view mode (and that all view modes by default fall back to this "default" mode unless explicitly stated) is important UX wise : a newly added field is automatically displayed in the "default" view mode using the default formatter.
In other words : newly added fields automatically appear in "full" mode, and stay hidden in "teaser", without the admin needing to visit "Manage Display" - aka "just works".

Does it mean newly created fields do not appear in any forms until the admin explicitly visits the "Manage form" tabs and configures the field he just created ?
That would be a net UX regression.

The problem that we have is that there is no way to sort the fields. It used to be that the default view mode just took over the sorting from the "Manage fields" page. But that is now sorted alphabetically since the introduction of form modes UI. So how should such a default form mode be populated?

On the "display" side, new fields are automatically added at the bottom (highest weight +1) of the "default" view mode.
That's just how EntityDisplay->setComponent() works if no 'weight' is specified.

Yes, I have the kept the 'default' form mode. It's just the default operation that we're dropping here. I agree that it makes this relationship a lot harder to understand though..

Oh, OK, makes perfect sense :-)
Thanks !

Yes, I guess I can take the blame for that :/ Imo, the interdiff from #46 doesn't really make sense, because it's still allowing contrib to be sloppy, as @tstoeckler points out in the next comment.

So, given that both the original author of the 'default' controller concept, entity api and field api maintainers agree that this is a good change that will actually help contrib, I would also really like to see the full patch in.

@@ -829,11 +801,18 @@ function entity_get_form_display($entity_type, $bundle, $form_mode) {
+  // Since form modes are also tied to entity form operations, we need to use
+  // the 'default' form display when the requested $form_mode is the default form
+  // operation.
+  if ($form_mode == $entity_info['default_operation']) {
+    $form_mode = 'default';
+  }

To be honest, I don't understand why it magically switches to 'default' no matter what is defined on the entity type, even there is quite some documentation.

There are some places like COnfigTest => ConfigTestInterface which could make the patch a bit smaller but nevermind.

It looks like the entity system has changed greatly since this thread started. There is a still an odd split between entities using "default" as form operation and entities using "add" and "edit" instead.

Independent of which names are used I would like custom form modes added by the UI to reuse existing classes. Right now calling a form mode added through the UI throws an error. See #2511720: Allow form modes to use default operation if a form operation is not explicitly set

so summary needs update and evaluation
@Tim, #54 shows that there's BC-compatible fix!

Also default operation changes a form name for entities so that affects contrib that uses node form alters at least

I may be interested in getting this moving again, but for now I'll try to fix the (personally) most glaring offender: #2848120: Short term fix: Make ContentTranslationController recognize 'add' and 'edit' form handlers.

I think this change can be usefull !!! Actually we have re-designed FormModeManager to permit to enhance all entities routes to use natively form-modes. I'm free to help this to be tested/continued as soon as possible.