Problem/Motivation

A batch builder object was added in #2401797: Introduce a batch builder class to make the batch API easier to use. The config module should be updated to use the new builder.

Proposed resolution

Update the the code in core/modules/config/* to use the batch builder. The code to update can most easily be found by looking for the uses of batch_set(). Example usage is in the class comment for \Drupal\Core\Batch\BatchBuilder.

Remaining tasks

  • Update the code

User interface changes

None

API changes

None

Data model changes

None

Comments

John Cook created an issue. See original summary.

Version: 8.4.x-dev » 8.5.x-dev

Drupal 8.4.0-alpha1 will be released the week of July 31, 2017, which means new developments and disruptive changes should now be targeted against the 8.5.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.5.x-dev » 8.6.x-dev

Drupal 8.5.0-alpha1 will be released the week of January 17, 2018, which means new developments and disruptive changes should now be targeted against the 8.6.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

john cook’s picture

Title: Update config module to use the new batch service » Update config module to use the new batch builder
Issue summary: View changes
Issue tags: +Novice

I've updated the summary to make the task clearer.

john cook’s picture

john cook’s picture

Issue summary: View changes
rajeevk’s picture

Status: Active » Needs review
StatusFileSize
new4.71 KB

Attaching patch for config module using new batch builder.

Status: Needs review » Needs work

The last submitted patch, 7: 2875283-7.patch, failed testing. View results

Status: Needs review » Needs work

The last submitted patch, 7: 2875283-7.patch, failed testing. View results

james.shee’s picture

Attempting to work as part of Drupalcon 2018 Nashville mentored sprint

john cook’s picture

  1. +++ b/core/modules/config/src/Form/ConfigSingleImportForm.php
    @@ -400,17 +402,16 @@ public function submitForm(array &$form, FormStateInterface $form_state) {
    +          ->setFinishCallback('Drupal\config\Form\ConfigSingleImportForm::finishBatch')
    ...
    -          $batch['operations'][] = [[ConfigSync::class, 'processBatch'], [$config_importer, $sync_step]];
    

    We should be passing the class and method as the first parameter. This would be along the lines of ->setFinishCallback([ConfigSync::class, 'finishBatch'])

    The classes passed in do not match the currently used classes.

  2. +++ b/core/modules/config/src/Form/ConfigSingleImportForm.php
    @@ -400,17 +402,16 @@ public function submitForm(array &$form, FormStateInterface $form_state) {
    +          ->setTitle(new TranslatableMarkup('Importing configuration'))
    +          ->setInitMessage(new TranslatableMarkup('Starting configuration import.'))
    +          ->setProgressMessage(new TranslatableMarkup('Completed @current step of @total.'))
    +          ->setErrorMessage(new TranslatableMarkup('Configuration import has encountered an error.'));
    

    $this->t() should be used to make strings translatable.

  3. +++ b/core/modules/config/src/Form/ConfigSync.php
    @@ -335,18 +337,17 @@ public function submitForm(array &$form, FormStateInterface $form_state) {
    +          ->setFinishCallback('Drupal\config\Form\ConfigSync::finishBatch')
    ...
    +          $batch_builder->addOperation('Drupal\config\Form\ConfigSync::finishBatch', [$config_importer, $sync_step]);
    

    The class names do not match the currently used classes.

    Use the ['MyClass', 'myCallbackMethod'] format for the first parameter of the setFinishedCallback() and addOperation() methods.

  4. +++ b/core/modules/config/src/Form/ConfigSync.php
    @@ -335,18 +337,17 @@ public function submitForm(array &$form, FormStateInterface $form_state) {
    +          ->setTitle(new TranslatableMarkup('Synchronizing configuration'))
    +          ->setInitMessage(new TranslatableMarkup('Starting configuration synchronization.'))
    +          ->setProgressMessage(new TranslatableMarkup('Completed step @current of @total.'))
    +          ->setErrorMessage(new TranslatableMarkup('Configuration synchronization has encountered an error.'))
    

    Please use $this->t() for translatable text.

james.shee’s picture

StatusFileSize
new4.15 KB

Hi John, thank you for the tips. Adding new patch. This is my first commit

You stated that we should use $this->t(). Is it okay to just use t()?
https://www.drupal.org/node/2875389

james.shee’s picture

StatusFileSize
new4.03 KB

Hopefully this patch works better, did a git diff this time

john cook’s picture

@James.Shee, when in class methods you use $this->t() and in functions (outside of classes) t() is used.

From the StringTranslationTrait documentation:

Using this trait will add t() and formatPlural() methods to the class. These must be used for every translatable string, ...

This helps with code injection (swapping the code of equivalent interface). This is most commonly used for unit tests.

james.shee’s picture

Accidentally removed the foreach loop, and added in $this->t()
That should resolve a few of the errors, will try to figure out what is going on with the rest, just uploading for now.

james.shee’s picture

StatusFileSize
new4.07 KB

Whoops, uploaded the wrong patch

rajeshwari10’s picture

Assigned: Unassigned » rajeshwari10
rajeshwari10’s picture

Assigned: rajeshwari10 » Unassigned
Status: Needs work » Needs review
StatusFileSize
new4.04 KB
new1.7 KB

Please Review the patch.

Thanks!!

borisson_’s picture

Status: Needs review » Needs work

There are some whitespace problems in this patch that should be fixed.

  1. +++ b/core/modules/config/src/Form/ConfigSingleImportForm.php
    @@ -400,19 +401,16 @@ public function submitForm(array &$form, FormStateInterface $form_state) {
    +            ->setTitle($this->t('Importing configuration'))
    +            ->setFinishCallback([ConfigSync::class, 'finishBatch'])
    +            ->setInitMessage($this->t('Starting configuration import.'))
    +            ->setProgressMessage($this->t('Completed @current step of @total.'))
    +            ->setErrorMessage($this->t('Configuration import has encountered an error.'));
    

    Should be 2 spaces instead of 4?

  2. +++ b/core/modules/config/src/Form/ConfigSingleImportForm.php
    @@ -400,19 +401,16 @@ public function submitForm(array &$form, FormStateInterface $form_state) {
    -          $batch['operations'][] = [[ConfigSync::class, 'processBatch'], [$config_importer, $sync_step]];
    +            $batch_builder->addOperation([ConfigSync::class, 'processBatch'], [$config_importer, $sync_step]);
    

    Should stay at 2 spaces.

  3. +++ b/core/modules/config/src/Form/ConfigSync.php
    @@ -335,20 +336,17 @@ public function submitForm(array &$form, FormStateInterface $form_state) {
    +        $batch_builder = (new BatchBuilder())
    +            ->setTitle($this->t('Synchronizing configuration'))
    

    ^

  4. +++ b/core/modules/config/src/Form/ConfigSync.php
    @@ -335,20 +336,17 @@ public function submitForm(array &$form, FormStateInterface $form_state) {
    -          $batch['operations'][] = [[get_class($this), 'processBatch'], [$config_importer, $sync_step]];
    +            $batch_builder->addOperation([get_class($this), 'processBatch'], [$config_importer, $sync_step]);
    

    ^

yogeshmpawar’s picture

Assigned: Unassigned » yogeshmpawar
yogeshmpawar’s picture

Assigned: yogeshmpawar » Unassigned
Status: Needs work » Needs review
StatusFileSize
new4.03 KB
new3.2 KB

Coding standard issues fixed as mentioned in #19& also added an interdiff.

rajeevk’s picture

Status: Needs review » Reviewed & tested by the community

Issues pointed out in #19 is fixed in #21. It looks good now..

alexpott’s picture

Status: Reviewed & tested by the community » Closed (duplicate)

Comment on meta:

Please let's do the sub issues here in one patch rather than multiple patches. They are the same task - see https://www.drupal.org/core/scope

Basically all of these patches should be merged there.