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

  1. Pull drupal-8.0.0-beta14
  2. Install with the standard profile
  3. Go to /admin/structure/types/manage/page/display
  4. See the secondary tabs
  5. Pull drupal-8.0.0-beta15
  6. Run updates (drush updb or update.php)
  7. Go to /admin/structure/types/manage/page/display
  8. 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.

Comments

DuaelFr created an issue. See original summary.

duaelfr’s picture

duaelfr’s picture

Status: Active » Needs review
StatusFileSize
new526 bytes

Here is the fix for the actual update function.

lauriii’s picture

duaelfr’s picture

Issue summary: View changes
StatusFileSize
new968 bytes

The same fix but in a new hook_update_N.

I have no idea about how to write upgrade path tests :/

duaelfr’s picture

Issue summary: View changes

Added steps to reproduce.

Status: Needs review » Needs work

The last submitted patch, 5: system_update_8008-fix_secondary_tabs-2569529-5.patch, failed testing.

The last submitted patch, 3: system_update_8005-fix_secondary_tabs-2569529-3.patch, failed testing.

The last submitted patch, 5: system_update_8008-fix_secondary_tabs-2569529-5.patch, failed testing.

lauriii’s picture

Priority: Normal » Major

Bumping to major

rainbowarray’s picture

Status: Needs work » Needs review
StatusFileSize
new1.05 KB
new619 bytes

This should fix the block update itself. I'll write the tests next.

duaelfr’s picture

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

rainbowarray’s picture

Issue tags: -Needs upgrade path tests +blocks, +Seven
StatusFileSize
new5.64 KB
new4.58 KB

Here'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!

catch’s picture

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

catch’s picture

Issue tags: +beta target

The last submitted patch, 11: 2569529-11-fix-seven-secondary-tabs.patch, failed testing.

Status: Needs review » Needs work

The last submitted patch, 13: 2569529-12-fix-seven-secondary-tasks.patch, failed testing.

rainbowarray’s picture

Status: Needs work » Needs review
StatusFileSize
new5.92 KB
new847 bytes

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

catch’s picture

OK that's a good point.

duaelfr’s picture

Manually retested and... it's still OK ;)
Thank you again!

duaelfr’s picture

Status: Needs review » Reviewed & tested by the community

:)

catch’s picture

Status: Reviewed & tested by the community » Fixed

Committed/pushed to 8.0.x, thanks!

duaelfr’s picture

I can't see the commit on the repository.
Did you forgot to push? :)

  • catch committed c018664 on 8.0.x
    Issue #2569529 by mdrummond, DuaelFr: system_update_8005 does not create...

The last submitted patch, 11: 2569529-11-fix-seven-secondary-tabs.patch, failed testing.

The last submitted patch, 13: 2569529-12-fix-seven-secondary-tasks.patch, failed testing.

Status: Fixed » Needs work

The last submitted patch, 18: 2569529-18-fix-seven-secondary-local-tasks.patch, failed testing.

jibran’s picture

Status: Needs work » Fixed

Status: Fixed » Closed (fixed)

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