Closed (fixed)
Project:
Menu Breadcrumb
Version:
8.x-1.x-dev
Component:
Code
Priority:
Normal
Category:
Bug report
Assigned:
Reporter:
Created:
29 Aug 2019 at 13:38 UTC
Updated:
17 Sep 2019 at 10:19 UTC
Jump to comment: Most recent, Most recent file
Comments
Comment #2
grimreaperComment #3
rphair commentedOK, would you agree with the following logic: When changing the config key
home_as_site_name(boolean in 1.9) to config keyfront_title(integer in 1.10):home_as_site_namewas FALSE, set front_title = 1 (Use the site name from the configuration settings)home_as_site_namewas TRUE, set front_title = 0 (Use "Home" as title)You're right: as the code is now, the default value 0 for
front_titlewhen version 1.10 is installed will 'Use "Home" as title' regardless of howhome_as_site_namewas set in previous module versions. Let me know if you agree, confirm I haven't missed anything, and I'll write the update hook to push it out in version 1.11 ASAP.Comment #4
grimreaperHello,
Agreed! :)
I had not done it myself, I wanted to let someone at my company to do it to mentor him/her, but if you want to do it quickly, no problem!
Also the hook_update_N should check that the key "front_title" do not exist for websites already upgraded to 1.10 and having resaved the config form or websites done directly on 1.10.
Comment #5
rphair commented@Grimreaper then I would also like to consider this a mentorship opportunity, since I'm pretty sure I could execute the logic for the variable settings but I don't know the Drupal standard way of checking the configuration relative to the version number. This is the first time an update hook has been required for this module & I want to be sure it's done right. So I would like to see the solution you have imagined & then will fast track that patch into a new version... what do you think?
Comment #6
grimreaperNo problem.
I don't think it will be necessary to test the current version of the module. Also I don't know how it would be relevant.
Here is a patch for what I have in mind.
I didn't have tested it.
Thanks for the review.
Comment #8
rphair commentedI see what you mean about the testing: I tried to get it to run
menu_breadcrumb.installwhen switching from the 1.9 module config & code to the 1.10 config & code, but couldn't make it work... i.e. I couldn't set up a test for how the upgrade would affect the config data without uninstalling the module & wiping out that config data.However your code looks complete & accurate to me. Therefore maybe can leave this in dev for a couple of days until some more people have a chance to look at it? I'll push out as 1.11 first thing on Tuesday if no problems & thanks for the support.
Comment #9
grimreaperHello @rphair,
Thanks for the commit. No problem keeping it in dev for some days.