Edit hook_help text for Drupal Upgrade UI module

Suggested text for uses section:

For upgrading from Drupal 6 and 7 to Drupal 8.

The module will present you with a list of available upgrade paths depending on the Drupal 8 modules you have enabled. Once you have verified that you have enabled all of the modules you need in your Drupal 8 site you can run the migration which will import the configuration and then import content into the new structure.

If a module does not have an upgrade path yet and one later becomes available you can re-run the migraiton when that module is present to bring those items into an existing Drupal 8 site.

Should you need control over which migrations run or wish to do only a partial migration you may want to consider an alternative module to run the migrations such as Migrate Tools.

@ekes,thanks. Here are some suggestions. Is the final paragraph needed? Does help usually refer people to contrib?

Upgrading from Drupal 6 or Drupal 7 to Drupal 8.

The Migrate Drupal UI module lists all available upgrade paths which depends on the Drupal 8 modules installed on this website. Once you have verified that all of the modules you need in your website are installed you can run the migration which will import the configuration and content from the source site into this site.

Upgrading incrementally
An upgrade can be re-run to import data changed since the last upgrade.

If a module does not have an upgrade path yet and one later becomes available you can re-run the upgrade when that module is present to bring those items into an existing Drupal 8 site.

Rolling back an upgrade
Selecting the rollback option will remove all the configuration and content imported from the source Drupal 6 or Drupal 7 site.

Needs a title
Should you need control over which migrations run or wish to do only a partial migration you may want to consider an alternative module to run the migrations such as Migrate Tools.

Making some minor code and text changes, followed Help text standards but couldn't find anything major.

This is about adding text to the UI module. Please provide a patch for that module, not migrate.module

Let's gather UX issues in one META.

An attempt to improve the help text.

@ifrik, thx. This looks much better now.

1. +++ b/core/modules/migrate_drupal_ui/migrate_drupal_ui.module
   @@ -13,7 +13,31 @@
   +      $output .= '<h3>' . t('Use') . '</h3>';
   

   Should be 'Uses' ?

2. +++ b/core/modules/migrate_drupal_ui/migrate_drupal_ui.module
   @@ -13,7 +13,31 @@
   +      $output .= '<dt>' . t('Rolling back an upgrade') . '</dt>';
   +      $output .= '<dd>' . t('Rollbacks and incremental migrations are not yet supported through the user interface.') . '</dd>';
   

   I think this should be two sections, one for rolling back and one for incremental.

+++ b/core/modules/migrate_drupal_ui/migrate_drupal_ui.module
@@ -13,7 +13,31 @@
+        array(
...
+        array(

Prefer short array syntax.

Decided to make the patch with the changes I suggested above.

Assigning for review.

Most of the recent modules (the experimental ones) use the short array syntax for their help texts, so I think it's OK to use it here too. Maybe the standard page needs an update?

English is not my first language, so I might be wrong, but I think some improvements can be made in the 'Performing the upgrade' section which sounds a bit clumsy to my ears. I'm gonna keep thinking about this because I don't have a better wording for it yet.

+      $output .= '<dd>' . t('You can review the upgrade log by clicking the link in the message provided after the upgrade or by filtering the messages for the type <em>migrate_drupal_ui</em> on the <a href=":messages">Recent log messages</a> page.',
+          [':messages' => \Drupal::url('dblog.overview')]) . '</dd>';

We could link to the already filtered log page by using the 'migrate_drupal_ui.log' route instead of 'dblog.overview'.
Since we are also thinking about adding links for the 'Upgrade' and 'Upgrade Log' pages in #2701795: Provide menu links for Drupal Upgrade UI pages, we could also mention them here.

+      $output .= '<dt>' . t('Incremental upgrade') . '</dt>';
+      $output .= '<dd>' . t('Incremental upgrades are not yet supported through the user interface.') . '</dd>';

I would use the plural 'Incremental upgrades' in both places.

+      $output .= '<dl>';

This <dl> tag is not closed at the end.

OK, I see, let's use the long syntax then.

Have rewritten the issue. Please review it.

I'm not sure what you were trying to do there prash_98, but the patch you posted only changes a space, and then doesn't keep the content of the previous patch.

Also you don't need to assign the issue to yourself, in fact in this case the opposite, as you want some one else to review.

My 2c worth on the array() or [] syntax while I'm here:
I think the documentation standard wants updating to use [], as that is what is used in the code everywhere else.
Is there an issue for that?

Sorry for assigning that to myself.

I've itemized the issues in #19 so I could keep track of them.

1. Use short array syntax?
2. Improve the working in the 'Performing the upgrade section'.
3. Link to migrate_drupal_ui.log instead of dblog.overview
4. Since we are also thinking about adding links for the 'Upgrade' and 'Upgrade Log' pages in #2701795: Provide menu links for Drupal Upgrade UI pages, we could also mention them here.
5. S/Incremental upgrade/Incremental upgrades/
6. Add missing dl tag at then end.

And here is what has been done to address those.

1. The policy on using short array syntax is now RTBC. So, those changes stay in the patch.#2135291: [Policy, no patch] PHP 5.4 short array syntax coding standards for Drupal 8
2. Todo
3. Done.
4. Prefer to do this after that issue is done, or maybe in that issue.
5. Done
6. Done.

Language seems spot on. I didn't review this from the perspective of the help standards. I believe there are some. But I like where this stands now.

The help text looks fine. No other comments from anyone else. Let's get this in.

This updated documentation looks great; just a few more small things.

1. +++ b/core/modules/migrate_drupal_ui/migrate_drupal_ui.module
   @@ -6,6 +6,7 @@
   +use Drupal\Core\Url;
   
   @@ -13,7 +14,25 @@
   +          [':upgrade' => Url::fromRoute('migrate_drupal_ui.upgrade')->toString()]) . '</dd>';
   ...
   +          [':messages' => Url::fromRoute('migrate_drupal_ui.log')->toString()]) . '</dd>';
   

   I checked and the only other places we do a route toString() in hook_help() (only a handful) are where we actually need other features of Url than just plain URL generation from route. So actually, it is preferable to use \Drupal::url() here for readability. Somewhere there is an issue about which of the 13,000 ways of generating links are intended to be deprecated and not.

