Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.* set up checkout extra pane (with build mode and set active
* tested a lot
* at some point in time noticed that "terms of service" are missing
* went to admin/commerce/config/checkout/extra
* noticed that my pane is inactive
* after setting active everything went fine
strange, but i had this twice now so i'm quite sure about this
and: noone else has the rights to access the settings
don't know how to reproduce,
still hope this won't happen again,
but documenting here just in case...
| Comment | File | Size | Author |
|---|---|---|---|
| #3 | 0001-fixed-1375132-checkout-extra-panes-become-inactive-o.patch | 2.55 KB | geek-merlin |
Comments
Comment #1
pcambraWithout any way to reproduce this, let's mark as need more info.
Maybe that testing you're doing involves deleting/disabling the node that is assigned as the terms of service one, then the pane is disabled automaticallly.
Comment #2
geek-merlinno change of the node had been done afaict.
yes, until we have further evidence and a way to reproduce this cannot be marked "active".
Comment #3
geek-merlingee, did this emerge as an automated update of olde nodebased code?
in our commerce site the TOS vanished several times... =:-()
(sorry, i'm still in a bit of fright...)
so hunted this down and squashed some critical bugs:
* extra pane is deleted via hook_entity_delete if ANY entity with the same id is deleted (no check of entity type)
> added type check
* extra pane is set inactive via hook_entity_update if ANY entity with the same id is deleted (no check of entity type)
> can see no sense in this at all, completely removed
* no messages
> added message
hotfixed, manually tested and deployed to production.
daring to set high prio and rtbc for this to get in fast.
Comment #4
pcambraI agree this is critical but that's not reason to just drop a bunch of code for the update hook, also RTBC status should be set by others but the patch provider.
This bug is about adding the entity type for the commerce_extra_panes_get_panes function, this is not likely to be hit it but I'm committing a simplified version of the patch that doesn't remove used code and declares properly the function. Also committed a bunch of improvements to the code after doing extensive review of the module (this was needed), so it looks way nicer now :)
Be really careful with your modification as it changes commerce_extra_panes_get_panes function header in a way that probably breaks all the extra panes get process by placing the type as the 1st argument.
Feel free to test the -dev version, marking as fixed.
Thanks for taking time to report back this and provide a fix!
Comment #5
geek-merlincool you committed that one last big commit - you really have done en extensive rewrite of all this and all i have seen looks much better. and about the function signature you are completely right, this should not be changed.
i'll be glad to test it - do you already use it somewhere or is it too adventurous to deploy it?
now setting active for the remaining minor thing: are you sure hook_entity update "checkout extra panes become inactive when node is edited" is a good thing?
ok, now there is a message for it, and it's not dangerous now as it was before, but to me it's just annoying and can see no valid use case for that (maybe a lack of fantasy of mine).
feel free to enlighten me or set "worksasdesigned".
Comment #6
pcambraSimple use case, maybe we need a comment in this line if it's not clear.
http://drupalcode.org/project/commerce_extra_panes.git/blob/refs/heads/7...
When you edit a
nodeentity, if its status is 0, or FALSE or disabled or whatever it is the defined for the entity, the pane should be disabled as I don't think you want to show unpublished stuff in the checkout.Said this, it's true that the last commit I made checks the entity after doing a load and ensures that nothing is displayed unless status is enabled.
http://drupalcode.org/project/commerce_extra_panes.git/blob/refs/heads/7...
If you still see a strong point in removing this code, I'm always open to discussion. Maybe it's too much overload for the system to invoke a hook_entity_update just for this, but definitely not part of this particular bug.