hook_field_extra_fields() allows a module to define a default weight, that can be overridden by the user, but not a default visibility:

function hook_field_extra_fields() {
  $extra['node']['page']['display']['my_display'] = array(
    'label' => t('My special display'), 
    'description' => t('Displays some blah blah ...'),
    'weight' => -4,
    'visible' => FALSE, // In D8, yet not in D7
  );

  return $extra;
}

Why not allow a default visibility setting, that can be overridden by the user? As of now all extra fields are displayed by default.

Files: 
CommentFileSizeAuthor
#91 drupal-n1256368-91-FALSE.patch1.29 KBDamienMcKenna
FAILED: [[SimpleTest]]: [MySQL] 40,948 pass(es), 215 fail(s), and 84 exception(s). View
#91 drupal-n1256368-91.patch1.29 KBDamienMcKenna
PASSED: [[SimpleTest]]: [MySQL] 41,171 pass(es). View

Comments

yched’s picture

Hm, why not, but right now it would only make sense for 'display' extra elements (as opposed to 'form' elements - Field UI doesn't let you hide elements in forms)

Niklas Fiekas’s picture

Right, I didn't notice I was pasting the part with the form elements.

Niklas Fiekas’s picture

Status: Active » Needs review
FileSize
2.07 KB
PASSED: [[SimpleTest]]: [MySQL] 32,856 pass(es). View

Please review.

yched’s picture

Status: Needs review » Needs work

[edited out, I was on crack]

This looks reasonable.

yched’s picture

Status: Needs work » Needs review

Back to CNR for the testbot.

Niklas Fiekas’s picture

Status: Needs review » Needs work

Thanks for the review yched. I think you missed something there. If it is not set we want it to be TRUE. However !empty will be FALSE in that case.
You were faster editing then I was replying ;)

Niklas Fiekas’s picture

Status: Needs work » Needs review
timcosgrove’s picture

FileSize
1.24 KB
PASSED: [[SimpleTest]]: [MySQL] 35,931 pass(es). View

In the course of reviewing this, I found the patch no longer applied, probably due to other subsequent changes. Re-rolled.

wbobeirne’s picture

Status: Needs review » Reviewed & tested by the community

Confirming #8 worked for me. Makes extra fields much more viable, as now they won't insert themselves on every node type they appear in without you setting them to visible.

timcosgrove’s picture

Status: Reviewed & tested by the community » Needs review
timcosgrove’s picture

FileSize
1.2 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch drupal-7.x-extra_fields_visibility-1256368.patch. Unable to apply patch. See the log in the details link for more information. View

Adding backport patch for 7.x

Status: Needs review » Needs work

The last submitted patch, drupal-7.x-extra_fields_visibility-1256368.patch, failed testing.

Niklas Fiekas’s picture

Status: Needs work » Reviewed & tested by the community

Thank you, timcosgrove. The patch filename should contain d7 in order to test it against Drupal 7 while the issue is on Drupal 8.

We should ask webchick to see if this is backportable, but it certainly would make a nice API addition.

Moving back to RTBC as of #9.

timcosgrove’s picture

Status: Reviewed & tested by the community » Needs review
FileSize
1.2 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch d7-extra_fields_visibility-1256368.patch. Unable to apply patch. See the log in the details link for more information. View

Resubmitting the patch with a d7 prefix. Thanks! I was wondering how testing was going to happen, given that this is an 8.x issue.

The field.info.inc code is nearly identical in both cases, so I see no reason why it wouldn't be backportable. If that can't be done in this issue, I'll open a feature request against 7.x and cross-reference this issue. I will admit that my primary motivation for working on this is to use it now, in our D7 sites, so I'm more interested in the backport.

Status: Needs review » Needs work

The last submitted patch, d7-extra_fields_visibility-1256368.patch, failed testing.

timcosgrove’s picture

