Follow-up to : #507488: Convert page elements (local tasks, actions) into blocks
Problem/Motivation
In #507488: Convert page elements (local tasks, actions) into blocks local tasks and actions were removed from templates and replaced by blocks and an upgrade path has been added to ensure existing sites won't loose these blocks. system_update_8005() put these blocks in all core themes plus all other active themes.
The problem is that that upgrade path creates a block for primary tasks in seven but fails at creating the block for secondary tasks, widely used in the admin area.
Steps to reproduce
- Pull drupal-8.0.0-beta14
- Install with the standard profile
- Go to /admin/structure/types/manage/page/display
- See the secondary tabs
- Pull drupal-8.0.0-beta15
- Run updates (drush updb or update.php)
- Go to /admin/structure/types/manage/page/display
- No more secondary tabs :'(
Proposed resolution
Fix that upgrade path or create a system_update_N() function that fix it.
Remaining tasks
- Find out where to put the fix (existing update function or new one?)
- Write the patch
User interface changes
None.
API changes
None.
Data model changes
None.
| Comment | File | Size | Author |
|---|---|---|---|
| #18 | interdiff-2569529-12-18.txt | 847 bytes | rainbowarray |
| #18 | 2569529-18-fix-seven-secondary-local-tasks.patch | 5.92 KB | rainbowarray |
Comments
Comment #2
duaelfrComment #3
duaelfrHere is the fix for the actual update function.
Comment #4
lauriiiComment #5
duaelfrThe same fix but in a new hook_update_N.
I have no idea about how to write upgrade path tests :/
Comment #6
duaelfrAdded steps to reproduce.
Comment #10
lauriiiBumping to major
Comment #11
rainbowarrayThis should fix the block update itself. I'll write the tests next.
Comment #12
duaelfrManually tested the #11 patch and it works.
Thank you fixing that. Now I look at my patch in #5 I'm just ashamed about how it's poorly made.
Comment #13
rainbowarrayHere's the upgrade path tests for this issue.
There's only a two-line difference between your patch and in #5 and mine in #11. You did great! Kudos: thanks for finding this and working to fix it!
Comment #14
catchWhy not do this directly in system_update_8005()?
What happens if that's run already and someone has manually fixed their Seven block layout or customized it?
Comment #15
catchComment #18
rainbowarrayThis should fix the test fails.
The update hook checks config to see if there is another block with this name already and only adds the block if that's not the case. So we should be okay there.
I fixed the 8005 update. However, that went in prior to beta 15, so for those who have already had that run, I think it's probably worthwhile to have this update as well. This is something somebody might not realize is missing.
Comment #19
catchOK that's a good point.
Comment #20
duaelfrManually retested and... it's still OK ;)
Thank you again!
Comment #21
duaelfr:)
Comment #22
catchCommitted/pushed to 8.0.x, thanks!
Comment #23
duaelfrI can't see the commit on the repository.
Did you forgot to push? :)
Comment #28
jibran