Problem/Motivation

This is a followup issue for #1535868: Convert all blocks into plugins.

Currently, Drupal 7 block configurations are not upgraded to the new Block architecture.

Proposed resolution

Per #26

  • Modules that implement blocks are responsible for upgrading their own blocks wrt mapping, name changes, etc...
  • Add _(block_)update_block_to_config
    1. Get/Make config entity using config() for the block name.
    2. Merge in changes like in _update_variables_to_config()?
    3. Get node type and role visibility settings for block id.
    4. Set properties necessary for block entities based on property map passed in?

Manual Testing

  • See screenshots in comment #49
  1. Download Drupal 8 development version
  2. Apply latest patch
  3. Import Drupal 7.22 database from zip file (#49)
    • This is a devel-module generated database, not a test-driven database dump.
  4. Create a settings file to point to the database with update access turned on.
  5. Go to update.php and try the upgrade
  6. Login as user 1 (use Drush 6 to generate a login link) and verify blocks
  7. Login as an authenticated user and verify blocks.
  8. Login as an anonymous user and verify blocks.

Remaining tasks

API changes

  • Adds an update helper function that core and contrib. can use to upgrade blocks.

Updated per comment #76

Files: 

Comments

xjm’s picture

Issue tags: -If SCOTCH fails

Actually, we can start work on this as soon as we finish the ConfigEntity conversion. At most I expect we will need to re-save exported block instances with a few additional plugin IDs, I think.

larowlan’s picture

Given where the code in #1871772: Convert custom blocks to content entities is going, we'll need to write to field_data_field_body|field_revision_field_body too as block.body moves to a field, plus initialize the {custom_block} and {custom_block_revision} tables
We can follow the 6-7 node body upgrade easily enough for the field stuff.

larowlan’s picture

or rather field_data_body|field_revision_body

xjm’s picture

In #1875260: Make the block title required and allow it to be hidden we're looking to make the block title required. For the upgrade path, we'll need to replace titles that are an empty string or <none> with the block description ({block_custom}.info for custom block instances and I suppose whatever the D8 plugin's default title is for other blocks).

Did we keep the same block type IDs for all the core blocks when we converted them?

xjm’s picture

xjm’s picture

Issue tags: +Block plugins
webchick’s picture

How long are we planning on postponing this? AFAICS, all of the architectural issues are done now, yeah?

larowlan’s picture

FWIW the custom_blocks (ie data from {block_custom}) are upgraded, this was fixed in #1871772: Convert custom blocks to content entities so that just leaves their configuration (ie data from the {block} table)

Updating issue summary

larowlan’s picture

Status: Postponed » Active

I'm with @webchick on this
Now that #1871696: Convert block instances to configuration entities to resolve architectural issues is in, this can be active imo

catch’s picture

Yeah even if it changes again there's no reason it can't be fixed now then updated later.

mradcliffe’s picture

I was looking into what the upgrade path for blocks would be and stumbled upon this issue. After looking into code, it looks like the basic upgrade path would be to go through each block table row, create an config instance for the block, and then remove the block table at the end. The hard part of this would be to identify any changes from block.delta to the config entity id (as well as serialized data).

xjm’s picture

@webchick, @catch, I've asked several times for this to be unpostponed, but @EclipseGc and others posit that it will be a huge duplication of effort before displays are done. (I'm not saying I agree it should remain postponed; just that is the response I've gotten when I ask about this issue every week or two.)

mradcliffe’s picture

Status: Active » Needs work
FileSize
3.75 KB
FAILED: [[SimpleTest]]: [MySQL] Setup environment: Test cancelled by admin prior to completion. View

Here's an incomplete patch that borks the site after block upgrade test, but I might as well post my approach. There are two issues I ran into:

  1. Everything requires a plugin and there is no way to find out what a future plugin is going to be. I started out trying to create Block entity instances directly, but quickly found that to be a pain. I got most of the way with just creating Config objects directly, but still ran into the plugin issue. I think it would be possible to lookup core block plugins if the second issue below is solved, but contrib is a different matter. Maybe a dummy plugin for contrib or core modules that aren't consistent with Drupal 7, and then those modules can deal with the problem in a later update..?
  2. Drupal 8 broke consistency with Drupal 7's block names/ids/whatever. An upgrade will need to have a matrix lookup to compare these as there does not seem to be a consistent pattern to upgrade the plugin names/ids/whatever (e.g. Compare the following: menu block, system powered by block, user login block, and language block). Ugh.

I am also having trouble finding docs on appropriate instantiation of configuration entities such as blocks. Maybe I'm missing something obvious..?

swentel’s picture

+++ b/core/modules/block/block.installundefined
@@ -336,6 +336,80 @@ function block_update_8008() {
+      // Remove the block table.
+      db_drop_table('block');

We don't drop tables in the upgrade path - see #1860986: Drop left-over tables from 8.x

Other than that, I had a quick look how to get this forward, and yes, we need some sort of mapping for the core blocks. Same would apply to contrib, so maybe we should create a helper upgrade function where core (and contrib) can pass on a list of old id's mapping to new id's ? A bit like how we handle the variables now as well.

mradcliffe’s picture

Ah, if the block tables still exist until 9.x, then contrib can take care of itself.

For core blocks, a block/plugin map. Then I'd just go back to creating the block entity with createInstance() instead of manually creating a Config object.

mradcliffe’s picture

FileSize
6.21 KB
FAILED: [[SimpleTest]]: [MySQL] Setup environment: Test cancelled by admin prior to completion. View

I tried switching back to entity_create(), but still ran into controller class issues so I switched back to config(). I quickly (read: sloppily) made a mapping function, which works mostly, and there are some issues still with system menu, menu, and locale blocks. Currently ignoring poll, profile, locale, language, and blog blocks.

Everything else either fits the consistent pattern or has a mapping, but needs a test (aggregator, etc...).

I modified the block upgrade test to temporarily just look at admin/structure/block and admin/structure/block/list/block_plugin_ui:seven. If anything bad happens it will break there.

Also still need to filter out contrib blocks per my last comment.

I started out making the block mapping function fairly verbose, but cut back. It may just be a good idea to make it a complete map with properties as well as plugin ids.

Needs work patch attached. No interdiff as I forgot to commit locally last time.

podarok’s picture

+++ b/core/modules/system/lib/Drupal/system/Tests/Upgrade/BlockUpgradePathTest.phpundefined
@@ -8,9 +8,10 @@
+ * Loads a standard installation of Drupal 7 and runs the upgrade process on ¶

trailing whitespace http://drupal.org/coding-standards#indenting

mradcliffe’s picture

FileSize
4.36 KB
6.28 KB
FAILED: [[SimpleTest]]: [MySQL] Setup environment: Test cancelled by admin prior to completion. View

I had some changes that I haven't posted yet and I am not sure when I'm going to get back to this issue so I'll post a new patch up.

I redid the mapping function so that it will be able to map other block properties instead of just id. Still trying to get menu and system menu blocks to upgrade properly. I think I need to stop relying on the block upgrade test so I can actually inspect config files that are being written and compare those to what they should be.

mradcliffe’s picture

FileSize
5.71 KB
7.97 KB
FAILED: [[SimpleTest]]: [MySQL] Setup environment: Test cancelled by admin prior to completion. View

Okay, all core blocks except locale, profile, poll, and blog have configuration entities "written" with visibility settings although it is messy code.

However I still end up with corrupt block configuration for block instances that are disabled due to label settings. It's almost as if the configuration entity writes differently when it's created manually rather than via the block plugin container (which does not seem to be possible in the upgrade path).

Any ideas from anyone else?

xjm’s picture

However I still end up with corrupt block configuration for block instances that are disabled due to label settings. It's almost as if the configuration entity writes differently when it's created manually rather than via the block plugin container (which does not seem to be possible in the upgrade path).

That's probably exactly the case. Somehow the entity plugin manager is available for Views though (I think?) Maybe it becomes available later?

There's a lot of criticals in this problem space right now:
#1949724: Allow simpletest child sites to additionally load a test-specific settings.php to allow testing anonymous and configless updates
#1848998: Problems with update_module_enable()
#1856972: config() isn't safe to use during minor updates if there's a change to configuration storage

alexpott’s picture

Well one thing is for sure - we can't use entity_* functions in updates... mad stuff happens!

This patch needs to update the block manifest file with the new entities created too.

alexpott’s picture

Also should the block module be upgrading blocks provided by other modules?

xjm’s picture

Also should the block module be upgrading blocks provided by other modules?

This is an interesting question. Since the data was originally in {block}, I think it should?

alexpott’s picture

This is an interesting question. Since the data was originally in {block}, I think it should?

But the implementation of hook_block_info is the modules... eg. http://api.drupal.org/api/drupal/modules%21system%21system.module/functi...

mradcliffe’s picture

Thank you for the direction, xjm and alexpott.

mradcliffe’s picture

Okay, new proposal after thinking about this on the plane.

  • Modules that implement blocks are responsible for upgrading their own blocks wrt mapping, name changes, etc...
  • Add (_block_)update_block_to_config
    1. Get/Make config entity using config() for the block name.
    2. Merge in changes like in _update_variables_to_config()?
    3. Get node type and role visibility settings for block id.
    4. Set properties necessary for block entities based on property map passed in?
jaredsmith’s picture

Issue summary should be updated to specify that it should no longer be postponed

jaredsmith’s picture

Issue summary: View changes

Updated issue summary.

pameeela’s picture

Updated issue summary based on the latest approach proposed in #26. This will need more updates around reviews, steps to test, API changes - but should be OK for now?

@mradcliffe are you happy to continue working on this, in which case I will stop hounding other people to do it? :)

mradcliffe’s picture

@pameeela, I'm planning on at least working on it before Friday, and then when I am not working as a mentor on Friday in Portland.

Edit: I don't have any additional code at the moment.

mradcliffe’s picture

Status: Needs work » Needs review
FileSize
5.17 KB
FAILED: [[SimpleTest]]: [MySQL] 55,894 pass(es), 0 fail(s), and 2 exception(s). View

Okay, I made some progress on an alternate way of doing this. I added a function to handle creating config entities in update.inc, and then each module will need to implement this. I added user and menu module as a test and to see what's going to fail.

I still need to do some more actual testing, but wanted to post a WIP patch for some review on this new approach.

Questions:

  • Should the (_block_)update_block_to_config() function be in update.inc, block.module, or block.install?

Status: Needs review » Needs work

The last submitted patch, 1871700-block-upgrade-take-two-1.patch, failed testing.

mradcliffe’s picture

Status: Needs work » Needs review
FileSize
4.12 KB
6.54 KB
FAILED: [[SimpleTest]]: [MySQL] 55,886 pass(es), 1 fail(s), and 0 exception(s). View

Okay, here's an added test and fixing the label issue. I think this is going to work.

Need to hammer out the rest of core.

Status: Needs review » Needs work

The last submitted patch, 1871700-block-upgrade-take-two-2.patch, failed testing.

pameeela’s picture

@mradcliffe looks like you are getting close. Can you or anyone else across this issue post some steps to test for when we get to green?

mradcliffe’s picture

FileSize
12.68 KB
18.77 KB
FAILED: [[SimpleTest]]: [MySQL] Invalid PHP syntax in core/modules/node/node.install. View

@pameela. I think if we want full test coverage, then the upgrade database needs to be filled with content for all blocks, and then tests each block for specific content/configuration. I think the most important assert would be if the block is not corrupt (i.e. the code created config entities incorrectly via config()).

I have added book, comment, forum, node, search, system, and shortcut block upgrade paths in this next patch. I haven't had a chance to look at the one failure yet.

Note: the interdiff has two commits in it (didn't post patch #3).

Edit

  1. Node and comment had variables that were not upgraded related to blocks. I added comment variable in the block upgrade and node in a previous upgrade. Not sure which is the best approach.
geerlingguy’s picture

Status: Needs work » Needs review

Status: Needs review » Needs work
Issue tags: -D8 upgrade path, -Configuration system, -VDC, -Blocks-Layouts, -Block plugins

The last submitted patch, 1871700-block-upgrade-take-two-4.patch, failed testing.

The last submitted patch, 1871700-block-upgrade-take-two-4.patch, failed testing.

mradcliffe’s picture

FileSize
4.88 KB
18.74 KB
FAILED: [[SimpleTest]]: [MySQL] Repository checkout: failed to checkout from [git://git.drupal.org/project/drupal.git]. View

Oops. That patch was terrible. this won't solve the 1 fail, but that's next.

mradcliffe’s picture

Status: Needs work » Needs review

Sure let's go for a test.

mradcliffe’s picture

Okay, this patch will fix some bad plugin ids that I had in the previous patches in the new approach. Again, I haven't looked at the one failing test yet to do with block title length. I am not sure what changed that would cause this.

Todo:

  • Debug failing test.
  • Come up with more tests of blocks (if necessary - @eclipsegc?).

Also screenshots added:

mradcliffe’s picture

Status: Needs review » Needs work

I am not saving the label correctly, which should account for the test failure.

oadaeh’s picture

Title: Blocks are not upgraded » Provide an upgrade path to the new Block architecture from Drupal 7

I was confused by the title and thought that it meant that blocks had not yet been upgraded in D8. Hopefully the new title still says what is intended while removing the confusion.

mradcliffe’s picture

Status: Needs work » Needs review
FileSize
2.01 KB
18.57 KB
FAILED: [[SimpleTest]]: [MySQL] 55,972 pass(es), 0 fail(s), and 14 exception(s). View

Ignore comment #42. The test fail comes from switching to standard_all instead of minimal. The test assumes that sidebar region will be available on admin pages, but that's not true for the standard_all database.

- Switched back to minimal database for now.
- Improved docs a bit (not complete) including fixing a typo in node.install.

Status: Needs review » Needs work

The last submitted patch, 1871700-block-upgrade-take-two-7.patch, failed testing.

mradcliffe’s picture

Status: Needs work » Needs review
FileSize
3.64 KB
1005 bytes
1.7 KB
18.92 KB
FAILED: [[SimpleTest]]: [MySQL] 55,875 pass(es), 0 fail(s), and 16 exception(s). View

This patch is mainly cleanup with a fix for the exceptions in the last test run and incrementing the node update number.

Status: Needs review » Needs work

The last submitted patch, 1871700-block-upgrade-take-two-8.patch, failed testing.

mradcliffe’s picture

Status: Needs work » Needs review
FileSize
856 bytes
6.83 KB
20.45 KB
PASSED: [[SimpleTest]]: [MySQL] 56,423 pass(es). View

Oh, heh, block_navigation != book_navigation. I also fixed not using array union correctly.

Let's give this patch a whirl while the queue is low.

mradcliffe’s picture

Issue summary: View changes

Updated issue summary

mradcliffe’s picture

Issue tags: +Needs issue summary update, +Needs manual testing
FileSize
527.55 KB
337.79 KB
3.12 KB
23.18 KB
PASSED: [[SimpleTest]]: [MySQL] 55,095 pass(es). View

New patch:

  • Added aggregator category and feed block upgrade path.

Manual testing.

  1. Download drupal 8 dev
  2. Apply patch
  3. Import Drupal 7.22 database from zip file (It's SQL w/ devel generated content, not a test-driven dump).
  4. Create a settings file to point to the database with update access turned on.
  5. Upgrade
  6. Go through anonymous, authenticated, and admin users (use drush 6 to generate a login link), and verify blocks. I added some screenshots to work with.
mradcliffe’s picture

Status: Needs review » Needs work
  • timplunkett: Try reworking the node/comment settings instead of adding them as permament settings in node/comment.settings.yml
  • mradcliffe: Upgrade/delete variables that are used in blocks in the node/comment block update, and then create follow-up issues for forum/book as that's what already exists now in D8
mradcliffe’s picture

Issue summary: View changes

Removing myself from the author field so I can unfollow. --xjm

Gaelan’s picture

Issue summary: View changes

Updated issue summary.

mradcliffe’s picture

Status: Needs work » Needs review
Issue tags: -Needs issue summary update
FileSize
4.2 KB
22.2 KB
PASSED: [[SimpleTest]]: [MySQL] 58,300 pass(es). View

Re-roll and remove the node/comment settings additions per #50.

mradcliffe’s picture

Issue summary: View changes

Whoops. I failed. This should be better.

mradcliffe’s picture

Issue summary: View changes

Remove completed task.

mradcliffe’s picture

FileSize
1.67 KB
22.24 KB
FAILED: [[SimpleTest]]: [MySQL] 57,466 pass(es), 4 fail(s), and 0 exception(s). View

Re-roll, patch no longer applies and changed update numbers for node, system, and user.

Gábor Hojtsy’s picture

Issue tags: +upgrade path
mradcliffe’s picture

Status: Needs review » Needs work
Issue tags: +upgrade path, +Needs manual testing, +D8 upgrade path, +Configuration system, +VDC, +Blocks-Layouts, +Block plugins

The last submitted patch, 1871700-block-upgrade-take-two-12.patch, failed testing.

mradcliffe’s picture

Status: Needs work » Needs review
FileSize
1 KB
22.23 KB
FAILED: [[SimpleTest]]: [MySQL] 57,727 pass(es), 1 fail(s), and 0 exception(s). View

Ah, that's not my fault. There's a regression in core and that default local task no longer works. If the previous patch was in core already than tests would have caught the regression. :*(

Here's a patch that uses the working menu route for the block form controller.

Filed related issue #2052019: Fix block configuration default local task to use block_admin_edit route.

Status: Needs review » Needs work

The last submitted patch, 1871700-block-upgrade-take-two-13.patch, failed testing.

mradcliffe’s picture

Status: Needs work » Needs review
FileSize
22.23 KB
PASSED: [[SimpleTest]]: [MySQL] 58,244 pass(es). View

That test is not failing locally for me so rebase and re-roll.

jibran’s picture

Awesome work @mradcliffe. I have found some minor doc issues.

+++ b/core/includes/update.incundefined
@@ -1680,3 +1680,99 @@ function update_language_list($flags = Language::STATE_CONFIGURABLE) {
+ * @param $block_name
...
+ * @param $module
...
+ * @param $delta
...
+ * @param $properties
...
+function update_block_to_config($block_name, $module, $delta, $properties) {

Typehint missing.

+++ b/core/modules/aggregator/aggregator.installundefined
@@ -330,3 +330,77 @@ function aggregator_update_8001() {
+  $results = db_select('block')->condition('module', 'aggregator')->fields('block')->execute()->fetchAll();

+++ b/core/modules/book/book.installundefined
@@ -79,3 +79,43 @@ function book_update_8000() {
+  $results = db_select('block')->condition('module', 'book')->fields('block')->execute()->fetchAll();

+++ b/core/modules/forum/forum.installundefined
@@ -270,3 +270,51 @@ function forum_update_8000() {
+  $results = db_select('block')->condition('module', 'forum')->fields('block')->execute()->fetchAll();

+++ b/core/modules/menu/menu.installundefined
@@ -89,3 +98,41 @@ function menu_update_8004() {
+  $results = db_select('block')->condition('module', 'menu')->fields('block')->execute()->fetchAll();

+++ b/core/modules/system/system.installundefined
@@ -2250,6 +2250,78 @@ function system_update_8059() {
+  $results = db_select('block')->condition('module', 'system')->fields('block')->execute()->fetchAll();

+++ b/core/modules/user/user.installundefined
@@ -1090,5 +1090,43 @@ function user_update_8020() {
+  $results = db_select('block')->condition('module', 'user')->fields('block')->execute()->fetchAll();

Can we make these multi lines?

mradcliffe’s picture

FileSize
22.4 KB
PASSED: [[SimpleTest]]: [MySQL] 57,345 pass(es). View
5.02 KB

Thank you for the review, jibran.

benjy’s picture

Status: Needs review » Needs work

I tested this using the database dump provided and the latest codebase and I get the following error when hitting update.php. Can anyone comment on what I might have done wrong, or replicate the same error?

WD menu: Drupal\Core\Database\DatabaseExceptionWrapper: SQLSTATE[42S02]: Base table [error]
or view not found: 1146 Table 'd8.router' doesn't exist: SELECT name, route FROM
{router} WHERE name IN (:names_0); Array
(
    [:names_0] => block_admin_display
)
 in Drupal\Core\Database\Connection->query() (line 571 of
/Users/bendougherty/Sites/d8/core/lib/Drupal/Core/Database/Connection.php).
WD php: Drupal\Core\Database\DatabaseExceptionWrapper: SQLSTATE[42S22]: Column not  [error]
found: 1054 Unknown column 'base.uuid' in 'field list': SELECT base.uid AS uid,
base.uuid AS uuid, base.name AS name, base.langcode AS langcode, base.pass AS pass,
base.mail AS mail, base.theme AS theme, base.signature AS signature,
base.signature_format AS signature_format, base.created AS created, base.access AS
access, base.login AS login, base.status AS status, base.timezone AS timezone,
base.preferred_langcode AS preferred_langcode, base.preferred_admin_langcode AS
preferred_admin_langcode, base.init AS init
FROM
{users} base
WHERE  (base.uid IN  (:db_condition_placeholder_0)) ; Array
(
    [:db_condition_placeholder_0] => 0
)
 in Drupal\Core\Database\Connection->query() (line 571 of
/Users/bendougherty/Sites/d8/core/lib/Drupal/Core/Database/Connection.php).
mradcliffe’s picture

Hi benjy,

Do you have a drupal 7 settings.php? You need to make that writable and place it in sites/default. I think most likely Drupal "detected" that 8 was installed and tried to bootstrap Drupal 8. You can see this if you find a Drupal 8 config_* directory or Drupal 8 tables created in the database (ugh). I found that I needed to go directly to core/update.php instead of being redirected from /update.php to core/update.php.

Unrelated to the above...

I was able to run the update with and without the patch on the database dump from a while ago, but I was left with a broken / bugged installation. Next step is to re-do the manual database dump.

  • PHP Fatal error: Call to a member function buildUrl() on a non-object in core/modules/rdf.module on line 341
  • ... and similar errors in image.module
mradcliffe’s picture

Issue summary: View changes

Added manual testing steps to issue summary.

benjy’s picture

OK I can confirm I got this patch to work my main problem been that I was using a D8 settings file I think.

It was a little flakey though, like afterwards I was getting quite a lot of random errors around the place. I'm going to try upgrade a an out the box D7 site populated with devel content and a live site sometime this week and i'll post back with some more steps and results.

webchick’s picture

Re-adding eaten tags.

Thanks SO much for working on this, folks!!

benjy’s picture

anyone know what causes tags to sometimes get destroyed? It's a bit annoying!

webchick’s picture

I blame gremlins.

klonos’s picture

:D

mradcliffe’s picture

The database dump provided is blocked on #2049465: Upgrade of image styles and effects broken.

benjy’s picture

FileSize
447 bytes
22.4 KB
PASSED: [[SimpleTest]]: [MySQL] 58,305 pass(es). View

OK I did some more testing and the results are below. I've also attached a new patch which increments the hook_update() number.

  • Install D8
  • Import attached D7 db.
  • Copy a D7 settings file into your D8 installation. http://paste2.org/4Lp1gWL5
  • Make sure you have $settings['update_free_access'] = TRUE; (This is from the D8 config file and differs from D7)
  • Run the update

Troubleshooting

  • Make sure you delete everything in sites/default/files/* if you've been using the same D8 codebase previously.
  • Try going straight to core/update.php if you don't get redirected.

Issues
1. My custom blocks were no longer "Placed" in their regions.

2. After the update I ran drush uli and got this error when submitting the user/edit page with my a new password.

InvalidArgumentException: Unable to set a value with a non-numeric delta in a list. in Drupal\Core\Entity\Field\Field->setValue() (line 105 of /Users/bendougherty/Sites/d8/core/lib/Drupal/Core/Entity/Field/Field.php)
benjy’s picture

Status: Needs work » Needs review

Update for the bot

benjy’s picture

FileSize
111.26 KB

DB dump attached, mentioned in #69

webchick’s picture

Issue tags: +beta blocker

Tagging all critical D8 upgrade path issues as "beta blocker."

Wim Leers’s picture

1. My custom blocks were no longer "Placed" in their regions.

AFAICT this is because there is no test coverage for the upgrade path of custom blocks from D7 to D8. Neither #1871772: Convert custom blocks to content entities nor #1919910: Refactor custom_block upgrade path to create tables using hook_schema_0() instead of in block_update_N() added that.

jibran’s picture

Is it NW or NR because of #73?

catch’s picture

This is going to need a re-roll due to #1888702: Use configuration selection instead of derivatives for some blocks which I'm about to commit.

Berdir’s picture

To clarify, that issue removes derivates for aggregator feeds and categories and instead puts the item count into the block instance configuration, where it already kinda is.

I haven't check what this issue is doing, but it simply needs to create a block instance for each (enabled, not sure if you care about others here?) aggregator block and put the feed/category id into the configuration. Then drop the block column.

Berdir’s picture

Issue summary: View changes

Remove use of deprecated function.

mradcliffe’s picture

Status: Needs review » Needs work

Yes, this is going to need work. Also adjusted status per remaining tasks section.

mradcliffe’s picture

Status: Needs work » Needs review
FileSize
3.3 KB
1.08 KB
745 bytes
22.71 KB
FAILED: [[SimpleTest]]: [MySQL] 57,757 pass(es), 0 fail(s), and 35 exception(s). View
  1. interdiff-1.txt: Took care of the config -> \Drupal::config change
  2. interdiff-2.txt: Fixed update numbers for user and node.
  3. interdiff-3.txt: Made an attempt at a fix for aggregator based on recent changes.

Hopefully this gets things up to speed for any Prague core sprinters.

Status: Needs review » Needs work

The last submitted patch, 1871700-block-upgrade-take-two-78.patch, failed testing.

benjy’s picture

Status: Needs work » Closed (won't fix)

This issue is no longer relevant now we're brining migrate into core. I'll post back with the new issue when it's created.

There is a group for the migrate efforts here: https://groups.drupal.org/imp

mradcliffe’s picture

I wanted to mention it for those at Badcamp tackling this issue, but the patch can still come in handy so you don't have to duplicate a lot of work.

David_Rothstein’s picture

Status: Closed (won't fix) » Needs work

Yeah, I don't think an issue should be closed when the code in it will still need to be added in some form or another.

Maybe at most this could be marked "postponed"?

mradcliffe’s picture

It depends on if the Migrate work should be split into a separate issue or not. I could not find any information on an existing tag for the Imp Initiative so I went ahead and added one as "Imp Initiative".

catch’s picture

Status: Needs work » Postponed

I'd commit this patch assuming it works + passes tests so there's a reference point to work from in core, however I don't want to encourage anyone to work on it when it's likely that a good chunk of the code will have to be done again for migrate. Assuming that doesn't happen 'postponed' feels like the right status - a lot of the code can likely be re-used or at worst shows what data needs to go to/from where.

catch’s picture

Issue summary: View changes

Updated issue summary per latest comments.

catch’s picture

Priority: Critical » Major
Issue summary: View changes
Issue tags: -beta blocker

No longer blocks release. But we need a way to track issues that block releasing a migration path at some point.

klonos’s picture

Issue summary: View changes
Parent issue: » #1535868: Convert all blocks into plugins
Related issues: +#1888702: Use configuration selection instead of derivatives for some blocks

...parent/related issues now have their own place as issue metadata. Also hiding past patches and interdiffs.

moshe weitzman’s picture

Status: Postponed » Closed (duplicate)

We have an IMP issue for #2120629: Write a source plugin + tests for 6.x Blocks. Metadata also needs migrating (enabled/disabled, region, etc.). That one is now #2149179: Write a source plugin + tests for 6.x Block Metadata (status, region, visibility, ...) and has Related Issue and Summary links back to this one.

Marking this as a duplicate.