Modules can alter a widget and some of its settings via hook_field_widget_properties_alter.

Some settings though are in field instance settings and can not be altered right now. Examples:
* maximum number of values (note: don't get this wrong, this is not about field schema cardinality)
* widget type for widgets like og_complex that do kinda chaining

Simply passing settings by reference in $context allows modules to alter these.

(D8 killed that hook per pluginification)

Original summary

Original title: Allow cardinality to be restricted (overriden and reduced) in the field instance settings

It is Impossible to override the field cardinality for a specific instance even when set to unlimited. When a field's cardinality is set to unlimited it shouldn't be a problem when it is overridden to a fixed number for a specific instance.

This functionality would be very handy. An example would be the audience field of the Organic groups module; you can't limit the number of groups for one entity type while keeping it unlimited for another.

Or when using for example views you might want to list nodes of several types including a icon. It would be handy if you could use the same field. But what if you want to limit the number of images for the one node type while allowing multiple images for another.

Unfortunately getting this to work is impossible. There are no suitable hooks (I found one way, but that one is very very hacky).

Of course it would be possible for the field defining module to include a instance setting to limit the cardinality. Also a wrapper widget would be possible, except it might give unexpected results for widgets that handle multiple values themselves.

My request is to provide the possibility to limit the cardinality for field instances (when the field cardinality is set to unlimited) for any field type.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

yched’s picture

Version: 7.x-dev » 8.x-dev

Ouch. I'm afraid that's D8 material, at this point :-(.

Alan D.’s picture

Title: Impossible to override the field cardinality even when set to unlimited. » Move cardinality settings from the field to the instance settings

Sub.

Mmmm.. any hints of a safe way of doing this? From a storage point of view, there doesn't appear to be any issues or reasons for this to be a field setting anymore.

My first thoughts were to set the value to max / unlimited and to block access to the add more button with a form alter on the types that have singular restrictions, but is there a better way?

Alan D.’s picture

The closest thing that I could find was in field_default_form(), but it needs a core hack to get it to work:

  // Let modules alter the widget properties.
  $context = array(
    'entity_type' => $entity_type,
    'entity' => $entity,
    'field' => $field,
    'instance' => $instance,
  );
  drupal_alter(array('field_widget_properties', 'field_widget_properties_' . $entity_type), $instance['widget'], $context);

This is the perfect position to alter the cardinality before the instance is rendered, but there is no way to hook into it. However, if the field is passed in by reference, you can use these hooks to alter it in context to the instance:

i.e.

  // Let modules alter the widget properties.
  $context = array(
    'entity_type' => $entity_type,
    'entity' => $entity,
    'field' => &$field, ## << This adds the reference allowing field modification in relation to the instance
    'instance' => $instance,
  );
  drupal_alter(array('field_widget_properties', 'field_widget_properties_' . $entity_type), $instance['widget'], $context);

Then it is trivial to use the hook to update this.

Any chances of changes like this getting into Drupal 7 to allow this to be done in contrib?

Alan D.’s picture

And to provide a working example. here was a quick hack at a module that provides this functionality.

The module was called portfolio (an unrelated project):


/**
 * Implements hook_field_widget_properties_alter().
 *
 * Requires the core hack to work.
 */
function portfolio_field_widget_properties_alter($widget, &$context) {
  $field = &$context['field'];
  $instance = $context['instance'];
  $cardinality = portfolio_cardinality($instance);
  if ($cardinality && $cardinality != $field['cardinality']) {
    $field['cardinality'] = $cardinality;
  }
}

/**
 * Implements hook_form_FORM_ID_alter().
 */
function portfolio_form_field_ui_field_edit_form_alter(&$form, &$form_state, $form_id) {
  $instance = $form['#instance'];
  $field = $form['#field'];
  $options = array(
    0 => t('Do not override'),
  ) + drupal_map_assoc(range(1, min(array($field['cardinality'] > 0 ? $field['cardinality'] : 10, 10))));
  $form['instance']['instance_cardinality'] = array(
    '#type' => 'select',
    '#title' => t('Number of values'),
    '#options' => $options,
    '#default_value' => portfolio_cardinality($instance),
    '#description' => t('This option overrides the field setting. The number of field values should always exceed the number of instance values.'),
  );
}

/**
 * Helper function to get the cardinality.
 */
function portfolio_cardinality($i) {
  $settings = variable_get('portfolio_cardinality', array());
  if (isset($settings[$i['entity_type']][$i['bundle']][$i['field_name']])) {
    return $settings[$i['entity_type']][$i['bundle']][$i['field_name']];
  }
  return 0;
}

/**
 * Implements hook_field_create_instance().
 */
function portfolio_field_create_instance($i) {
  $settings = variable_get('portfolio_cardinality', array());
  $settings[$i['entity_type']][$i['bundle']][$i['field_name']] = $i['instance_cardinality'];
  variable_set('portfolio_cardinality', $settings);
}

/**
 * Implements hook_field_update_instance().
 */
function portfolio_field_update_instance($i, $prior_instance) {
  $settings = variable_get('portfolio_cardinality', array());
  $settings[$i['entity_type']][$i['bundle']][$i['field_name']] = $i['instance_cardinality'];
  variable_set('portfolio_cardinality', $settings);
}

/**
 * Implements hook_field_delete_instance().
 */
function portfolio_field_delete_instance($i) {
  $settings = variable_get('portfolio_cardinality', array());
  unset($settings[$i['entity_type']][$i['bundle']][$i['field_name']]);
  variable_set('portfolio_cardinality', $settings);
}

The last three hooks are used to save the value as I have yet to find a way to add a submit handler directly to the form alter.

sun’s picture

Status: Active » Closed (won't fix)

Actually, I'm pretty sure that this won't fix. Field cardinality affects the field schema, and the same field schema (storage) is used across field instances.

There are valid field storage engine implementations that may actually rely on the fixed cardinality. Prime example being http://drupal.org/project/pbs (which mimics the CCK/D6 approach)

Alan D.’s picture

The general schema in Drupal 7 for the main storage engines does not appear to change per engine, so a project that appears unused does not seem like a very good reason for killing the idea. With some one for contrib to do this would simply make this idea possible, and users can use it at their own risk.

ZhukV’s picture

Version: 8.x-dev » 7.8
Priority: Normal » Major
Issue tags: +field

One solution to the problem. (Hack drupal core - field.attach)

In field.attach.inc file replace function _field_invoke to:

function _field_invoke($op, $entity_type, $entity, &$a = NULL, &$b = NULL, $options = array()) {
  // Merge default options.
  $default_options = array(
    'default' => FALSE,
    'deleted' => FALSE,
    'language' => NULL,
  );
  $options += $default_options;

  // Determine the list of instances to iterate on.
  list(, , $bundle) = entity_extract_ids($entity_type, $entity);
  $instances = _field_invoke_get_instances($entity_type, $bundle, $options);

  // Iterate through the instances and collect results.
  $return = array();
  foreach ($instances as $instance) {
    $field_name = $instance['field_name'];
    $field = field_info_field($field_name);
    $function = $options['default'] ? 'field_default_' . $op : $field['module'] . '_field_' . $op;
    if (function_exists($function)) {
      // Determine the list of languages to iterate on.
      $available_languages = field_available_languages($entity_type, $field);
      $languages = _field_language_suggestion($available_languages, $options['language'], $field_name);
      foreach (module_implements('_field_invoke') as $module) {  #<<< Added hook hook__field_invoke, for edit $field
        $funct = $module . '__field_invoke';
        $funct($op, $entity_type, $entity, $field, $options); #<<< Call hook
      }
      foreach ($languages as $langcode) {
        $items = isset($entity->{$field_name}[$langcode]) ? $entity->{$field_name}[$langcode] : array();
        $result = $function($entity_type, $entity, $field, $instance, $langcode, $items, $a, $b);
        if (isset($result)) {
          // For hooks with array results, we merge results together.
          // For hooks with scalar results, we collect results in an array.
          if (is_array($result)) {
            $return = array_merge($return, $result);
          }
          else {
            $return[] = $result;
          }
        }

        // Populate $items back in the field values, but avoid replacing missing
        // fields with an empty array (those are not equivalent on update).
        if ($items !== array() || isset($entity->{$field_name}[$langcode])) {
          $entity->{$field_name}[$langcode] = $items;
        }
      }
    }
  }

  return $return;
}

Example use.

/**
 * Implementation hook__field_invoke
 *   [HACK field.attach.inc - 195-198 lines]
 */
function mm_all__field_invoke($op, $entity_type, $entity, &$field, $options) {
  if ($entity_type == 'node' && $entity->type == 'resume') {
    switch ($field['field_name']) {
      case 'field_image':
        $field['cardinality'] = 1;
        break;
    }
  }
}
Alan D.’s picture

Version: 7.8 » 8.x-dev
Status: Closed (won't fix) » Active

Sadly, definitely D8. But I'm all for the idea!

sun’s picture

Priority: Major » Normal
Issue tags: -field

Again: Cardinality has to be fixed per field across all instances, because field storage engines need to be able to query the data in an efficient way.

I can only guess what you're really after here is an artificial or "soft" limitation of field cardinality to only allow less items on a specific instance, whereas the field itself allows for more items (or possibly unlimited).

If that is the case, a proper issue title would be nice.

Alan D.’s picture

Title: Move cardinality settings from the field to the instance settings » Allow cardinality to be restricted (overriden and reduced) in the field instance settings

If that is an acceptable solution from the core team, it would save some (minor) pain in the community :)

patrickd’s picture

+1

alexkb’s picture

I've spun up a quick sandbox project that kind of gets around this for Drupal 7.x without hacking core or delving into drupal's entity and field logic. Check it out here: http://drupal.org/sandbox/alexkb/1549896 and let me know of any feedback.

Drupa1ish’s picture

#7 is the cleanest solution, even is a core hack

joachim’s picture

Status: Active » Needs review
FileSize
1.39 KB

It can be done even more simply than #7, and with a one line change to core:

  // Let modules alter the widget properties.
  $context = array(
    'entity_type' => $entity_type,
    'entity' => $entity,
    'field' => &$field, // <-- pass $field by reference
    'instance' => $instance,
  );
  drupal_alter(array('field_widget_properties', 'field_widget_properties_' . $entity_type), $instance['widget'], $context);

Though for consistency of course we should pass $instance by reference too.

Then a contrib module just has to change the field cardinality in hook_field_widget_properties_alter():

  if ([some test on the field settings]) {
    $context['field']['cardinality'] = 1;
  }

I've just tested this on D7 and it works perfectly.

Is this something we could backport to D7? It does make the hook name something of a misnomer, as it can now alter a lot more than widget properties, but it wouldn't break anything.

joachim’s picture

I made a proof-of-concept contrib module to work with this patch, which I've released as it can work in the meantime with options widgets: http://drupal.org/project/field_instance_cardinality

Uncomment the hook_field_widget_properties_alter() to make it work with this patch.

Alan D.’s picture

Hi Joachim, just wondering, why only the unchanged field cardinality or one setting?

The comment in #4 had both the code from the patch and a working example for reducing from the field settings from 1 to field cardinality limit, albeit needing some minor work.

joachim’s picture

Huh. This is what happens when I don't read the full issue -- I see now you suggested the exact same change to core that I just did!

AFAICT your module code requires the core hack to work. What I've posted does less, but doesn't require the hack.

johnv’s picture

I am glad I found this issue. I marked my 'orphan' as duplicate:
#1375138: Make "Number of values" a per-instance-setting (instead of global)

Recapping contrib modules that try to to the same/similar thing:
#12: http://drupal.org/sandbox/alexkb/1549896
#15: http://drupal.org/project/field_instance_cardinality
new: http://drupal.org/project/multivalue_settings

I am not sure which above version (from comment or module) to test.

The 'core hack' opens up the cardinality from the field to the instance. I seems to me that you would not need a contrib module to change this value: can't you create a new 'cardinality'value on the instance value, assuringit is always lower then the field settings?

joachim’s picture

> I seems to me that you would not need a contrib module to change this value: can't you create a new 'cardinality'value on the instance value, assuringit is always lower then the field settings?

No, because widgets look at the $field info array to get the cardinality.
The core patch allows this to be changed for an individual widget in the alter hook.

johnv’s picture

I did the following test using Field instance cardinality as a contrib module.

- Set field cardinality to 4, set instance cardinality to 2.
- Edit content, set 6 values: error " this field cannot hold more than 4 values."
- Edit content, set 3 values: no error, the 3 values are saved.

So, IMO the patch in #14, might be complete, but should be completed by this flaw in another part of field module.

Alan D.’s picture

Edit content, set 6 values: error " this field cannot hold more than 4 values."

No, that's perfect. The field has block anything above the limit. Apparently non-sql storage engines may not be able to handle anything other than the limit imposed (see sun's comment above in #5). This limit is fixed and any implementation that alters this can not bypass this limit.

[EDIT] I'm am assuming that when you said set 6, you were altering the instance settings?

johnv’s picture

Hi Alan,
no, 4 was the field setting, 2 was the instance setting. I expect:
in the 1st case: "this field cannot hold more than 2 values." (now: '4 values')
in the 2nd case: "this field cannot hold more than 2 values." (now: no error)

My test implies that the check 'maximum number of values' does NOT use the changed instance settings, but still uses the old field settings.

nevergone’s picture

And now…?

pvhee’s picture

I backported #14 to a patch that applies to Drupal 7.16. I know this won't get committed to core, but it provides a very useful and relatively clean fix for controlled customization of the widgets.

Status: Needs review » Needs work

The last submitted patch, 1029298-drupal.field_widget_properties_alter-reference-24.patch, failed testing.

Pls’s picture

Status: Needs work » Needs review

Status: Needs review » Needs work

The last submitted patch, 1029298-drupal.field_widget_properties_alter-reference-24.patch, failed testing.

Jarode’s picture

Any news ?

mototribe’s picture

this would be a useful feature

geek-merlin’s picture

Status: Needs work » Needs review
geek-merlin’s picture

Issue summary: View changes

updated summary

geek-merlin’s picture

Title: Allow cardinality to be restricted (overriden and reduced) in the field instance settings » Pass field&instance settings BY REF to hook_field_widget_properties_alter to let it fiddle
Version: 8.x-dev » 7.x-dev
FileSize
1.8 KB

Updated summary and title. Hope this makes clear what this is about.
Also the old title caused some concerns basing on misunderstandings (that this patch messed with field schema).

Fortunately the hook this is all about died in D8 by pluginification, so this is a D7 issue now. Re-rolled against current 7.x.

Hope we can get this reviewd and committed now.

geek-merlin’s picture

Note: Testbot lights have been GREEN for #14 before i blindly re-tested in #30.

geek-merlin’s picture

geek-merlin’s picture

Re-rolled #14 to current D7 dev.

geek-merlin’s picture

Angry Dan’s picture

FileSize
2.52 KB

I've been using this patch for a while, and it's perfect for what I needed (altering cardinatlity)

The attached patch also allows affecting the default value by moving the call to field_get_default_value to after the drupal_alter().

Status: Needs review » Needs work

The last submitted patch, 1029298-36.patch, failed testing.

jpstrikesback’s picture

FileSize
2.45 KB

Re-roll of 36 to see test results

jpstrikesback’s picture

Status: Needs work » Needs review

Status: Needs review » Needs work

The last submitted patch, 1029298-36_0.patch, failed testing.

jpstrikesback’s picture

Status: Needs work » Needs review
FileSize
2.39 KB

And now a proper re-roll

Jay.Chen’s picture

ericsol’s picture

Hi,

I've tested the patch from #41. I might be wrong but I couldn't get it working unless I changed the AHAH behavior by changing $field to $instance on lines 185, 192 and 268 in field.form.inc. Otherwise in the forms only one field and the add-more button are shown based on the FIELD_CARDINALITY_UNLIMITED from the field, not the instance.

If this is correct I'd be happy to add it to the patch from #41.

ericsol’s picture

Issue summary: View changes

d8 notice

guillaumev’s picture

I've tested the patch from #41 in order to be able to add field collection items to a form programmatically (see https://drupal.org/node/2196347 for the use case) and it WORKED beautifully ! Would love to see this committed.

anon’s picture

Would love to see this committed too.

geek-merlin’s picture

So is this RTBC as of #44 or needswork as of #43 (which i don't really grok)?

arosboro’s picture

I use #41 here and it's definitely working for my needs.

geek-merlin’s picture

Status: Needs review » Reviewed & tested by the community

OK, the patch itself is a nobrainer, and as of #47 we should finally be able to get this committed.

David_Rothstein’s picture

Status: Reviewed & tested by the community » Needs review

This doesn't look like a safe change to me; the passed-in variable was previously local so code could easily have made changes to it without worrying. Suddenly changing these variables to be passed by reference could therefore really break things.

It's also a bit of a sledgehammer approach; there are certainly a lot of things in $field and $instance that should never be altered. Can't we look for a more direct solution to the original problem?

And can anyone confirm that this is really no longer an issue in Drupal 8? (Just because the hook doesn't exist in Drupal 8 doesn't mean that its replacement actually provides a way to achieve the goal of this issue.)

sun’s picture

@David_Rothstein: Fields (and "field instances") are classed objects in D8 now, so they are pointers ("references") by design and by definition of PHP core language constructs. If anything, D8 would have to ensure that objects are cloned before being passed off to hooks and stuff (if we'd care). "Passed by reference" is a construct that mainly exists for arrays only.

I'm ambivalent on your concerns though. I agree that there could be some code that unintentionally manipulates the passed-in data. OTOH, it sounds just natural for an alter hook to be able to alter things, doesn't it? :-)

It almost sounds we need to make a general, abstract, higher-level decision of whether "API changes to suddenly pass stuff by reference" in (legacy) stable releases is allowed or not, because this issue certainly won't be the last one of its kind...

David_Rothstein’s picture

Title: Pass field&instance settings BY REF to hook_field_widget_properties_alter to let it fiddle » Allow cardinality to be restricted (overriden and reduced) in the field instance settings

Re Drupal 8, part of the problem here may be the issue title. Restoring the earlier one (so we can focus on the problem rather than a possible solution).

rudiedirkx’s picture

Works like a charm. Such a simple improvement.

I don't know about the 'safeness'. It being a developer oriented CMS, I think alterability everywhere is good. That's up to the smart people.

geek-merlin’s picture

Title: Allow cardinality to be restricted (overriden and reduced) in the field instance settings » Allow field instance cardinality to be altered
Status: Needs review » Needs work

Changing title to one i would have understood ;-)

As of sun #50:
> It almost sounds we need to make a general, abstract, higher-level decision of whether "API changes to suddenly pass stuff by reference" in (legacy) stable releases is allowed or not, because this issue certainly won't be the last one of its kind...

...and david #49:
> This doesn't look like a safe change to me; the passed-in variable was previously local so code could easily have made changes to it without worrying.

Completely independent from this issue i would call such code broken in the first place as it relies on something it mustn't.
Clarifying "Never alter passed parameters" in developer docs might be a good thing though.

To solve david's concerns from 49 pragmatically, we might - instead of changing existing context array members - add new context array members by ref, as i see it "cardinality" and "required" (more?). So setting needswork.

Alan D.’s picture

Do you mean like this? Seems a bit hacky...

   $context = array(
     'entity_type' => $entity_type,
     'entity' => $entity,
     'field' => $field,
     'instance' => $instance,
+   'references' => array(
+      'field' => &$field,
+      'instance' => &$instance,
+   ),
   );
geek-merlin’s picture

Something like this would include davids points:

   $context = array(
     'entity_type' => $entity_type,
     'entity' => $entity,
     'field' => $field,
     'instance' => $instance,
+   // All the properties that might want to be altered.
+   'alterable' => array(
+      'cardinality' => &...,
+      'widget_type' => &...,
+      'required' => &...,
+      ...
+   ),
   );
Alan D.’s picture

My 2Cs on this is that I recon that it is all (instance/field) or none; if you change the widget type for example, chances are you would have to change other settings too. Specifically adding 'cardinality' implies that it is ok to change this, albeit things could explode and that this is a very edge case workaround that the core team would not want to support ;)

solotandem’s picture

Title: Allow field instance cardinality to be altered » Move cardinality setting from the field to the instance definition
Status: Needs work » Needs review
FileSize
13.46 KB

As everyone reading this issue probably knows, there are other issues out there requesting this functionality, namely that cardinality be a field instance setting. Comments on these issues as well as my own gut feel tells me pretty much every site of any substance has reason to reuse a field and change the cardinality. (Given that Drupal is aiming at the enterprise, that would imply every site unless you are claiming some to lack substance.)

People want this to work this way because it intuitively makes sense. All data is instance based; there is no 'field' level data. (If you disagree, please provide a valid example.) It follows from this that the cardinality setting belongs on the instance.

Field reuse is significantly hampered by a cardinality limit at the field level instead of the instance. If some theoretical storage method wants to vary the storage details based on the cardinality of all instances of a field, then it should deal with that edge case. Having the setting apply at the instance level does not prevent such a storage implementation (like 'per bundle storage'). It just means it needs to handle the oddities of its implementation in a slightly different manner.

As the attached patch demonstrates, the SQL storage implementation provided by core (which is the overwhelming implementation in use) does not rely on cardinality being a field setting. In other words there are no queries that depend on cardinality at the field level and that would break if this setting is moved to the instance.

The lone derailing comment weakly states, "There [are] valid field storage engine implementations that [may] actually rely on the fixed cardinality." There "are" that "may" -- which is it? Is there a valid example? (None are mentioned on this issue.) Even so, is there a valid example that can not be implemented when the cardinality setting is at the instance level?

Due to this derailing comment, the "solution" being offered up focuses on exposing the instance definition and allowing other code to set the cardinality value on an instance basis at runtime. Instead of attacking the problem head on (with a direct solution as suggested by David in #49), we are being asked to write "configuration" code in custom and contributed modules. Instead of a direct solution, we are suggesting lots of field modules implement this alter hook to provide what should properly be provided by core.

How does this square with the configuration initiative for D8? The derailed solution is putting configuration in non-exportable code instead of in standard configuration (i.e., database in D7 and files in D8). Lots of ugly, hard to maintain statements like:

if ([some test on the field and instance settings]) {
  $context['field']['cardinality'] = [new value];
}

The derailed approach voids the field user interface and prevents export with CTools or Features. Cloning a field in code would fail to pick up the cardinality value set in the alter hook for the original field. All in all, this approach makes it easy to trip up oneself.

To implement the derailed solution would make contributed modules more dependent on core API changes (since they would be implementing its API through a hook), not less.

Regarding the attached patch

For those primarily interested in having this functionality on their sites, the attached patch is sufficient AFAIK. It allows the cardinality to be set on the instance in the user interface. It reads and writes the settings and the content respecting this cardinality setting.

Until the direction of this issue is restored, it makes no sense to complete the patch to address things like:

  • The patch does not update the tests, nor add one to test cardinality at the instance level.
  • The patch retains the cardinality setting at the field level to avoid dealing with schema changes (and to placate those unspecified storage implementations that consider it necessary to be there).
  • The patch does not implement a hook_update_N() to move cardinality to the instance.
  • The patch does not enforce consistency in the cardinality value at field level versus instance. In other words, a user may set this value as one on the field and unlimited on an instance; that is allowed as the patch does not care about the field setting but relies on each instance value.
  • The patch contains a 'todo' comment that ventures beyond column 80.

Basically, the patch works for those who need to build sites efficiently but, pending approval of this approach, needs work before being committed. If this issue continues to be derailed, then that justifies submitting the patch in this state.

Pros

  • intuitively makes sense
  • agrees with data being instance based
  • consistent with configuration initiative of D8
  • does not break some theoretical use case
  • avoids identical and less maintainable junk code in many custom and contributed modules
  • avoids making contributed modules more dependent on core API changes
  • future-proof

Cons

are there any?

Status: Needs review » Needs work

The last submitted patch, 57: 1029298-move-cardinality-setting-57.patch, failed testing.

rudiedirkx’s picture

I very much agree, but that patch will never get it. I myself use the patch from #41 and this module.

solotandem’s picture

Status: Needs work » Needs review
FileSize
13.62 KB

Updated patch handles deleted fields and initializes cardinality on instance when exported.

Status: Needs review » Needs work

The last submitted patch, 60: 1029298-60-move-cardinality-setting.patch, failed testing.

schifazl’s picture

IMHO this functionality is really important when building web applications based on Drupal. I can test it, but I'm confused a little on which patch should I try: #41 or #60?

rudiedirkx’s picture

@schifazl They're different.

  • #41 allows per-instance cardinality technically/programmatically, but not in config or with UI.
  • #60 adds the config, fixes existing fields, adds UI etc.

IF this functionality would get in core (it won't), it would be #60-style, but for now, I like #41 better, because it's simpler and all you need is hook_field_widget_properties_alter() in your custom code, or use this sandbox module.

schifazl’s picture

@rudiedirkx thanks for your reply. Why are you saying that it won't get committed in core?

rudiedirkx’s picture

@schifazl There's a reason not all fields are unlimited and the config is on the field instead of the instance. I don't remember what it is. (Something with the pluggable field storage system, maybe..?) The same reason still applied in D8, so it won't change. I thought it wasn't a good reason, but I'm not the boss =) Search for it if you want the detailed reason.

geek-merlin’s picture

+1 for reverting to the original issue for this reason.

Anonymous’s picture

This is really a nice api feature for developers, #41 work well for me and i hope will be committed in core

johnv’s picture

I wanted to use this feature in D7, to be able to use the same dield in multiple content types.
However, in D8, you cannot share the same field. Each content type must declare its own field with a separate table.
Is this feature then still useful? Who would use the same field twice on one content type? Any other use case?

rudiedirkx’s picture

@johnv Not on the same content type, but on the same entity type (e.g. 'node'), definitely. That's the point of reusing fields. And it's still useful, because 5 content types can have very different cardinality settings for field_images.

solotandem’s picture

@rudiedirkx, you and others (in this issue) have stated something to the effect of "there is a reason for cardinality being a field setting not an instance setting." However, none can give a reason (you and others admit to not remembering what it is). I believe a clear case was made in #1029298-57: Allow cardinality to be restricted (overriden and reduced) in the field instance settings for why that is untrue. So I challenge you (or anyone else) to produce a reason. Otherwise, please stop stating it as such and confusing others who land on this thread.

If you (or anyone else) chooses to respond, let's keep the discussion civil.

rudiedirkx’s picture

I think he nailed it, (fake-)quoting:

"There [are] valid field storage engine implementations that [may] actually rely on the fixed cardinality."

The powers that be think it's a good enough reason to keep cardinality in the field settings. In D7 and still in D8. There's no discussion to be had. I don't have a reason. There doesn't even have to be a reason. The people that decide how Drupal fields work, have decided. Still can't give you any proof though, sorry. I think it was mentioned in https://www.youtube.com/watch?v=tV3bW21hGeU but I'm not sure.

solotandem’s picture

That is what is known as circular reasoning: because they (the powers that be) said so, therefore it is. The question is whether it is a better design to have the settings on the field or the instance, and why. The reasons for preferring the instance have been clearly stated above. The opposing view is absent any evidence.

yched’s picture

IIRC :

- Historically cardinality is on the field level because it affected the storage up until CCK D6.
In Views D6, queries are optimized differently depending on whether the field is multiple or not (and views are cross-bundles, so they know no "field instance")
- This state was preserved in D7, even though strictly speaking this doesn't affect storage anymore (all fields have their own table and have a delta column, so that we don't modify the schema and shuffle data around on config changes). Off-hand, I don't remember if the Views query building is the same (I think it is)
- In D8 though, cardinality again affects (SQL) storage, for *base* fields (mono-valued base fields are stored in a shared base table, multi-valued base fields are stored in a dedicated per-field table).

I honestly can't see how we can consider changing things in D7 at this point ?
And in D8, treating base fields and configurable fields differently would be fairly complex :-/

solotandem’s picture

When you use the phrase 'configurable field' does that imply cardinality > 1? If so, has not D8 then reverted to CCK in D6? And is not this treating base and configurable fields differently which is inherently more complex than a consistent treatment regardless of cardinality? Does it not basically force special casing between those two worlds? What happens when the use case for a field goes from cardinality of one to more? Does the same magic happen to 'modify the schema and shuffle data around'?

yched’s picture

@solotandem :

- base fields are fields defined in code, present in all bundles (like node nid, vid, title, node type, author...)
- configurable fields are fields defined by config records, creaable/editable by site admins in Field UI

Both kinds of fields share the same APIs :
FieldDefinitionInterface (= D7 "instance", specific to a bundle)
FieldStorageDefinitionInterface (= D7 "field", shared across bundles)
That is the crucial point, since this allows the rest of the ecosystem (widgets, formatters, views, access rules, whatever does stuff with fields) to just not care and work with any field indifferently.

Both kinds of fields can be single or multiple-valued, and this lives at the level of the FieldStorageDefinitionInterface : getCardinality() / setCardinality().

The default SQL storage stores :
- each configurable field in a dedicated table, for the same reasons as in D7 (avoid changing the schema and moving data around when the config of the field is changed in the UI)
- each *multi-valued* base field in a dedicated table, because, well, no other ways when there are several deltas
- all *single-valued" base fields in the same shared table, to keep the storage of base properties (nid, title, etc) more or less the same as the one in previous Drupal versions (one table per base field would be quite bad)

So cardinality matters for the storage of base fields, meaning it cannot be on the "instance" level, and is on the FieldStorageDefinition level (it defines storage...).
And base fields and configurable fields share the same interfaces, so it's the same for configurable fields.

This is why things are how they are currently.

solotandem’s picture

@yched, thank you for the explanation on fields in D8.

donquixote’s picture

Ok, so I think enough has been said here about cardinality on storage level.

This still leaves the question if we can make it easier to restrict cardinality on the UI level, per instance.
E.g. a field that is unlimited on storage level, could be limited to 5 items max for one instance, and 10 items max for another instance.

I think it is already possible to write a widget that has a limit built-in. But there is no generic way to do this, or is it?

donquixote’s picture

Title: Move cardinality setting from the field to the instance definition » Allow cardinality to be restricted (overriden and reduced) in the field instance settings

The original issue title is the only one that ever made sense.

I also don't get the updated issue summary. The original one seemed just fine.
What is now the introduction should rather be somewhere at the end, under "Possible solutions".

donquixote’s picture

Btw, one possible solution that could live 100% in contrib:
(talking about D7 for now)

A new widget type with FIELD_BEHAVIOR_CUSTOM, available for all field types, which has the following configuration parameters in its widget settings form:

  • A cardinality limit (that will only have effect if it is smaller than the field-level granularity).
  • The name of another ("decorated") field widget type that can handle this field type.
  • All the settings for the decorated field widget type.

This is the decorator pattern, without any objects involved.. So we will call our new widget the "decorator", and the other one the "decorated".

The decorator widget, in its hook_field_widget_form() implementation, will call the decorated widget's hook_field_widget_form(), either once for each item, or for all items at once, depending on whether the decorated widget is using FIELD_BEHAVIOR_CUSTOM or FIELD_BEHAVIOR_DEFAULT.

If the decorated widget is FIELD_BEHAVIOR_DEFAULT, the decorator widget will behave similar to field_multiple_value_form(), just that for $max, it will take the minimum of the instance limit and field granularity.

If the decorated widget is FIELD_BEHAVIOR_CUSTOM, the decorator widget can change the $field['cardinality'] to give the decorated widget a fake cardinality.

donquixote’s picture

Or, another solution in contrib (D7).

  1. Add a number field in the instance settings form (for all widget types), to allow setting a cardinality limit.
  2. Use hook_field_widget_properties_alter() to
    • replace $instance['widget']['module'], and save the original value in $instance['widget']['_module'].
    • override $instance['widget']['type'] with our own widget type with FIELD_BEHAVIOR_CUSTOM, and save the original value in $instance['widget']['_type'].

    Note: The hook allows to alter $instance['widget'], just not the $instance itself.

  3. In our own hook_field_widget_form(), we do the decorator trick as above in #79.

The advantage compared to #79 is that the widget choice is not hidden behind a decorator during site building.

donquixote’s picture

This could also allow granularity by user role, or other criteria.

------

Question about the issue summary:

Some settings though are in field instance settings and can not be altered right now. Examples:
* maximum number of values (note: don't get this wrong, this is not about field schema cardinality)

Where does the field instance have a max number of values?

rudiedirkx’s picture

As I said in #59, contrib can do this very nicely, IF that patch gets through. If it gets in, any contrib or custom can do what it wants to "Allow cardinality to be restricted (overriden and reduced) in the field instance settings", as is now the perfect issue summary. Really need the patch though. Can't go copying all widgets to make them all limitable.

I'm not sure why the issue status is Needs work, but I still think #41 is perfect. It's simple and it might actually get in. D8 is another story completely.

donquixote’s picture

Well, #79 and #80 would work *without* any patch to core.
(and without "copying all widgets")

I'm not sure why the issue status is Needs work, but I still think #41 is perfect. It's simple and it might actually get in.

Passing $context['instance'] and $context['field'] by reference might have side effects.

Firstly, obviously, if an existing module already does mess with $context['instance'], this currently has no effect. But it would have an effect with this patch. So it would change the behavior of existing modules.

Secondly, by-reference generally tends to increase fragility.. also it has unexpected effects on array copy performance.
I'm not even saying any of this applies here, I just like to avoid them if not required :)

rudiedirkx’s picture

I'd love to see #79 and #80 work =) You doing anything?

I haven't noticed any adverse side effects yet, although I don't use all 15000 modules, so there might be. Since everything in D8 is an object, I'd say that makes it even more 'fragile'.

Another option (with patch) is adding yet another drupal_alter()... Won't break anything, but would require a core patch.

I'm looking forward to #79 and #80 sandbox modules!

donquixote’s picture

I'd love to see #79 and #80 work =) You doing anything?

Not sure yet.
There is a possible use case in a project I'm currently working on, but now they said "Let's just set limit=10 for all those fields.".
I will see :)

kenorb’s picture

I've tried to replace items_count and cardinality in form alter but it didn't help:

/**
 * Implements hook_form_alter().
 */
function foo_form_alter(&$form, &$form_state, $form_id) {
  if (strpos($form_id, 'entityform_edit_form') !== FALSE && isset($form['#entity'])) {

    // Cardinality test.
    $collection = $form['field_foo'][LANGUAGE_NONE];
    $field_name = $collection['#field_name'];
    $langcode = $collection['#language'];
    $parents = $collection['#field_parents'];
    $field_state = &field_form_get_state($parents, $field_name, $langcode, $form_state);
    $field_state['items_count'] = 5;
    $field_state['field']['cardinality'] = 5;
  }
}

They did work in field_state, but field_multiple_value_form is ignoring these settings.

Tested patch from #41, but it didn't work.

kenorb’s picture

kenorb’s picture

Some workaround using Rules and form_alter:

/**
 * Implements hook_action_info().
 */
function mymodule_entityform_rules_rules_action_info() {
  return array(
    'foo_entityform_rules_set_cardinality_value' => array(
      'label' => t('Change items count of a multi value field'),
      'group' => t('Rules Forms'),
      'parameter' => array(
        'form' => array('type' => 'form', 'label' => t('Form')),
        'form_state' => array('type' => 'form_state', 'label' => t('Form State')),
        'element' => array('type' => 'form_element', 'label' => t('Target Element')),
        'value' => array('type' => 'form_element', 'label' => t('Value from Element')),
      ),
      'base' => 'foo_entityform_rules_set_cardinality_value',
      'callbacks' => array(
        'access' => 'rules_forms_integration_access',
        'form_alter' => 'rules_forms_element_form_alter',
      ),
    ),
  );
}

/**
 * Implementation of rule callback.
 */
function mymodule_entityform_rules_set_cardinality_value($form, $form_state, $element, $value) {
  module_load_include("module", "rules_forms", 'rules_forms');
  $val_elem = &_rules_forms_get_element($form, $value);
  if (!isset($form_state['field'])) {
    return;
  }

  $element = substr($element, strpos($element, ':') + 1);
  $path = explode(':', $element);
  if (count($path) == 1) {
    $path[] = LANGUAGE_NONE;
  }

  $field_state = &drupal_array_get_nested_value($form_state['field'], $path);
  if (!$field_state) {
    return;
  }

  $limit = !empty($val_elem['#value']) ? $val_elem['#value'] : (!empty($val_elem['#default_value']) ? $val_elem['#default_value'] : NULL);
  $field_state['items_count'] = $limit;
  $field_state['field']['cardinality'] = $limit; // Won't work for field_multiple_value_form().
}

/**
 * Implements hook_form_alter().
 */
function mymodule_form_alter(&$form, &$form_state, $form_id) {

    // Check cardinality.
    // If items_count for a field has reach its cardinality limit,
    // then remove Add more and Remove buttons.
    foreach ($form as $key => $item) {
      if (is_array($item) && !empty($item['#type']) && !empty($item['#language']) && $item['#type'] == 'container') {
        $field = &$form[$key];
        $langcode = $field['#language'] ?: LANGUAGE_NONE;
        $field_state = field_form_get_state($field[$langcode]['#field_parents'], $field[$langcode]['#field_name'], $langcode, $form_state);
        if ($field_state['items_count'] == $field_state['field']['cardinality']) {
          $field[$langcode]['add_more']['#access'] = FALSE;
          foreach (array_column($field[$langcode], 'remove_button') as $key2 => $rm_item) {
            $field[$langcode][$key2]['remove_button']['#access'] = FALSE;
          }
        }
      }
    } // end: foreach
  }
}

For more details, see: How to change field cardinality per form either programatically or by rule?

davidwbarratt’s picture

Version: 7.x-dev » 8.2.x-dev
Status: Needs work » Active

Moving this feature request to Drupal 8.2.x. If we want to backport the feature later we can.

I think from reading this thread, from an instance, you should be able to restrict a field to anything less than what the storage of a field dictates. So if you select a cardinality of 1 when you create a field, you can't limit it to be less than one, if you select 5, you can limit it to be 3 or 1. If you select "Unlimited" you can limit it to be anything you want. Technically you could change the storage from 5-6 and that would change your instance limits, but I don't think you can move up from 1 if it's on the same database table.

Also, the same restrictions should happen on the REST API for the bundle as well.

David_Rothstein’s picture

Issue tags: +Needs backport to D7

There were some comments above like #73 and #75 suggesting this might not be possible in Drupal 8, although I tend to agree it should be possible as long as it's implemented as a UI restriction only (and as long as you can only reduce the cardinality).

So leaving at Drupal 8 now, but marking for backport.

davidwbarratt’s picture

I'm working on a module to do this, but it looks like #1592814: Add hook alter to override field_multiple_value_form() is preventing this from happening, I'm looking through the workaround now..

rafenden’s picture

I created a module that alters cardinality by rendering the form with overridden settings: https://www.drupal.org/project/multivalue_extras

ttournie’s picture

Hello,

here is the patch for Drupal 7.44

Version: 8.2.x-dev » 8.3.x-dev

Drupal 8.2.0-beta1 was released on August 3, 2016, which means new developments and disruptive changes should now be targeted against the 8.3.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.3.x-dev » 8.4.x-dev

Drupal 8.3.0-alpha1 will be released the week of January 30, 2017, which means new developments and disruptive changes should now be targeted against the 8.4.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.4.x-dev » 8.5.x-dev

Drupal 8.4.0-alpha1 will be released the week of July 31, 2017, which means new developments and disruptive changes should now be targeted against the 8.5.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

eloivaque’s picture

On drupal 8 Field Config cardinality in case anyone is interested.

Version: 8.5.x-dev » 8.6.x-dev

Drupal 8.5.0-alpha1 will be released the week of January 17, 2018, which means new developments and disruptive changes should now be targeted against the 8.6.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Wim Leers’s picture

  1. https://www.drupal.org/project/field_config_cardinality is a nicely packaged attempt to solve this issue in contrib. It gets at least one thing right: the use of Config API's third_party_settings API. Well done! It gets one major thing wrong though, which many previous comments (including #90) get wrong: this must NOT be implemented at the level of forms/field widgets.
    (And actually, because it uses template_preprocess_field_multiple_value_form(), it only works for some field type widgets!)
  2. https://www.drupal.org/project/multivalue_extras and https://cgit.drupalcode.org/field_instance_cardinality are D7-only, so I didn't look at them.
  3. The problem with implementing this at the form level (using form alters or a field type widget alter) is that it won't apply to entities being accessed via Entity API or HTTP APIs (core's REST API, https://www.drupal.org/project/jsonapi, https://www.drupal.org/project/graphql, or others). This means different behavior depending on how one interacts with entities. Which in turn means inconsistent data handling and likely data loss!
  4. The one way that requires no Entity/Field API modifications and works for HTTP APIs is: validation constraints. It'd roughly look like this:
    1. Add a constrained_cardinality key to third_party_settings, whose value should be smaller than or equal to the storage's cardinality.
    2. For each field ("instance" in D7) that has such a setting, call \Drupal\Core\TypedData\DataDefinitionInterface::addConstraint().
    3. Model this call after \Drupal\Core\Field\FieldItemList::getConstraints().

    AFAICT that should work. This will work everywhere: in core REST, "regular forms", JSON API, and so on.

  5. This approach does not require any core changes at all. As @yched explained in #73 and #75, there are very good reasons for the current architecture. But what #73/#75 didn't quite explain is why it's a bad idea to reduce field (FieldConfig in D8/"instance" in D7) cardinality for configurable fields, despite the DB table for the field even being created before asking the user what the desired cardinality is.
    That reason is: the entire module ecosystem would need to be updated to support this. Contrib modules won't know to call getCardinality() on the FieldDefinitionInterface rather than the underlying FieldStorageDefinitionInterface. In other words: making this change in core would be a massive BC break.
    So those of you who want this, go and create a field_constrained_cardinality ("constrained" rather than "reduced" because it's being constrained through a constraint — get it? 😜) contrib module that achieves the exact same thing one step up the abstraction ladder: in the land of Entity/Field API validation!

Version: 8.6.x-dev » 8.7.x-dev

Drupal 8.6.0-alpha1 will be released the week of July 16, 2018, which means new developments and disruptive changes should now be targeted against the 8.7.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.7.x-dev » 8.8.x-dev

Drupal 8.7.0-alpha1 will be released the week of March 11, 2019, which means new developments and disruptive changes should now be targeted against the 8.8.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.8.x-dev » 8.9.x-dev

Drupal 8.8.0-alpha1 will be released the week of October 14th, 2019, which means new developments and disruptive changes should now be targeted against the 8.9.x-dev branch. (Any changes to 8.9.x will also be committed to 9.0.x in preparation for Drupal 9’s release, but some changes like significant feature additions will be deferred to 9.1.x.). For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 8.9.x-dev » 9.1.x-dev

Drupal 8.9.0-beta1 was released on March 20, 2020. 8.9.x is the final, long-term support (LTS) minor release of Drupal 8, which means new developments and disruptive changes should now be targeted against the 9.1.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 9.1.x-dev » 9.2.x-dev

Drupal 9.1.0-alpha1 will be released the week of October 19, 2020, which means new developments and disruptive changes should now be targeted for the 9.2.x-dev branch. For more information see the Drupal 9 minor version schedule and the Allowed changes during the Drupal 9 release cycle.

devkinetic’s picture

I'd love to make some traction on this in the contrib space. I've taken a maintainer role on the field_config_cardinality module and I will be creating an issue for this, and most likely cut a 2.x for this overhaul.

@wim-leers From what I understand of what you said, the way we store in 3rd party settings is good, but we must change up the setting and then re-implement the form stuff as a constraint? I've made exactly one of those before, so we'll see how it goes :)

Version: 9.2.x-dev » 9.3.x-dev

Drupal 9.2.0-alpha1 will be released the week of May 3, 2021, which means new developments and disruptive changes should now be targeted for the 9.3.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.3.x-dev » 9.4.x-dev

Drupal 9.3.0-rc1 was released on November 26, 2021, which means new developments and disruptive changes should now be targeted for the 9.4.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.4.x-dev » 9.5.x-dev

Drupal 9.4.0-alpha1 was released on May 6, 2022, which means new developments and disruptive changes should now be targeted for the 9.5.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.5.x-dev » 10.1.x-dev

Drupal 9.5.0-beta2 and Drupal 10.0.0-beta2 were released on September 29, 2022, which means new developments and disruptive changes should now be targeted for the 10.1.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 10.1.x-dev » 11.x-dev

Drupal core is moving towards using a “main” branch. As an interim step, a new 11.x branch has been opened, as Drupal.org infrastructure cannot currently fully support a branch named main. New developments and disruptive changes should now be targeted for the 11.x branch, which currently accepts only minor-version allowed changes. For more information, see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

devkinetic’s picture

Just as an update, field config cardinality is going strong and will be addressing the constraint update soon, so if your looking into this feature, give it a spin.

plach’s picture

It seems the use cases described in the OP could also be addressed by #1234624: Limit the number of fields to display.

amateescu’s picture

Issue tags: -Needs backport to D7

I've been thinking about this issue in the past couple of days, and I think there is a way to implement this cleanly thanks to the OO code that we didn't have in D7.

Here's my plan for how we could do this in D10+:

  • deprecate FieldStorageDefinitionInterface::isMultiple() and FieldStorageDefinitionInterface::getCardinality()
  • introduce FieldStorageDefinitionInterface::isStorageMultiple() and FieldStorageDefinitionInterface::getStorageCardinality() to replace them
  • introduce FieldDefinitionInterface::isMultiple() and FieldDefinitionInterface::getCardinality() that would default to the new storage methods
  • the new methods above also solve the problem of BaseFieldDefinition, which implements both FieldStorageDefinitionInterface and FieldDefinitionInterface, so the storage-level cardinality would still have to be specified, but each bundle-level field definition could use a different cardinality (equal to or lower than the storage-level one)
  • write a detailed change record to explain the difference between the storage-level and field-level cardinality, and when each of them should be used (hint: the field-level cardinality is the one that should be used in most cases)

This approach achieves the following goals:

  • before the existing methods are removed from core, contrib and custom module authors will get a heads-up that they have to change something
  • after the methods are removed from core (e.g. in 11.0), they will need to take this change into consideration and update their code accordingly

@Wim Leers, re #99, note that core already defines a Count constraint based on the field storage cardinality in \Drupal\Core\Field\FieldItemList::getConstraints(), and it would just need to be updated to use the field-level cardinality instead.

Edit: note that this issue doesn't conflict with #1234624: Limit the number of fields to display at all. Even with the plan outlined above, I think that a feature like "allow formatters to display only a subset of the field items" is still valid and worthy to have