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
StatusFileSize
new13.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
StatusFileSize
new7.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
StatusFileSize
new14 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
StatusFileSize
new14 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
StatusFileSize
new13.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
StatusFileSize
new534 bytes
new13.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
StatusFileSize
new7.29 KB
new14.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
StatusFileSize
new6.36 KB
new15.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

StatusFileSize
new4.02 KB
new15.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.