Problem/Motivation

I was testing Drupal 8.0.x this morning and discovered that when you create a view mode it doesn't automatically show up in the Manage Display section of a content type.

see here:
no One more time.

I expect my new display mode "One more time" to display among the available choices. It IS accessible if I use it's specific url but I don't see the link for it.

Proposed resolution

It was pointed out that this problem can be fixed if the cached list of local tasks is cleared when view modes are created.

Needs:

  • Clear cache when view modes are created
  • Add a test to check if the view mode can be displayed after it's created

Remaining tasks

User interface changes

API changes

Original report by @cosmicdreams

I was testing Drupal 8.0.x this morning and discovered that when you create a view mode it doesn't automatically show up in the Manage Display section of a content type.

see here:
no One more time.

I expect my new display mode "One more time" to display among the available choices. It IS accessible if I use it's specific url but I don't see the link for it.

It was pointed out that this problem can be fixed if the cached list of local tasks is cleared when view modes are created.

CommentFileSizeAuthor
#21 2341455-21.patch3.12 KBswentel
#20 2341455-20.patch3.16 KBswentel
Drupal issue.PNG67.33 KBcosmicdreams
#10 2341455-9.patch953 bytesswentel
#13 2341455-13.patch3.14 KBswentel
#13 2341455-13-fail.patch1.45 KBswentel
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
CreditAttribution: larowlan commented
Issue tags: +Needs tests
dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner commented

Yeah this is just a cache clear problem, though. You just have to clear the local tasks cache tags in order to achieve that.

cosmicdreams’s picture

cosmicdreams CreditAttribution: cosmicdreams commented

@dawehner are you telling me that after I create a new display mode I HAVE TO clear the cache in order to use it?

dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner commented

No YOU haven't but the code should clear these cache tags.

cosmicdreams’s picture

cosmicdreams CreditAttribution: cosmicdreams commented
Title: Regression: Manage Display tab does not display new view modes » Regression: Creating a new view mode should clear cache, so the new view mode can be displayed.
Issue summary: View changes

OK, then this is a bug. Will change title to better target the problem.

Berdir’s picture

Berdir
Primary language German
Location Switzerland
CreditAttribution: Berdir commented

field_ui.module already has field_ui_view_mode_presave(), and field_ui_view_mode_delete().

I think those hooks are actually dead right now because the entity type is now called entity_view_mode. But that's the place where we should clear whatever cache we have.

It's probably not just the local task cache (clearCachedDefinitions() on the local task plugin manager) but also the view mode caches in EntityManager (clearCachedFieldDefinitions() there I think as views modes are cached with the entity_field_info tag).

I also think we need this for form modes, so we should add the same logic there (hook_entity_form_mode_presave() and delete).

Berdir’s picture

Berdir
Primary language German
Location Switzerland
CreditAttribution: Berdir commented
Title: Regression: Creating a new view mode should clear cache, so the new view mode can be displayed. » Creating a new view mode should clear cache, so the new view mode can be displayed.

Also, I'd say this is just a bug, no need for prefixing, regressions have no special privileges (anymore).

swentel’s picture

swentel CreditAttribution: swentel commented
Component: entity system » field_ui.module

Moving to field ui module

swentel’s picture

swentel CreditAttribution: swentel commented
Assigned: Unassigned » swentel

The cache clear needs to happen when you enable or disable a view mode on the default screen of manage display.
Also when deleting a view/form mode we need to clean up the entity displays as well.

swentel’s picture

swentel CreditAttribution: swentel commented
Status: Active » Needs review
FileSize
2341455-9.patch953 bytes

So scratch my comment, berdir was right, the hooks needed to be renamed.

I'd say we remove the entity displays on removing a mode in another issue.

aspilicious’s picture

aspilicious CreditAttribution: aspilicious commented

Don't we need a test for this?

swentel’s picture

swentel CreditAttribution: swentel commented

working on it

swentel’s picture

swentel CreditAttribution: swentel commented
FileSize
2341455-13-fail.patch1.45 KB
2341455-13.patch3.14 KB

Now with tests

The last submitted patch, 13: 2341455-13-fail.patch, failed testing.

aspilicious’s picture

aspilicious CreditAttribution: aspilicious commented
Status: Needs review » Reviewed & tested by the community
catch’s picture

catch
he/him
Primary language English
CreditAttribution: catch commented
Status: Reviewed & tested by the community » Needs review 
+++ b/core/modules/field_ui/field_ui.module
@@ -186,20 +187,35 @@ function field_ui_form_node_type_form_submit($form, FormStateInterface $form_sta
   \Drupal::service('router.builder')->setRebuildNeeded();

Should this really be in presave or insert/update?

swentel’s picture

swentel CreditAttribution: swentel commented
Issue tags: -Needs tests

@catch If we use insert/update we'll have two more hook implementations, so we'll end up with 6 instead of 4. The other difference would be that insert/update runs after the entity is saved, but as far as I can see, this has no impact - I think.

Berdir queued 13: 2341455-13.patch for re-testing.

The last submitted patch, 13: 2341455-13.patch, failed testing.

swentel’s picture

swentel CreditAttribution: swentel commented
FileSize
2341455-20.patch3.16 KB
swentel’s picture

swentel CreditAttribution: swentel commented
FileSize
2341455-21.patch3.12 KB

Better patch

The last submitted patch, 20: 2341455-20.patch, failed testing.

Berdir’s picture

Berdir
Primary language German
Location Switzerland
CreditAttribution: Berdir commented
Status: Needs review » Reviewed & tested by the community

Looks good to me, easy fix that is confusing users quite a bit, as seen by multiple Drupal answers questions.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott commented
Status: Reviewed & tested by the community » Fixed

This issue is a normal bug fix, and doesn't include any disruptive changes, so it is allowed per https://www.drupal.org/core/beta-changes. Committed 0b5587b and pushed to 8.0.x. Thanks!

  • alexpott committed 0b5587b on 8.0.x 
    Issue #2341455 by swentel, cosmicdreams: Creating a new view mode should...

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.