Part of #1971384: [META] Convert page callbacks to controllers

For instructions on how to convert a page callback into a controller, see the WSCCI Conversion Guide.

Files: 
CommentFileSizeAuthor
#28 1987660-config-sync-28.patch15.54 KBygerasimov
PASSED: [[SimpleTest]]: [MySQL] 57,521 pass(es). View
#28 1987660-config-sync-28-interdiff.txt4.02 KBygerasimov
#26 1987660-config-sync-26.patch15.02 KBygerasimov
PASSED: [[SimpleTest]]: [MySQL] 57,124 pass(es). View
#26 1987660-config-sync-26-interdiff.txt6.36 KBygerasimov
#23 1987660-config-sync-23.patch14.8 KBygerasimov
PASSED: [[SimpleTest]]: [MySQL] 57,213 pass(es). View
#23 1987660-config-sync-23-interdiff.txt7.29 KBygerasimov
#19 1987660-config-sync-19.patch13.55 KBvijaycs85
FAILED: [[SimpleTest]]: [MySQL] 58,141 pass(es), 13 fail(s), and 5 exception(s). View
#19 1987660-diff-17-19.txt534 bytesvijaycs85
#17 1987660-config-sync-17.patch13.03 KBvijaycs85
FAILED: [[SimpleTest]]: [MySQL] Drupal installation failed. View
#15 config-sync-1987660-15.patch14 KBdougvann
FAILED: [[SimpleTest]]: [MySQL] 56,194 pass(es), 11 fail(s), and 4 exception(s). View
#13 config-sync-1987660-13.patch14 KBdougvann
FAILED: [[SimpleTest]]: [MySQL] 55,831 pass(es), 11 fail(s), and 4 exception(s). View
#11 config-sync-1987660-11.patch7.38 KBdougvann
FAILED: [[SimpleTest]]: [MySQL] 55,849 pass(es), 12 fail(s), and 0 exception(s). View
#1 config-sync.patch13.06 KBmarcingy
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch config-sync_0.patch. Unable to apply patch. See the log in the details link for more information. View

Comments

marcingy’s picture

Status: Active » Needs review
FileSize
13.06 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch config-sync_0.patch. Unable to apply patch. See the log in the details link for more information. View

First pass

marcingy’s picture

Title: Convert config_admin_diff_page() to a new style controller » Convert config_sync() to a new style controller

Status: Needs review » Needs work
Issue tags: -WSCCI-conversion

The last submitted patch, config-sync.patch, failed testing.

marcingy’s picture

Status: Needs work » Needs review
Issue tags: +WSCCI-conversion

#1: config-sync.patch queued for re-testing.

aspilicious’s picture

+++ b/core/modules/config/lib/Drupal/config/Form/ConfigSync.phpundefined
@@ -0,0 +1,218 @@
+  ¶

Trailing whitespace

alexpott’s picture

Status: Needs review » Postponed

Postponing this on #1890784: Refactor configuration import and sync functions as that issue will result in significant changes to this patch

vijaycs85’s picture

Status: Postponed » Needs review
vijaycs85’s picture

Issue tags: -WSCCI-conversion

#1: config-sync.patch queued for re-testing.

Status: Needs review » Needs work
Issue tags: +WSCCI-conversion

The last submitted patch, config-sync.patch, failed testing.

swiftsure’s picture

Assigned: Unassigned » swiftsure

Code sprint note.

I am working on this issue.

dougvann’s picture

Assigned: swiftsure » dougvann
Status: Needs work » Needs review
FileSize
7.38 KB
FAILED: [[SimpleTest]]: [MySQL] 55,849 pass(es), 12 fail(s), and 0 exception(s). View

Rerolled patch
corrected white space in lib/Drupal/config/Form/ConfigSync.php
corrected file removal of config.admin.inc

BIG THNX to Benjifisher http://drupal.org/user/683300 for lots of help and my team mate swiftsure for our 1st major contribution.. ;-)

aspilicious’s picture

Status: Needs review » Needs work

The new file is missing :)
Ask for help on creating patches with new files!

Go for it!

dougvann’s picture

Status: Needs work » Needs review
FileSize
14 KB
FAILED: [[SimpleTest]]: [MySQL] 55,831 pass(es), 11 fail(s), and 4 exception(s). View

Thnx aspilicious !
Here it is.

Status: Needs review » Needs work

The last submitted patch, config-sync-1987660-13.patch, failed testing.

dougvann’s picture

Status: Needs work » Needs review
FileSize
14 KB
FAILED: [[SimpleTest]]: [MySQL] 56,194 pass(es), 11 fail(s), and 4 exception(s). View

My n00b mistake has been corrected. Addition of whitespace in hunk has been removed.

Status: Needs review » Needs work

