Problem/Motivation
As far as I understand revision_scheduler_field_attach_form()
is responsible to add the revision scheduling elements on entity add / edit pages.
Unfortunately hook_field_attach_form()
is used on quite some other occasions.
Adding comments, entering the billing information in the commerce checkout and so on.
This can lead to notices because revision_scheduler_edit_form()
expects to be dealing with a full blown entity form and not with a partial form.
Issues found so far: #2391751-3: Notice: Undefined index: #id in omega_form_alter() (line 360 (Comment form), Commerce Checkout
Proposed resolution
Try to detect if it really is a full blown entity add / edit form and skip the processing otherwise.
So far it seems like such a full form always has the entity that's added / edited in the $form_state
keyed by the entity type.
Attached patch has such a check.
Remaining tasks
Reviews needed.
User interface changes
None.
API changes
None.
Comment | File | Size | Author |
---|---|---|---|
#2 | revision_scheduler-make-revision_scheduler_field_attach_form-less-clingy-2442959-2.patch | 928 bytes | das-peter |
Comments
Comment #1
Dave ReidIs there proof this works reliably for all entity types? I don't see what assigns $form_state[$entity_type] in any code aside from node_form() which makes me think this would only work for nodes. For example, taxonomy_form_term() assigns $form_state['term'], not $form_state['taxonomy_term'].
Comment #2
das-peter CreditAttribution: das-peter commentedI've found this pattern in:
node
user
entity
entity_operations
entity_rules
commerce
commerce_coupon
file_entity
Hints:
wysiwyg has this comment
// $profile is the primary object of this form, and as an entity, usually expected to live in $form_state[$entity_type].
Term seems to be the "usual" issue. It seems to have a naming inconsistency there which also applies e.g. for tokens.
I updated the patch to fix terms.
However, it certainly isn't a bullet proof solution but I think that applies to the current solution as well.
Comment #4
Dave ReidAgreed. I think I'm going to file this as a solution to a couple more modules like metatag and redirect as well. Thanks @das-peter!