Provide menu links for Drupal Upgrade UI pages

I'm working on this issue:

I have installed the patch and there are problems to be resolved

I have changed the menu link to a menu group for migration as a link collection page under admin/structure/migrate/ where other contrib modules can add their links as well.

Sorry, I uploaded the wrong patch!

+++ b/core/modules/migrate_drupal_ui/migrate_drupal_ui.routing.yml
@@ -1,3 +1,13 @@
+migrate_drupal_ui.migration:
+  path: '/admin/structure/migrate'
+  defaults:
+    _controller: '\Drupal\system\Controller\SystemController::systemAdminMenuBlockPage'
+    _title: 'Migration'
+  requirements:
+    _custom_access: '\Drupal\migrate_drupal_ui\MigrateAccessCheck::checkAccess'
+  options:
+    _admin_route: TRUE

Is it possible to define this base route in the migrate module or somewhere else instead of in migrate_drupal_ui? If we define it in Migrate Drupal UI, what happens if you install a contrib module like https://www.drupal.org/project/migrate_tools which will have a UI for running migrations, but not require Migrate Drupal UI (which is aimed at Drupal to Drupal migrations only)?

Not clear what is the next steps on this issue.

I'm picking this up today at DrupalCon Dublin.

Will be updating issue summary and re-rolling the patch.

Remaining tasks:
- Move /admin/structure/migrate route to the Migrate module
- Adjust permissions on main menu link

Upon further review, given that migrate_drupal is only run once and can only be run by the root user, we may not want menu links for migrate_drupal_ui after all. Especially since the /admin/structure/migrate route is already being claimed currently by the Migrate Tools contrib module.

Let's gather UX issues in one META.

There is another issue which talks about this, see
https://www.drupal.org/node/2679931#comment-10935637, for a suggested item in /admin/config

Both yoroy and Bojhan also thought that putting this in reports made sense. So, I thought I'd try it. Attached is a patch that does that and adds an item for /upgrade/log.

Having the upgrade link in reports seems odd, but fits with the fact that 'available updates' is there. I don't mind where it is, I'll leave that for others to decide.

Assigning for review.

+  parent: system.admin_config_development

+  path: '/admin/config/development/upgrade'

I think that 'admin/reports' might be a good place to place these menu items that are only accessible by user '1', but the patch still puts them under 'admin/config/development'

+  description: 'View upgrade log.'

I would say 'View the upgrade log.'

Going to attempt to work on the changes mentioned during SprintWeekend2017

@ifrik I do agree it was mistake the patch I am working on it. I found this issue by checking the tag SprintWeekend2017. FYI she is going work for the same but she is not assigned for herself from last the 6 hours of the comment. So did worked on it.

@ifrik Ok, Will do that. Kindly consider to guide them assign themselves before starting working on an issue. That is one of best practice.

The menu items are STILL under admin/config/development. But before or along with fixing this, I think the remaining task in the issue summary should be updated: Decide where the menu items should be located.

We still need to reach agreement on where the menu items should be.

Personally, I'm leaning to /admin/reports because updates is also there.