Status: Needs work » Needs review
FileSize
1.24 KB
PASSED: [[SimpleTest]]: [MySQL] 36,051 pass(es). View

So #13 is mistaken; this does not work, nor has it ever been the case that it worked. Please see http://drupal.org/node/1319154 for a description of how the backporting process works.

I am reattaching the d8 patch so that tests get run again against d8.

timcosgrove’s picture

Also see http://drupal.org/node/332678 for more on automated patch testing.

Niklas Fiekas’s picture

Indeed, sorry. I thought that had been added, but only "do-not-test" support is there, as of now.

iamEAP’s picture

Tagging contributed project blocker because i18n is unable to hide node language fields by default: #1350638: Language display should default to hidden when enabling "Multilingual content"

Backport would be possible since it's an API addition, which is allowed.

Alan D.’s picture

@iamEAP I'm not sure that this is a project blocker, just a huge pain in the ...! I can post the code we used to update one of our sites for two new extra fields if you are interested.

+1 for getting this in AND backporting, the update scripts to handle this would need to modify field_bundle_settings variable, which would mean that it is likely that contrib is likely to kill the display settings for all fields at least once or twice until this gets in...

Alan D.’s picture

Status: Needs review » Reviewed & tested by the community

As per #9.

tim.plunkett’s picture

Status: Reviewed & tested by the community » Needs work
+++ b/core/modules/field/field.api.phpundefined
@@ -31,6 +31,8 @@
+ *   - visible: Optional, Defaults to TRUE. The default visibility of the
+ *     display component. Not used for form components.

I think this should read

 - visible: (optional) The default visibility of the display component, defaults to TRUE. Not used for form components.

Obviously still needs to be wrapped at 80.

Niklas Fiekas’s picture

Status: Needs work » Needs review
FileSize
1.28 KB
PASSED: [[SimpleTest]]: [MySQL] 36,582 pass(es). View

Rerolled with that.

dalin’s picture

Status: Needs review » Reviewed & tested by the community

This is much needed. Currently if a new field gets added to a large site, it gets displayed everywhere and there's no sane way to disable it.

iamEAP’s picture

Speaking of #24, an almost-sane way to hide them for those who desperately need it now: hook_field_extra_fields_display_alter().

catch’s picture

Status: Reviewed & tested by the community » Needs review
Issue tags: +Needs tests, +needs backport to D7

This looks fine but can we add tests for the new feature?

Niklas Fiekas’s picture

Not without a testing module that provides an extra field that is hidden by default. Should we add one? We could use the module we need for #1593888: Tests for indentation in Field UI, or a different one.

catch’s picture

