Problem/Motivation

The migration UI re-uses the installation database form to gather db credentials from the user. This works great apart from the fact that we do:

    $db_prefix = ($profile == 'standard') ? 'drupal_' : $profile . '_';
    $form['advanced_options']['prefix'] = [
      '#type' => 'textfield',
      '#title' => t('Table name prefix'),
      '#default_value' => empty($database['prefix']) ? '' : $database['prefix'],
      '#size' => 45,
      '#description' => t('If more than one application will be sharing this database, a unique table name prefix – such as %prefix – will prevent collisions.', ['%prefix' => $db_prefix]),
      '#weight' => 10,
    ];

The description makes no sense in the migrate case (it does during installation).

This was found in #3086374: Make Drupal 8 & 9 compatible with PHP 7.4 due to the global $install_state not being set and new errors being emitted from PHP. Running Drupal\Tests\migrate_drupal_ui\Functional\d6\MultilingualReviewPageTest showed the error.

Proposed resolution

Remove the description.
Before

After

Original proposal
We should change the description for this form field in \Drupal\migrate_drupal_ui\Form\CredentialForm::buildForm() and perhaps rework the logic in \Drupal\Core\Database\Install\Tasks::getFormOptions so we don't set a nonsense one for migrate in the first place.

Remaining tasks

User interface changes

New UI text on migrate credentials form.

API changes

None

Data model changes

None

Release notes snippet

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

alexpott created an issue. See original summary.

alexpott’s picture

amjad1233’s picture

Assigned: Unassigned » amjad1233

@alexpott Do you have anything in mind for the description of the field?

alexpott’s picture

Well it needs to tell the user to set it to the prefix used in the drupal site you are migrating from.

Version: 8.9.x-dev » 9.1.x-dev

Drupal 8.9.0-beta1 was released on March 20, 2020. 8.9.x is the final, long-term support (LTS) minor release of Drupal 8, which means new developments and disruptive changes should now be targeted against the 9.1.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

quietone’s picture

Issue summary: View changes
Status: Active » Needs review
FileSize
868 bytes
27.3 KB
32.24 KB

I'm not sure I agree with alexpott and the original proposed resolution because the title for the section is 'Provide credentials for the database of the Drupal site you want to upgrade.' and I don't see the need to repeat that. With that in mind, here is a patch that removes the description.

mikelutz’s picture

Assigned: amjad1233 » Unassigned
Status: Needs review » Reviewed & tested by the community

I agree with @quietone, we don't need a description for the field, the form description describes what to do. I don't think we need a test for this 'bug report' as we are just making some UI text changes.

larowlan’s picture

Status: Reviewed & tested by the community » Needs review
Issue tags: +Bug Smash Initiative
+++ b/core/modules/migrate_drupal_ui/src/Form/CredentialForm.php
@@ -134,6 +134,8 @@ public function buildForm(array $form, FormStateInterface $form_state) {
+      unset($form["database"]["settings"][$key]["advanced_options"]["prefix"]['#description']);

super uber nit pick - any reason we're mixing single and double quotes here? single will suffice in my book

mikelutz’s picture

Status: Needs review » Needs work

nw for #8

jungle’s picture

Status: Needs work » Reviewed & tested by the community
FileSize
868 bytes
886 bytes

Replaced all double quotes with single ones where possible.

mikelutz’s picture

Yes, +1 for back to RTBC

  • larowlan committed 7aca346 on 9.1.x
    Issue #3110839 by quietone, jungle: Use of \Drupal\Core\Database\Install...
larowlan’s picture

Title: Use of \Drupal\Core\Database\Install\Tasks::getFormOptions() in \Drupal\migrate_drupal_ui\Form\CredentialForm::buildForm() results in confusing description for prefix form element » [backport] Use of \Drupal\Core\Database\Install\Tasks::getFormOptions() in \Drupal\migrate_drupal_ui\Form\CredentialForm::buildForm() results in confusing description for prefix form element
Version: 9.1.x-dev » 9.0.x-dev
larowlan’s picture

Committed 7aca346 and pushed to 9.1.x. Thanks!

  • larowlan committed 67c0d6b on 8.9.x
    Issue #3110839 by quietone, jungle: Use of \Drupal\Core\Database\Install...

  • larowlan committed 769509b on 9.0.x
    Issue #3110839 by quietone, jungle: Use of \Drupal\Core\Database\Install...
larowlan’s picture

Title: [backport] Use of \Drupal\Core\Database\Install\Tasks::getFormOptions() in \Drupal\migrate_drupal_ui\Form\CredentialForm::buildForm() results in confusing description for prefix form element » Use of \Drupal\Core\Database\Install\Tasks::getFormOptions() in \Drupal\migrate_drupal_ui\Form\CredentialForm::buildForm() results in confusing description for prefix form element
Version: 9.0.x-dev » 8.9.x-dev
Status: Reviewed & tested by the community » Fixed

Backported to 9.0.x and 8.9.x as this is a bug and there is little risk of disruption

Status: Fixed » Closed (fixed)

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