Early Bird Registration for DrupalCon Portland 2024 is open! Register by 23:59 PST on 31 March 2024, to get $100 off your ticket.
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.
Comment | File | Size | Author |
---|---|---|---|
#49 | 680416-no-ui-fix-49.patch | 2.18 KB | amitaibu |
#47 | 680416-no-ui-fix-47.patch | 2.97 KB | amitaibu |
#44 | no-ui-admin.patch | 814 bytes | amitaibu |
#40 | 680416-add-via-ui-field-40.patch | 6.48 KB | amitaibu |
#33 | 680416-add-via-ui-field-33.patch | 7.16 KB | yched |
Comments
Comment #1
yched CreditAttribution: yched commentedHm. Subscribe for now.
Can you expand a little about your use case ?
You want a field type whose fields can only be added programmatically ?
Comment #2
amitaibu> 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)
Comment #3
yched CreditAttribution: yched commentedOK, makes sense.
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.
Comment #4
yched CreditAttribution: yched commentedRelated but not similar: #680910: Allow a given field to be restricted to some entity types
Comment #5
amitaibuRe-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...
Comment #6
yched CreditAttribution: yched commented'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() :-(.
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:
I'd rather have a more specific test like:
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.
Comment #7
amitaibuthanks for the review. re-rolled.
Comment #8
yched CreditAttribution: yched commentedMinor: '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.
Comment #9
amitaibuAlso fixed typo. If bot likes it I'll RTBC.
Comment #10
webchickIs there a reason you can't accomplish this with hook_form_alter()?
Comment #11
amitaibu1) 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.
Comment #12
moshe weitzman CreditAttribution: moshe weitzman commentedIts actually a good idea for us to encourage alternate field_ui modules (analogous to simpleviews). Our field ui UX needs help.
Comment #13
yched CreditAttribution: yched commented- Patch is good.
- Agreed with the answers to webchick's question
So RTBC.
Comment #14
yched CreditAttribution: yched commentedSorry, 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().
Comment #15
amitaibuDon't worry ycehd, no hate involved here ;)
I thought that createFieldProgrammatically() could be reused on future tests, but ok - re-rolled.
Comment #16
yched CreditAttribution: yched commentedIn 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.
Comment #17
amitaibuI 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.
Comment #18
amitaibumeh, disregard comment on #17 -- too early in the morning here ;)
I've re-rolled patch to fix typo and wrong indention in comments.
Comment #19
yched CreditAttribution: yched commentedIndentation 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.
Comment #20
amitaibufixed comments from #19
Comment #21
yched CreditAttribution: yched commentedThx. Back to RTBC.
Comment #22
amitaibu#20: 680416-add-via-ui-field-20.patch queued for re-testing.
Comment #24
amitaibuRe-rolled.
Comment #26
amitaibuI have run those test on my local, and image test returns OK. is it pfir issue?
Comment #27
amitaibu#24: 680416-add-via-ui-field-24.patch queued for re-testing.
Comment #28
yched CreditAttribution: yched commentedBot hiccup, I guess
Comment #29
amitaibu#24: 680416-add-via-ui-field-24.patch queued for re-testing.
Comment #31
yched CreditAttribution: yched commented$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
Comment #32
amitaibuLet's see this one, with the renamed entity_type.
Comment #33
yched CreditAttribution: yched commentedWow, I thought this has gone in long ago :-).
Fixed minor comment formatting issues, and enhanced / cleaned the test a bit.
RTBC.
Comment #34
amitaibu#33: 680416-add-via-ui-field-33.patch queued for re-testing.
Comment #35
amitaibuRe-testing doesn't seem to be working here? This issue is mostly chasing HEAD now ;)
EDIT: My mistake -- test ran on 28/4
Comment #36
amitaibu> Wow, I thought this has gone in long ago :-).
hmm, bump ;)
Comment #37
Dries CreditAttribution: Dries commentedPatch needs a quick reroll as it no longer applies. I'll ask for a re-test.
Comment #38
Dries CreditAttribution: Dries commented#33: 680416-add-via-ui-field-33.patch queued for re-testing.
Comment #40
amitaibuRe-rolled.
Comment #41
yched CreditAttribution: yched commentedGreen - back to RTBC.
Comment #42
Dries CreditAttribution: Dries commentedThanks for the reroll. Committed to CVS HEAD.
Comment #44
amitaibuRe-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.
Comment #45
amitaibu@yched, is it on your radar? :)
Comment #46
yched CreditAttribution: yched commented"@yched, is it on your radar? :)"
Not really ;-) - I guess I was waiting for news about "figure out is how the tests worked" ?
Comment #47
amitaibuAs far as I can tell, the #id of the select input has changed, thus the tests were incorrect.
Comment #49
amitaibuCan't run test on my slow laptop, so let's give this one a try. Patch fixes tests to check the correct #id.
Comment #50
yched CreditAttribution: yched commentedAh 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.
Comment #51
Dries CreditAttribution: Dries commentedCommitted to CVS HEAD. Thanks.
Comment #52
yched CreditAttribution: yched commentedThks Dries.
Resetting title for posterity