2. +++ b/core/modules/migrate_drupal_ui/migrate_drupal_ui.module
   @@ -13,7 +14,25 @@
   +      $output .= '<dd>' . t('You need to enable all modules on this site that are enabled on the previous site. For example, if you have used the book module on the previous site then you must enable the book module on this site for that data to be available on this site.') . '</dd>';
   

   Capitalization for module names should be like: "the Book module".

3. +++ b/core/modules/migrate_drupal_ui/migrate_drupal_ui.module
   @@ -13,7 +14,25 @@
   +      $output .= '<dd>' . t('On the <a href=":upgrade">Upgrade</a> page, you are guided through performing the upgrade in several steps. You need to enter the database credentials of the Drupal site that you want to upgrade. You can also include its files directory in the upgrade. The next page then provides an overview over which upgrade paths are available or missing, before you proceed to perform the upgrade. Afterwards a message is displayed about the number of upgrade tasks that were successful or failed.',
   

   Maybe we should put this in an <ol> for readability?

Thanks everyone!

Ah, from #2491981: There are too many ways to generate URLs and links:

t(…) -> should NOT do the manual ->toString() call, that should be taken care of by t() automatically

All fixed. I did think about using <ol> but I have a tendency to go overboard compartmentalizing stuff, so I didn't.

Now that there are steps, I think the wording may need to be changed. And for step #2, how do we say that at that point the can pause and enable/disable modules or cancel?

The screenshot really helped. Some moderate nits.

• add a link to the log in "reviewing the log".
• The "Afterwords a message is displayed..." Let's add that as list item 3 and a suggestion on re-wording, "Lastly, a message is displayed..."

Everything else looked fine.

Minor Fixes.

Interdiff?

+++ b/core/modules/migrate_drupal_ui/migrate_drupal_ui.module
@@ -27,8 +27,8 @@
+      $output .= '<dd>' . t('Lastly, a message is displayed about the number of upgrade tasks that were successful or failed.') . '</dd>';

This should move up as a bulleted item in the doing the upgrade section.

The link for the logs should point to 'migrate_drupal_ui.log'.

If it isn't too much, can I get one last screenshot of what it looks like? I think this is ready to go.

It's the way you suggested , "Lastly ... " as 3rd point in li and correct log link.

And a screenshot.

Looks good.

Thanks, this looks great! Thanks also for the screenshot.

There's one small new thing that has been introduced since #28 which is a bit off. I was going to fix it on commit, until I realized I didn't have enough information to do so:

+++ b/core/modules/migrate_drupal_ui/migrate_drupal_ui.module
@@ -13,7 +14,28 @@
+      $output .= '<dt>' . t('Reviewing the upgrade <a href=":messages">Log</a>',[':messages' => \Drupal::url('migrate_drupal_ui.log')]) . '</dt>';

The word "log" should not be linked here, as the link does not provide enough information out of context for accessibility. Instead, it should be linked in the following paragraph. The user also needs to understand how this link is different from the "Recent log messages" page (which is a totally different route). What does the migrate_drupal_ui.log route link to?

Also, the word "Log" should not be capitalized here as it's just a noun, not the proper name of something.

Updating the proposed issue credit for reviewers. Thanks everyone!

Fixes for #45.

And a screenshot of the changed lines only.

And back to rtbc.

Alright, that works for me!

This is a UI and string change, but since it is for an alpha experimental module, it's backportable to 8.3.x. Thanks everyone for working on this; it is a really great improvement.

@JohnOku open a follow-up and post a patch?