Hello,

Looking to update from menu_breadcrumb 8.x-1.9 to 8.x-1.10. I am seeing that the config structure has been changed in #3045132: Use title parent menu instead of home or sitename in breadcrumb.

A hook_update_N should be done for existing websites to init the new config key.

Comments

Grimreaper created an issue. See original summary.

grimreaper’s picture

rphair’s picture

Assigned: Unassigned » rphair

OK, would you agree with the following logic: When changing the config key home_as_site_name (boolean in 1.9) to config key front_title (integer in 1.10):

  • if home_as_site_name was FALSE, set front_title = 1 (Use the site name from the configuration settings)
  • if home_as_site_name was TRUE, set front_title = 0 (Use "Home" as title)

You're right: as the code is now, the default value 0 for front_title when version 1.10 is installed will 'Use "Home" as title' regardless of how home_as_site_name was 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.

grimreaper’s picture

Hello,

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.

rphair’s picture

@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?

grimreaper’s picture

Status: Active » Needs review
StatusFileSize
new1.49 KB

No 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.

  • rphair committed c3fafc6 on 8.x-1.x authored by Grimreaper
    Issue #3077958 by Grimreaper, rphair: Make hook_update_N for new config...
rphair’s picture

Status: Needs review » Fixed

I see what you mean about the testing: I tried to get it to run menu_breadcrumb.install when 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.

grimreaper’s picture

Hello @rphair,

Thanks for the commit. No problem keeping it in dev for some days.

  • rphair committed cea67e8 on 8.x-1.x authored by Grimreaper
    Issue #3077958 by Grimreaper, rphair: Make hook_update_N for new config...

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.