Patch allows adding 'add via UI' = FALSE; on a field level. My use case is OG7 where the module allows adding its fields to enteties, but doesn't want users to add it manually, as the field name (og_group) is important and can't be field_og_group.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

yched’s picture

Hm. Subscribe for now.

Can you expand a little about your use case ?
You want a field type whose fields can only be added programmatically ?

amitaibu’s picture

> You want a field type whose fields can only be added programmatically ?

Yes. OG now lives in fields so to find out if a content is a group type we
return (bool)field_info_instance($obj_type, 'og_group', $bundle_name);

(i.e. the field name is hard-coded)

yched’s picture

Status: Needs review » Needs work

OK, makes sense.

+++ modules/field/field.info.inc
@@ -95,6 +95,8 @@ function _field_info_collate_types($reset = FALSE) {
           $field_info += array(
             'settings' => array(),
             'instance_settings' => array(),
+            // Allow adding fields via UI.
+            'add via ui' => TRUE,
           );

We don't really need a comment here, but rather a new entry in the description of the $field structure at the beginning of field.crud.inc

Also, I'm not fond of the 'add via ui' property name.
a) existing 'several words' properties in the $field struct use _ as a separator
b) I'd rather have a 'negative' property, which would default to FALSE. Would be more inline with our current 'locked' property.

Powered by Dreditor.

yched’s picture

amitaibu’s picture

Status: Needs work » Needs review
FileSize
4.38 KB

Re-rolled patch, and added tests.

btw, regarding (a) - in hook_field_widget_info() the 'field types' isn't with an underscore. IMO it's more readable without underscores - but that's another issue...

yched’s picture

Status: Needs review » Needs work

'field types' in hook_widget_info() is indeed inconsistent regarding _ separator. Hmm, too bad, a bit late to change this. At least it's consistent with hook_field_formatter_info() :-(.

+++ modules/field/field.crud.inc
@@ -52,6 +52,9 @@
+ * - hide_in_ui (integer)
+ *     TRUE if field shouldn't be added via the user interface, and can be added
+ *     only programmatically.

Oops, my very bad :-(. We're talking of a property of the field type, not of a given $field, so this part belongs in the PHPdoc for hook_field_info() in field.api.php. Sorry about that.

Suggested name : 'no_ui'. Views has that too, for display plugins that shouldn't be proposed in the UI.
Suggested text:

- no_ui: (optional) A boolean specifying that users should not be allowed to create fields and instances of this field type through the UI. Such fields can only be created programmatically with field_create_field() and field_create_instance(). Defaults to FALSE.
+++ modules/field_ui/field_ui.test
@@ -266,4 +266,11 @@ class FieldUITestCase extends DrupalWebTestCase {
+    $this->assertNoText('Hidden from UI test field', t('Hidden field was not shown.'));

I'd rather have a more specific test like:

$this->assertFalse($this->xpath('//select[@id="edit--add-new-field-type"]//option[@value="hidden_test_field"]'), t("The 'add new field' select respects field types 'no_ui' property."));

Similarly, we need a test that, after creating a field of such a type (programmatically), it's not available in the 'add existing field' select.

Powered by Dreditor.

amitaibu’s picture

Status: Needs work » Needs review
FileSize
6.33 KB

thanks for the review. re-rolled.

yched’s picture

Status: Needs review » Needs work
+++ modules/field_ui/field_ui.test
@@ -266,4 +266,59 @@ class FieldUITestCase extends DrupalWebTestCase {
+        'widget_type' => 'number',

Minor: 'number' is not a valid widget type for a 'hidden_test_field' field. This should be 'test_field_widget'.

Other than that, RTBC.

Powered by Dreditor.

amitaibu’s picture

Status: Needs work » Needs review
FileSize
6.34 KB

Also fixed typo. If bot likes it I'll RTBC.

webchick’s picture

Is there a reason you can't accomplish this with hook_form_alter()?

amitaibu’s picture

1) If one will decide to have a webchick_field_ui module, I will not able to form_alter() it.
2) I think it's easier for implementing modules to set to no_ui property than to form_alter that form.

moshe weitzman’s picture

Its actually a good idea for us to encourage alternate field_ui modules (analogous to simpleviews). Our field ui UX needs help.

yched’s picture

Status: Needs review » Reviewed & tested by the community

- Patch is good.
- Agreed with the answers to webchick's question

So RTBC.

yched’s picture

Status: Reviewed & tested by the community » Needs work

Sorry, one last detail, I should have noted that when reviewing #7 but I was in between trains - Amitaibu will hate me :-p.

_createFieldProgrammatically() in test is cruft and convoluted non-standard syntax around dead-simple field_create_field() and field_create_instance() calls. Plz remove it and inline the field_create_*() calls directly in testHiddenFields().

amitaibu’s picture

Status: Needs work » Needs review
FileSize
5.68 KB

Don't worry ycehd, no hate involved here ;)

I thought that createFieldProgrammatically() could be reused on future tests, but ok - re-rolled.

yched’s picture

Status: Needs review » Reviewed & tested by the community

In the early field API tests, we had such test helper functions wrapping field_[create|update]_[field|instance]() calls, but we soon realized they brought nothing really interesting at the cost of forcing you to learn a new syntax. We killed them :-).

Anyway. Thanks. Back to RTBC.

amitaibu’s picture

Status: Reviewed & tested by the community » Needs work