The last submitted patch, config-sync-1987660-15.patch, failed testing.

vijaycs85’s picture

Status: Needs work » Needs review
FileSize
13.03 KB
FAILED: [[SimpleTest]]: [MySQL] Drupal installation failed. View

Re-rolling...

Status: Needs review » Needs work

The last submitted patch, 1987660-config-sync-17.patch, failed testing.

vijaycs85’s picture

Status: Needs work » Needs review
FileSize
534 bytes
13.55 KB
FAILED: [[SimpleTest]]: [MySQL] 58,141 pass(es), 13 fail(s), and 5 exception(s). View

Missed routing file change...

Status: Needs review » Needs work
Issue tags: -WSCCI-conversion

The last submitted patch, 1987660-config-sync-19.patch, failed testing.

vijaycs85’s picture

Status: Needs work » Needs review

#19: 1987660-config-sync-19.patch queued for re-testing.

Status: Needs review » Needs work
Issue tags: +WSCCI-conversion

The last submitted patch, 1987660-config-sync-19.patch, failed testing.

ygerasimov’s picture

Assigned: dougvann » Unassigned
Status: Needs work » Needs review
FileSize
7.29 KB
14.8 KB
PASSED: [[SimpleTest]]: [MySQL] 57,213 pass(es). View

Rebuilt the patch (nearly from scratch). Lets see how tests will pass.

dawehner’s picture

+++ b/core/modules/config/lib/Drupal/config/Form/ConfigSync.phpundefined
@@ -0,0 +1,246 @@
+    $this->config_admin_sync_form($form, $form_state);

Let's camelCase the method.

+++ b/core/modules/config/lib/Drupal/config/Form/ConfigSync.phpundefined
@@ -0,0 +1,246 @@
+  private function config_admin_sync_form(array &$form, array &$form_state) {

I don't see why we need a helper function here ...

+++ b/core/modules/config/lib/Drupal/config/Form/ConfigSync.phpundefined
@@ -0,0 +1,246 @@
+          'title' => t('View differences'),

t could be replaced by the translation manager.

Cottser’s picture

Status: Needs review » Needs work

Based on the above this needs work.

ygerasimov’s picture

Status: Needs work » Needs review
FileSize
6.36 KB
15.02 KB
PASSED: [[SimpleTest]]: [MySQL] 57,124 pass(es). View

Patch changed according to #24. Thank you for the review.

dawehner’s picture

+++ b/core/modules/config/config.routing.ymlundefined
@@ -22,3 +22,9 @@ config_import:
+  pattern: 'admin/config/development/sync'

Pattern should start with a slash.

+++ b/core/modules/config/lib/Drupal/config/Form/ConfigSync.phpundefined
@@ -0,0 +1,246 @@
+   *
+   * @var \Drupal\Core\StringTranslation\TranslationManager
...
+   * @param \Drupal\Core\StringTranslation\TranslationManager
...
+  public function __construct(StorageInterface $sourceStorage, StorageInterface $targetStorage, LockBackendInterface $lock, EventDispatcherInterface $event_dispatcher, ConfigFactory $config_factory, EntityManager $entity_manger, TranslationManager $translation_manager) {

I would vote to go with the TranslatorInterface

+++ b/core/modules/config/lib/Drupal/config/Form/ConfigSync.phpundefined
@@ -0,0 +1,246 @@
+          'href' => 'admin/config/development/sync/diff/' . $config_file,

For now we could use getPathFromRoute on the url generator.

+++ b/core/modules/config/lib/Drupal/config/Form/ConfigSync.phpundefined
@@ -0,0 +1,246 @@
+  /**
+   * {@inheritdoc}
+   */
+  public function validateForm(array &$form, array &$form_state) {
+  }

Let's keep the usual order and move validateForm before submitForm

ygerasimov’s picture

FileSize
4.02 KB
15.54 KB
PASSED: [[SimpleTest]]: [MySQL] 57,521 pass(es). View

Rerolled patch.

dawehner’s picture

Status: Needs review » Reviewed & tested by the community

Just in general this page is really nice with its diff functionality.

alexpott’s picture

Status: Reviewed & tested by the community » Fixed

Committed 0adc91a and pushed to 8.x. Thanks!

-        // Return a negative result for UI purposes. We do not differentiate between
-        // an actual synchronization error and a failed lock, because concurrent
-        // synchronizations are an edge-case happening only when multiple developers
-        // or site builders attempt to do it without coordinating.
+        // Return a negative result for UI purposes. We do not differentiate
+        // between an actual synchronization error and a failed lock, because
+        // concurrent synchronizations are an edge-case happening only when
+        // multiple developers or site builders attempt to do it without
+        // coordinating.

Comment needed to be reformatted so that lines where less than 80 characters long.

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