Yes this will need a testing module, it may well be possible to re-use the existing field_test module (or whatever it's called) and just add the hook implementation to it.

Niklas Fiekas’s picture

Issue tags: -Needs tests
FileSize
2.62 KB
3.91 KB
PASSED: [[SimpleTest]]: [MySQL] 36,691 pass(es). View

Ok, thanks. Added tests.

Alan D.’s picture

Status: Needs review » Reviewed & tested by the community

Looks great, thanks Niklas

Alan D.’s picture

Version: 8.x-dev » 7.x-dev
Status: Reviewed & tested by the community » Needs review
FileSize
3.55 KB
FAILED: [[SimpleTest]]: [MySQL] 39,291 pass(es), 1 fail(s), and 0 exception(s). View

Actually, two minor points.

-class ExtraFieldsTest extends WebTestBase {
+class FieldExtraFieldsTestCase extends FieldTestCase {
-      'name' => 'Extra fields tests',
+      'name' => 'Field extra fields tests',

Re-rolled patches against D7 for testing.

Alan D.’s picture

FileSize
4 KB
FAILED: [[SimpleTest]]: [MySQL] 39,254 pass(es), 2 fail(s), and 0 exception(s). View

The tests missed the third option, that this doesn't negatively impact the existing API. This patch (against 7.x atm, but will reroll to 8.x) checks that if the visibility is not specified, then the field will show.

Status: Needs review » Needs work

The last submitted patch, drupal-1256368-extra-fields-visibility-32.patch, failed testing.

tim.plunkett’s picture

Version: 7.x-dev » 8.x-dev
Status: Needs work » Reviewed & tested by the community

#29 is RTBC. Let's leave it that way, until committed.

Alan D.’s picture

K. I was going to quickly run some D7 tests then re-roll for D8 with minor text mods / additional tests. Accidentally deleted setUp() and it took a while to figure out what was going wrong. Out of time to re-roll to D8 :(

The D7 one adds all 3 use-cases, visibility specified as true, false, unspecified, and adds a test on another content type to ensure that the view modes only effect the specified bundle (there are no tests for anything related to extra fields afaict). Appending to assist backport to D7.

tim.plunkett’s picture

Status: Reviewed & tested by the community » Needs work

Well, if the D7 one has more assertions, they should be added to D8 before this goes in.

Alan D.’s picture

Status: Needs work » Needs review
FileSize
4.49 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch drupal-1256368-extra-fields-visibility-37-d8.patch. Unable to apply patch. See the log in the details link for more information. View

Only one additional assertion. I was testing the field_test module with 3 of the additional ones ;)

Status: Needs review » Needs work
Issue tags: -needs backport to D7, -Contributed project blocker

The last submitted patch, drupal-1256368-extra-fields-visibility-37-d8.patch, failed testing.

Niklas Fiekas’s picture

Status: Needs work » Needs review
Issue tags: +needs backport to D7, +Contributed project blocker

Test failure looks unrelated to me.
#37: drupal-1256368-extra-fields-visibility-37-d8.patch queued for re-testing.

swentel’s picture

So, by default, all extra fields which do not not have the visible key will be displayed, right ? Isn't the intent of the issue (which also causes a lot of frustration for site builders) that they should be hidden by default if there's no specification ? I'd personally vote for hiding them by default to be honest, unless I'm reading the code and tests completely wrong of course (it's late here).

Niklas Fiekas’s picture

I'd suggest accepting this as is, because it's the only way this is backportable. Once this has been committed to 8.x, I'll open an issue to reconsider the visibility settings of the extra fields available in D8.

Alan D.’s picture

I have to agree with Niklas, anything else would be a change in the API and be unlikely to accepted. We should do our best to no hold this issue up (this 2 liner change is nearly a year old).

I am sure that contrib will quickly update their additional field definitions.

Summary, fix to this issue is a 2 liner.

The rest are tests, for this issue and fields via hook_node_view() successfully adding content to nodes that are displayed via the default visibility configuration.

Niklas Fiekas’s picture

Status: Needs work » Needs review
Issue tags: -needs backport to D7, -Contributed project blocker

Let's see if this still applies. #37: drupal-1256368-extra-fields-visibility-37-d8.patch queued for re-testing.

Edit: Nope, it doesn't. Needs a reroll.

Status: Needs review » Needs work

The last submitted patch, drupal-1256368-extra-fields-visibility-37-d8.patch, failed testing.

webflo’s picture

Issue tags: +needs backport to D7, +Contributed project blocker
FileSize
3.44 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch hook_field_extra_fields-visibility-test-1256368-45.patch. Unable to apply patch. See the log in the details link for more information. View

I tried to reroll the patch from #37. This feature/regression was fixed in #1757504: Regression: language field is not visible on manage display. But there are no test for this feature.

rooby’s picture

This patch would be a very welcome addition.

Wouldn't it be sensible to also add label visibility as an option as well if we have visibility?

[EDIT] Sorry, just realised that extra fields don't have a label, but that's a whole separate issue.

Chi’s picture

Status: Needs review » Needs work
Issue tags: +needs backport to D7, +Contributed project blocker

The last submitted patch, hook_field_extra_fields-visibility-test-1256368-45.patch, failed testing.

Chi’s picture

Status: Needs work » Needs review
FileSize
1.82 KB
PASSED: [[SimpleTest]]: [MySQL] 53,620 pass(es). View

Rerolled patch from #45

andypost’s picture

Is this a feature or bug? Also there's a related issue #1954188: Revaluate the concept of 'extra fields'

Berdir’s picture

Status: Needs review » Needs work

The re-roll is missing the test.

Could use this as well in a 7.x project. I guess for 8.x it's a task to write tests for it as it seems to have been fixed already.

jhodgdon’s picture

Coming here from #1614108: Document hook_field_extra_fields()'s "edit" and "delete" properties... I would just like to point out that back in June of 2012, fields *did* support visibility.

The function _field_info_prepare_extra_fields():
http://drupalcode.org/project/drupal.git/blob/e039aaaa32c5053a441330b403...
supported a 'visible' element for the 'display' mode, and this was used in field_ui_display_overview_form():
http://drupalcode.org/project/drupal.git/blob/e2bcdbcedb605325e4f0889428...

So if this is not present in the current code, it may be a regression? I could also be mistaken about whether the previous functionality is what is being requested here... I didn't read all 50 comments, was just responding to David Rothstein's comment here on that other issue:
http://drupal.org/node/1614108#comment-7299436

Alan D.’s picture

@jhodgdon
Needs work for tests. The core functionality was committed accidentally in another commit.

Alan D.’s picture

Duh. It has been lost.

class FieldInfo {
  public function prepareExtraFields($extra_fields, $entity_type, $bundle) {
    ....

    // Extra fields in displayed entities.
    $data = $extra_fields['display'];
    foreach ($extra_fields['display'] as $name => $field_data) {
      $settings = isset($bundle_settings['extra_fields']['display'][$name]) ? $bundle_settings['extra_fields']['display'][$name] : array();
      $view_modes = array_merge(array('default'), array_keys($entity_type_info['view modes']));
      foreach ($view_modes as $view_mode) {
        if (isset($settings[$view_mode])) {
          $field_data['display'][$view_mode] = $settings[$view_mode];
        }
        else {
          $field_data['display'][$view_mode] = array(
            'weight' => $field_data['weight'],
            'visible' => TRUE,
          );
        }
      }
      unset($field_data['weight']);
      $result['display'][$name] = $field_data;
    }

    return $result;
  }
}

[edit] I think, completely new to this class

David_Rothstein’s picture

@jhodgdon, @Alan D., I don't understand. Those code examples show that the default 'visible' setting is hardcoded to TRUE, don't they? But the point of this issue is to allow it to be provided by hook_field_extra_fields() (so that people defining new "extra fields" can start them off as 'visible' = FALSE if they want to).

That change was committed to Drupal 8, but I don't see any evidence that it was ever in Drupal 7.

Alan D.’s picture

Quick look at the D7 git history didn't show anything.

AFAICT, this never got to that point to backport, as this was never finalized for D8 due to lack of tests. And this lack of tests allowed the committed code to be reverted if I am reading the latest 8.x branch code correctly. So now neither branches appear to have this. :(

jhodgdon’s picture

RE #55... See #52 (the links are to what was committed about a year ago in May). It looks to me as though if you pass in 'visible' => TRUE or 'visible' => FALSE in the version of _field_info_prepare_extra_fields() as of that date, then it is used/return, but if you don't set up a display mode, 'visible' is set to TRUE. Right? And that function is called from the collate function
http://drupalcode.org/project/drupal.git/blob/e039aaaa32c5053a441330b403...
which invokes hook_field_extra_fields() to get the information to pass to _field_info_prepare_extra_fields(). So I think as of about a year ago, this was possible to do with the hook. Right?

David_Rothstein’s picture

@Alan D., you might be right, I'm not seeing any evidence of it in Drupal 8 either right now. I guess the tests can prove that, one way or another...

David_Rothstein’s picture

@jhodgdon, what I'm seeing in that code from May 2012 (leaving out the surrounding code) is:

  foreach ($extra_fields['display'] as $name => $field_data) {
...
    if (isset($settings[$view_mode])) {
      $field_data['display'][$view_mode] = $settings[$view_mode];
    }
    else {
      $field_data['display'][$view_mode] = array(
        'weight' => $field_data['weight'],
        'visible' => TRUE,
      );
    }

$extra_fields contains the default settings passed in from the hook, but $settings comes from somewhere else (I believe that comes from the database, i.e. for existing fields it respects whatever was saved in the admin UI most recently, but for new fields it won't be there). And no matter what, the display settings on the $field_data variable are getting overwritten.

In the case of 'weight', they can be overridden based on the value that was provided in $extra_fields, but in the case of 'visible', they can't?

jhodgdon’s picture

Duh, you are correct. I read that code several times and glossed over that.

So we should go back to that docs issue and remove 'visible' from the hook docs for both 7 and 8 currently then?

Alan D.’s picture

Tracked this down to #1852966: Rework entity display settings around EntityDisplay config entity.

Posted there to ask for some feedback here (without reopening, so hopefully someone will trace this here)

David_Rothstein’s picture

@jhodgdon: Hah, well, I've certainly been in that position before too :)

So I decided to check if it works in Drupal 8, and although I can't figure out where in the code is making it work, it does actually seem to. For example, node_field_extra_fields() sets 'visible' => FALSE on the Language form element, and it correctly shows up as hidden on the Manage Display screen. And if you remove that line of code and clear caches, it goes back to being un-hidden.

So I think for Drupal 8 the issue here just needs tests after all (only Drupal 7 needs the functionality)... I'll go back and change the docs issue accordingly also.

manatwo’s picture

FileSize
1.18 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch d7-extra_fields_visibility-1256368-64.patch. Unable to apply patch. See the log in the details link for more information. View

Posting a new D7 patch in case anyone needs it.

jpstrikesback’s picture

Status: Needs work » Needs review

fire up the bot

Status: Needs review » Needs work

The last submitted patch, d7-extra_fields_visibility-1256368-64.patch, failed testing.

jhodgdon’s picture

Please do not post Drupal 7 patches on Drupal 8 issues until the 8.x issue is resolved.

David_Rothstein’s picture

I think it's OK to post a Drupal 7 patch but it's best to follow the instructions for how to do so that appear on the file upload field:

For patches that should not be tested by the testbot (for example, patches that do not work with the current API version in the issue), add "do-not-test" before the .patch. For example "drupal.do_nothing_555555-do-not-test.patch"

Although in this case the issue was already "needs work" so the testbot wasn't supposed to run on it anyway :)

Posting the Drupal 7 patch somewhere on drupal.org is also a requirement if anyone is going to use it in a Drupal distribution before it's committed (see https://drupal.org/node/1475972) and while it could be posted in another issue to reduce noise, I think it's better to keep it in one place?

jpstrikesback’s picture

apologies :)

theunraveler’s picture

Re-rolled for 7.23.

GoZ’s picture

7.23 is passed and patch wasn't included. Do we have to open new issue for d7 and keep this for d8 so patch can be committed ?

jhodgdon’s picture

Can someone please update the issue summary so we know what needs to be done for both Drupal 7 and Drupal 8?

If the patches are substantially similar for both, our policy is normally that bugs have to be fixed in 8 before fixes can be committed to 7.

If the patches/code is substantially different in 7, we might consider branching off to a separate issue. I'm just not sure what the plan is for 8 and 7 and the issue summary doesn't illuminate the issue at all.

kristiaanvandeneynde’s picture

Patch in #70 looks good to me for D7.

kristiaanvandeneynde’s picture

Comparing the following two (correct file for D8?)

I'd say the changes in D8 are vast enough for this to be separated into a D7 and a D8 issue. Judging from #62 this now works for D8 but needs only testing.

Can we get the patch from #70 into D7 please? :)

jhodgdon’s picture

OK, feel free to (a) create a separate D7 issue (or a separate D8 issue?) and (b) update the issue summary for both and (c) make sure the issues are linked to each other.

kristiaanvandeneynde’s picture

Title: Add default visibility setting to hook_field_extra_fields() » Add 'visible' key to hook_field_extra_fields()
Version: 8.x-dev » 7.x-dev
Issue summary: View changes
Status: Needs work » Reviewed & tested by the community
Issue tags: -Needs issue summary update, -needs backport to D7
Related issues: +#2165879: Write tests for 'visibility' of fields exposed with hook_entity_extra_field_info()

Split off the D8/D7 tests into their own issue.

jhodgdon’s picture

Status: Reviewed & tested by the community » Needs work
Issue tags: +Needs tests, +Needs issue summary update

OK, so this is now just a D7 issue, but there is still no real issue summary. And we do not commit patches to D7 or D8 without tests -- the proposed D7 patch has not even been run though the test bot, and it has no test for the proposed change.

It is fine to make a separate issue for D8 but it is not fine to say that making the test for new behavior is a separate issue. We need tests for reviewers to verify the new code is OK.

kristiaanvandeneynde’s picture

I understand your point about the necessity of tests. It's a bit difficult in this case because the functionality is, according to #62, already in D8 without having tests written for it.

Seeing as we will not backport the D8 functionality (because it's a complete rewrite?), I figured we could already get this into D7 while others work on the tests for both versions. Of course this is silly, because it could cause regressions.

I just cloned a copy of D8 to do some grep magic on and perhaps find a test or comment left or right that may point us in the right direction.

In all fairness: I did update the issue summary, just nothing ground-breaking. How would you like to see it updated?

klonos’s picture

...hiding all patches except the last one.

kristiaanvandeneynde’s picture

Found it in D8! :)

The extra fields now use EntityDisplay objects along with regular fields: https://drupal.org/node/1875952

So I did some digging and found the D8 functionality in Drupal\field_ui\DisplayOverviewBase::buildExtraFieldRow(). See the code (line 447).

So it could very well be that there are tests already for D8, but that they now are more generic tests for EntityDisplay objects.

(Cross-posting in related issue)

jhodgdon’s picture

I guess the issue summary is good enough. We do have a template. :) This issue went through a lot of iterations about whether it was a bug, a docs bug, a feature request, etc. but the current summary does seem to reflect what we want to happen.

Anyway, we do need a test if we want to get this into D7.

And at this point, since it's not a purely documentation issue, I'm going to stop following it and let the maintainers of this sub-system and/or the D7 maintainers weigh in on whether they would accept such a feature request this late in the D7 game.

NancyDru’s picture

This works fine for me on 7.22, tests or no tests.

tim.plunkett’s picture

Status: Needs work » Needs review
FileSize
1.34 KB
FAILED: [[SimpleTest]]: [MySQL] 41,102 pass(es), 212 fail(s), and 86 exception(s). View
1.34 KB
PASSED: [[SimpleTest]]: [MySQL] 40,598 pass(es). View

That conditional made me go cross-eyed, so I rewrote it a bit.

I might have time this week to look at a test...

I'm going to update a version of this that uses FALSE as the default just to see if we have any coverage at all.

The last submitted patch, 83: extra-fields-visible-1256368-83-FALSE.patch, failed testing.

tim.plunkett’s picture

So we do have lots of implicit test coverage.
We just need explicit tests that setting it to FALSE does work.

Fabianx’s picture

Status: Needs review » Needs work

Besides the missing test I think this is RTBC :).

ordermind’s picture

Subscribing, this is an important issue.

The last submitted patch, 83: extra-fields-visible-1256368-83-FALSE.patch, failed testing.

Jaypan’s picture

Ordermind - you don't need to post to subscribe to topics, you can just click the 'follow' (or subscribe?) button at the top of the thread, underneath the issue information.

DamienMcKenna’s picture

Status: Needs work » Needs review
FileSize
1.29 KB
FAILED: [[SimpleTest]]: [MySQL] 40,948 pass(es), 215 fail(s), and 84 exception(s). View
1.29 KB
PASSED: [[SimpleTest]]: [MySQL] 41,171 pass(es). View

Rerolled.

The last submitted patch, 91: drupal-n1256368-91-FALSE.patch, failed testing.

iampuma queued 91: drupal-n1256368-91.patch for re-testing.

Fabianx’s picture

Status: Needs review » Reviewed & tested by the community

The last submitted patch, 91: drupal-n1256368-91-FALSE.patch, failed testing.

jcisio’s picture

Aren't tests always missing?

GoZ’s picture

Tests with FALSE by default drupal-n1256368-91-FALSE.patch will always fail since in their tests, modules assume extra field is displayed by default.

jcisio’s picture

Status: Reviewed & tested by the community » Needs work

Re #98 I meant, see #85. We need PASSED tests when the default value is FALSE.

pjcdawkins’s picture

+++ b/modules/field/field.api.php
@@ -35,6 +35,8 @@
+ *   - visible: (optional, defaults to TRUE) The default visibility of the

Just a small note: this comment should obviously say FALSE if the default value is FALSE

Alan D.’s picture

I think Tim means that test will need to be added to the actual patch to ensure that this is actually covered, rather than generating a patch that fails :)

The code and reviews are rtbc, the doco is correct, this is just missing some test to check that:

a) The fields still show that are already define
b) Adding a new field with visibility set to FALSE does not show

While imho, FALSE default makes sense, that would be an massive API change in this area, so this needs to be TRUE by default. Which the second patch does (drupal-n1256368-91.patch).

tim.plunkett’s picture

#101.b is what I meant.
Leave the default as is, but we need a test showing that a declaration of FALSE does what we expect.

David_Rothstein’s picture

Status: Needs work » Needs review

Yes, as a new feature this really should have tests, especially since it looks like they are already in Drupal 8 (at least to some extent); see discussion in #2165879: Write tests for 'visibility' of fields exposed with hook_entity_extra_field_info(). That could be the basis for some Drupal 7 tests. Or the basis could be many of the earlier patches in this issue, many of which look like they included tests in the patch, but then those got lost in later patches...?

David_Rothstein’s picture

Status: Needs review » Needs work

Oops.

gabesullice’s picture

Until this patch is finished, I just wanted to leave this here for anyone who comes looking for an answer on how to hide an extra field by default. Below is a D7 snippet to hide newly created extra fields when they're defined for the first time. With it, you won't need to wait on this patch to be committed or patch core yourself.

<?php
/**
 * Utility function to hide any newly created extra_fields.
 *
 * @param $entity_type string
 *   Entity type on which the new extra field will reside.
 *
 * @param $bundle string
 *   Bundle on which the new extra field will reside.
 *
 * @param $field_name string
 *   Machine name of extra field.
 */
function mymodule_hide_new_extra_field($entity_type, $bundle, $field_name) {
  $settings = field_bundle_settings($entity_type, $bundle);
  $settings_updated = FALSE;
  $entity_info = entity_get_info($entity_type, $bundle);
  $view_modes = array_keys($entity_info['view modes']);
  if (!in_array('default', $view_modes)) $view_modes[] = 'default';
  foreach ($view_modes as $view_mode) {
    if (!isset($settings['extra_fields']['display'][$field_name][$view_mode])) {
      $settings_updated = TRUE;
      $settings['extra_fields']['display'][$field_name][$view_mode] = array(
        'weight' => 100,
        'visible' => FALSE,
      );
    }
  }
  if ($settings_updated) {
    field_bundle_settings($entity_type, $bundle, $settings);
  }
}
?>