@lokapujya please see the attached image. After apply [#34], clear the cache and see.

@quietone I am also agreeing with /admin/reports

Let's go with /admin/reports/upgrade?

And /upgrade

/admin/reports/upgrade

+1 for this path when reporting the results of the upgrade.

Will respond to #39 and #40.

The migrate meeting is in favor of these two paths:

/upgrade
/admin/reports/upgrade

This patch puts both of them in the menu at Reports->Site Upgrade->Upgrade and Reports->Site Upgrade->Upgrade log, respectively.

I don't think we want to change migrate_drupal_ui.migration into reports. Just migrate_drupal_ui.log. I recommend migrate_drupal_ui.migration and migrate_drupal_ui.upgrade should be /upgrade & /upgrade/initialize, respectfully.

And I wonder if the parent for migrate_drupal_ui.log should be the admin reports route.

Discussed with heddn and mikeryan on IRC. It was pointed out that install.php and update.php do no have menu items but there seems a need for one for /upgrade. And mikeryan suggested a status report page, which I'll make a new issue for.

Finally, /upgrade and /upgrade/initialize are in the 'Content' menu item.

This should fix the errors. But the behavior of admin->content isn't what I expect. However, before addressing that, do we all agree on 'Site upgrade' being in the 'Content' menu?

@yoroy - Any thoughts on where the upgrade page should be linked in the admin interface?

Thanks.

How I think about it is that Migrate should be friends with Configuration Synchronisation because they're both about getting things in/out of Drupal. That would mean the Development section on /admin/config

I'd expect the migration logs to be under /admin/reports. For the logs it's probably more important to clearly link to them from within the Migrate UI screens. Or is the log such an integral part of a migration scenario that we should not move them to somewhere else?

I'm also thinking about the Development section because although this is about content, it's likely a task done not by content editors but site builders, developers. So more thinking about the role of the person performing the task.

Like this maybe?

Let's do it. Moving back to NW.

I have transferred the migrate menu items to the development section, but I think this needs a little more work. Namely, what should the title (and description) be? In the code it is "Site Upgrade", in @yoroy's screenshot it is "Drupal upgrade", but I think the word "upgrade" is misleading. I realise we are taking the user to /upgrade, but should it not be something more like "Migrate content"? Discuss...

but I think the word "upgrade" is misleading. I realise we are taking the user to /upgrade, but should it not be something more like "Migrate content"? Discuss...

I think "Migrate content" would be misleading. This UI is strictly for a complete upgrade of D6 or D7 to D8 - we shouldn't give the impression that it is useful for the general case of migration.

Having discussed with @mikeryan in IRC we're going with "Drupal upgrade" from @yoroy's screenshot.

Here's a couple of screenshots of how it now looks.

Looks good to me.

Embedding the screenshots, to make it easier for reviews.

@yoroy, can you give this your OK (screenshots in the issue summary)?

Thanks.

Thanks. Is "structure" a thing? I know we have content and configuration. Not sure structure is worth calling out explicitly. And why "versions" instead of "sites"?

We place the entity field forms underneath 'Structure'. I've always mentally called it structure. But mhhh. Not a big deal to me.

I'm also OK with explicitly stating "versions". Because it doesn't support upgrades from 8.1, which is an "older" drupal site. We explicitly only support upgrading from an older D6 or D7 site. So maybe we just state the versions and leave it at that?

Upgrade content and configuration from either a Drupal 6 or a Drupal 7 site.

Updated description according to @heddn's suggestion.

I've updated the IS to include the new wording. An updated screenshot would help, but I think its fairly clear without so just moving this along to RTBC.

One final nitpick: Is there a reason for using "Upgrade" as the verb in the description vs. "Import"? Maybe a small distinction, but to me "Upgrade" communicates changing things within the same environment, and "Import" communicates moving stuff from old site A to new site B. Isn't the latter closer to what will actually happen?

Maybe a small distinction, but to me "Upgrade" communicates changing things within the same environment, and "Import" communicates moving stuff from old site A to new site B. Isn't the latter closer to what will actually happen?

To me, the task that someone using this UI is trying to accomplish is to transform their existing Drupal 6 or 7 site into an exactly corresponding Drupal 8 site (i.e., they are upgrading their D6/D7 site), and the fact that this is accomplished by copying into a skeletal D8 installation rather than altering in-place is an implementation detail, not the essence of what is being accomplished.

As with "migrate", my problem with "import" is it implies a more general scope than the very narrowly-focused tool we are actually providing. Contrib is providing migrate/import tools, and the existence of a core UI that looks like a general migrate/import tool would be confusing.

Thanks for explaining. Given that, I agree that "import" is not the better word.

Is there a reason why we have chosen the path /upgrade instead of /admin/config/development/upgrade which would be the convention other admin paths are following?

I also updated the commit credits.

The short path mirrors /update.php, which this UI replaces for major system upgrades. With the menu link, having the short path now seems less important than it was (although I personally find it convenient when debugging the upgrade path;). The feeling in #drupal-migrate seems to be we like /upgrade just fine, but aren't wedded to it - whatever the UX people think.

The reason for /upgrade is to make it analogous to /update.php. It makes it convenient for documentation and communication how to upgrade. However, with a UI menu item, it isn't as critical any more. I cannot say I feel too strongly about keeping it where it is, but I sure like it.

The only strong pro I can think of is that changing it would require a CR, because a lot of folks know that /upgrade is the path.

I suppose we could bikeshed on the URL, but I just want to get this in so if the consensus is to change, I'll go with the consensus.

Yeah my reading was that as an operation this is of similar impact to /install and /update, especially since you want to do this first thing with an empty installation because you lose any prior changes.

So reading this issue, the question from @eojthebrave did not seem to be answered other than @ifrik saying

that's a good point. The main menu item should go into migrate, and and only the menu item for /upgrade should be in the Drupal Upgrade UI module.

All the menu items are in migrate_drupal_ui. And this way it may not make sense to make the UI extensible with the interim page, is there? I am not 100% aware of how other contribs integrate with (and depend on) migrate_drupal_ui. Do they? Sounds like the migration menu item would house more UI modules, which may or may not be limited to Drupal 6/7 => 8 upgrades? Then having the menu item in migrate_drupal_ui is not good and its description is not good. On the other hand, if the description and menu item placement is good, then which modules would put other migration options on the new overview page?

Discussed in the weekly migrate maintainers meeting. We decided to remove the landing page at /upgrade and move /upgrade/initialize into its place. /upgrade will launch you into the upgrade process directly.

Logic: There is no need to make /upgrade extensible; it is for upgrades only. If migrate needs a landing page, it isn't going to use /migrate or something of that ilk.

Moving back to NW, but this should come back to review pretty quick after those changes.

Yay that sounds great.

Updated the IS a little to document the logic on where the links are located. This is now ready as all feedback is now addressed.

Tested this with the intention to commit. The /upgrade page and its link works well, placed well. The out of the line URL seems a bit odd, and we can change it since its still an alpha experimental module. So if there is no reason to keep it like that anymore, we can change it. If we want to keep it to be similar to other basic Drupal paths, that is fine by me as well.

What strikes me odd is that the status report path just redirects to a filtered version of the logs page, so its not really its own page. When you click the URL it ends up at a different URL / breadcrumb, etc. Do we have anything like that in Drupal core and/or expect users to not be puzzled by this? I can see how practical that is to implement, but it looks odd as a user.

Gábor Hojtsy, there is an issue about making a migration status report page, #2853708: Make a status report page for Migrate UI with links to incremental migrations/rollbacks/etc.. Maybe that could be used to address ending up at a different URL / breadcrumb. Perhaps a landing page, which has a link to the filtered report? Just a quick thought.

Random testbot failure.

Random Test Failure. Setting back to RTBC

Passing again, back to RTBC

Thanks all, looks good now, committed :)

Correct final branch and adding string change tag.