I have realized I have another use case, where the 'no_ui' should be on the instance level as-well.
I have a field that uses the 'widget_type' => 'options_onoff', but again OG uses the hard coded field name to check the group type. So the field type isn't hidden but the instance is.

I'll re-roll.

amitaibu’s picture

Status: Needs work » Needs review
FileSize
5.69 KB

meh, disregard comment on #17 -- too early in the morning here ;)

I've re-rolled patch to fix typo and wrong indention in comments.

yched’s picture

+++ modules/field/field.api.php
@@ -130,6 +130,10 @@ function hook_field_extra_fields() {
  *     instance definition. This formatter must be available whenever the field
  *     type is available (i.e. provided by the field type module, or by a module
  *     the field type module depends on).
+ *    - no_ui: (optional) A boolean specifying that users should not be allowed
+ *      to create fields and instances of this field type through the UI.
+ *      Such fields can only be created programmatically with
+ *      field_create_field() and field_create_instance(). Defaults to FALSE.
  */

Indentation for 'no_ui' seems off with the previous paragraph ?

While we're at it, the patch changes / adds two occurrences of "don't / doesn't". Core uses no abbreviations. There are probably others in field / field_ui, but we can fix those.

Powered by Dreditor.

amitaibu’s picture

fixed comments from #19

yched’s picture

Status: Needs review » Reviewed & tested by the community

Thx. Back to RTBC.

amitaibu’s picture

#20: 680416-add-via-ui-field-20.patch queued for re-testing.

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 680416-add-via-ui-field-20.patch, failed testing.

amitaibu’s picture

Status: Needs work » Needs review
FileSize
5.75 KB

Re-rolled.

Status: Needs review » Needs work

The last submitted patch, 680416-add-via-ui-field-24.patch, failed testing.

amitaibu’s picture

I have run those test on my local, and image test returns OK. is it pfir issue?

amitaibu’s picture

Status: Needs work » Needs review

#24: 680416-add-via-ui-field-24.patch queued for re-testing.

yched’s picture

Status: Needs review » Reviewed & tested by the community

Bot hiccup, I guess

amitaibu’s picture

#24: 680416-add-via-ui-field-24.patch queued for re-testing.

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 680416-add-via-ui-field-24.patch, failed testing.

yched’s picture

$field['object_types'] now needs to be $field['entity_types'].
$instance['object_type'] needs to be $instance['entity_type'].
See #707724-37: Call them entities instead of objects

amitaibu’s picture

Status: Needs work » Needs review
FileSize
5.77 KB

Let's see this one, with the renamed entity_type.

yched’s picture

Status: Needs review » Reviewed & tested by the community
FileSize
7.16 KB

Wow, I thought this has gone in long ago :-).

Fixed minor comment formatting issues, and enhanced / cleaned the test a bit.
RTBC.

amitaibu’s picture

#33: 680416-add-via-ui-field-33.patch queued for re-testing.

amitaibu’s picture

Re-testing doesn't seem to be working here? This issue is mostly chasing HEAD now ;)

EDIT: My mistake -- test ran on 28/4

amitaibu’s picture

> Wow, I thought this has gone in long ago :-).

hmm, bump ;)

Dries’s picture

Patch needs a quick reroll as it no longer applies. I'll ask for a re-test.

Dries’s picture

#33: 680416-add-via-ui-field-33.patch queued for re-testing.

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 680416-add-via-ui-field-33.patch, failed testing.

amitaibu’s picture

Status: Needs work » Needs review
FileSize
6.48 KB

Re-rolled.

yched’s picture

Status: Needs review » Reviewed & tested by the community

Green - back to RTBC.

Dries’s picture

Status: Reviewed & tested by the community » Fixed

Thanks for the reroll. Committed to CVS HEAD.

Status: Fixed » Closed (fixed)

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

amitaibu’s picture

Status: Closed (fixed) » Needs review
FileSize
814 bytes

Re-opening as it doesn't work - hidden fields appear in the admin page. Patch fixes it. What I still need to figure out is how the tests worked.

amitaibu’s picture

@yched, is it on your radar? :)

yched’s picture

"@yched, is it on your radar? :)"

Not really ;-) - I guess I was waiting for news about "figure out is how the tests worked" ?

amitaibu’s picture

FileSize
2.97 KB

As far as I can tell, the #id of the select input has changed, thus the tests were incorrect.

Status: Needs review » Needs work

The last submitted patch, 680416-no-ui-fix-47.patch, failed testing.

amitaibu’s picture

Status: Needs work » Needs review
FileSize
2.18 KB

Can't run test on my slow laptop, so let's give this one a try. Patch fixes tests to check the correct #id.

yched’s picture

Title: Allow defining fields that can not be added via UI » [Fix tests] Allow defining fields that can not be added via UI
Status: Needs review » Reviewed & tested by the community

Ah of course :-)
Hint : if tests take too long to run on your machine, you can comment out the test classes you're not working on - or rename them to 'class aaatestFooBar' to take them out.

Summary for core committers : the tests that were committed earlier in #42 pass because they test the absence of a DOM element that is never there - thus, always true.

Dries’s picture

Status: Reviewed & tested by the community » Fixed

Committed to CVS HEAD. Thanks.

yched’s picture

Title: [Fix tests] Allow defining fields that can not be added via UI » Allow defining fields that can not be added via UI

Thks Dries.

Resetting title for posterity

Status: Fixed » Closed (fixed)

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