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.Hi guys,
I just wanted to report a small/minor issue related with the install file of the module.
Upon uninstall, it seems module is not deleting the block_node_type database records related with menu_block.
Basically, we would need to add in menu_block_uninstall (menu_block.install line 10), another call to db_delete('block_node_type').
As a side note, I would assume perhaps a reason why this wouldn't have already been implemented would be because menu_block_uninstall would be a direct port from the 6.x-2.x branch, and since the block_node_type was only added in drupal-7.x, it didn't exist for the 6.x version.
Additionally, I would like to suggest replacing all the calls to variable_del, with a single db_delete with a 'menu_block_% wildcard for the field in the condition.
I guess this change could also be applied to the 6.x-2.x branch, see menu_block.install.
I would greatly appreciate to have your feedback on this task, and if you could let me know if I overlooked or missed anything in module's implementation, or the Drupal API in general.
Feel free to let me know if you would have any questions, comments or concerns on any aspects of the discussed issues, I would be glad to explain in more details.
Thanks very much to all, in advance, for your comments, feedback, and reporting.
Cheers!
Comments
Comment #1
DYdave CreditAttribution: DYdave commentedQuick follow up on this ticket.
Please find attached to this comment two patch files against existing supported branches:
menu_block-7.x-2.x-uninstall-delete-block_node_type-records-1959312-1.patch
menu_block-6.x-2.x-uninstall-delete-block_node_type-records-1959312-1.patch
These two patches have been tested and seem to work as expected.
For both versions, the calls to variable_del have been replaced with a single call to db_delete (7.x-2.x) or db_query (6.x-2.x).
For the 7.x-2.x version, an additional db_delete is required to remove menu_block data from the
block_node_typedatabase table.This change doesn't add any new feature or function to the module, however, it improves its maintainability, readability or code organization.
Please let me know if you would have any questions, objections, comments, suggestions, recommendations or concerns on the patch files or any aspects discussed in this ticket, I would be glad to provide more information, explain in more details or re-roll the patches if necessary.
I would greatly appreciate some help from module maintainers and if any of you could take a bit of time to look into any of the attached patches (6.x-2.x or 7.x-2.x) to give me your feedback/opinion on this ticket.
Any questions, feedback, testing, changes, recommendations would be highly appreciated.
Thanks to all in advance.
Comment #2
Dave ReidChanging all the variable_del() calls warrants more discussion than the block_node_type deletion. Can we split these into two different issues, because I would be more than ok committing the block_node_type table deletions.
Comment #4
Dave ReidResolved in 7.x-2.x now.
Comment #5
Dave Reid