Follow up for #1971384: [META] Convert page callbacks to controllers

Rewrite taxonomy_overview_terms (or Drupal\taxonomy\Form\OverviewTerms when #1974492: Convert taxonomy_overview_terms to a Form Controller is in to use a proper route and confirm form instead of an alternate form builder.

Files: 
CommentFileSizeAuthor
#28 taxonomy.oop-reset.1976296.28.patch7.66 KBGaelan
PASSED: [[SimpleTest]]: [MySQL] 57,635 pass(es).
[ View ]
#22 taxo-1976296-22.patch7.65 KBtim.plunkett
PASSED: [[SimpleTest]]: [MySQL] 57,469 pass(es).
[ View ]
#22 interdiff.txt3.59 KBtim.plunkett
#20 1976296-20.patch7.4 KBjibran
FAILED: [[SimpleTest]]: [MySQL] 57,489 pass(es), 5 fail(s), and 1 exception(s).
[ View ]
#20 interdiff.txt5.56 KBjibran
#16 1976296-16.patch7.46 KBjibran
PASSED: [[SimpleTest]]: [MySQL] 57,493 pass(es).
[ View ]
#16 interdiff.txt1.72 KBjibran
#13 drupal-1976296-13.patch7.07 KBdawehner
FAILED: [[SimpleTest]]: [MySQL] 57,150 pass(es), 5 fail(s), and 2 exception(s).
[ View ]
#13 interdiff.txt1.18 KBdawehner
#11 tax-alpha-1976296-11.patch7.08 KBpcambra
FAILED: [[SimpleTest]]: [MySQL] 57,142 pass(es), 5 fail(s), and 1 exception(s).
[ View ]
#4 tax-alpha-1976296-4.patch7.06 KBandypost
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch tax-alpha-1976296-4.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]

Comments

Assigned:Unassigned» larowlan

Assigned:larowlan» sandhya.m
Issue tags:+LONDON_2013_APRIL

Status:Active» Needs review
Issue tags:+Needs manual testing
StatusFileSize
new7.06 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch tax-alpha-1976296-4.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]

Suppose this should be fine!

PS: currently ordering of terms does not work for me

Status:Needs review» Needs work

+++ b/core/modules/taxonomy/taxonomy.routing.ymlundefined
@@ -5,3 +5,10 @@ taxonomy_vocabulary_list:
+  requirements:
+    _entity_access: 'taxonomy_vocabulary.view'

I think we need something like vocabulary or terms update, as this permission is certainly open for way to many people.

@dawehner There was a issue to change list permission so this one should inherit listing access

+++ b/core/modules/taxonomy/taxonomy.routing.ymlundefined
@@ -5,3 +5,10 @@ taxonomy_vocabulary_list:
     _permission: 'administer taxonomy'
...
+    _entity_access: 'taxonomy_vocabulary.view'

This should be the same

Status:Needs work» Needs review
Issue tags:-Needs manual testing, -WSCCI, -FormInterface, -WSCCI-conversion, -LONDON_2013_APRIL

#4: tax-alpha-1976296-4.patch queued for re-testing.

Status:Needs review» Needs work
Issue tags:+Needs manual testing, +WSCCI, +FormInterface, +WSCCI-conversion, +LONDON_2013_APRIL

The last submitted patch, tax-alpha-1976296-4.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new7.08 KB
FAILED: [[SimpleTest]]: [MySQL] 57,142 pass(es), 5 fail(s), and 1 exception(s).
[ View ]

Just a plain reroll of #4

Status:Needs review» Needs work

The last submitted patch, tax-alpha-1976296-11.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new1.18 KB
new7.07 KB
FAILED: [[SimpleTest]]: [MySQL] 57,150 pass(es), 5 fail(s), and 2 exception(s).
[ View ]

That was easy to fix.

Status:Needs review» Needs work

The last submitted patch, drupal-1976296-13.patch, failed testing.

+++ b/core/modules/taxonomy/lib/Drupal/taxonomy/Form/VocabularyReset.phpundefined
@@ -0,0 +1,113 @@
+use Symfony\Component\DependencyInjection\ContainerInterface;

vendor namespaces should be at the end.

+++ b/core/modules/taxonomy/lib/Drupal/taxonomy/Form/VocabularyReset.phpundefined
@@ -0,0 +1,113 @@
+  /**
+   * {@inheritdoc}
+   *
+   * @param \Drupal\taxonomy\VocabularyInterface $taxonomy_vocabulary
+   *   The taxonomy vocabulary to reset.

Last I know this doesn't mix.

+++ b/core/modules/taxonomy/lib/Drupal/taxonomy/Form/VocabularyReset.phpundefined
@@ -0,0 +1,113 @@
+  public function buildForm(array $form, array &$form_state, VocabularyInterface $taxonomy_vocabulary = NULL) {
...
+    return parent::buildForm($form, $form_state);

This also needs $request as param @see ConfirmFormBase::buildForm

Status:Needs work» Needs review
StatusFileSize
new1.72 KB
new7.46 KB
PASSED: [[SimpleTest]]: [MySQL] 57,493 pass(es).
[ View ]

Fixed #15.

Status:Needs review» Needs work
Issue tags:-Needs manual testing, -WSCCI, -FormInterface, -WSCCI-conversion, -LONDON_2013_APRIL

The last submitted patch, 1976296-16.patch, failed testing.

Status:Needs work» Needs review
Issue tags:+Needs manual testing, +WSCCI, +FormInterface, +WSCCI-conversion, +LONDON_2013_APRIL

#16: 1976296-16.patch queued for re-testing.

Looking at this code, I think it might make more sense as extending EntityConfirmFormBase.
Those aren't just for add/edit/delete, we use it in views for the "break lock" form.
It shouldn't be too much to switch over.

Assigned:sandhya.m» Unassigned
StatusFileSize
new5.56 KB
new7.4 KB
FAILED: [[SimpleTest]]: [MySQL] 57,489 pass(es), 5 fail(s), and 1 exception(s).
[ View ]

Now with EntityConfirmFormBase.

Status:Needs review» Needs work

+++ b/core/modules/taxonomy/taxonomy.routing.ymlundefined
@@ -25,3 +25,10 @@ taxonomy_vocabulary_delete:
+    _entity_access: 'taxonomy_vocabulary.view'

this access permission still wrong - see #7 & #8

Status:Needs work» Needs review
Issue tags:-Needs manual testing
StatusFileSize
new3.59 KB
new7.65 KB
PASSED: [[SimpleTest]]: [MySQL] 57,469 pass(es).
[ View ]

Rerolled with some fixes. Manually tested as well.

Status:Needs review» Reviewed & tested by the community

Great!

Status:Reviewed & tested by the community» Needs review
Issue tags:+Needs manual testing

#20 demonstrates that this does have test coverage, but let's please test manually at least once and document it on the issue? Thanks!

Issue tags:-Needs manual testing

I manually tested in #22.

When viewing the listing of an empty vocabulary, or one with one term, there was no reset button (as expected)
When viewing the listing with more than one term, while they are both in alphabetical order, the button appears, leads to the confirmation form, submits and shows the correct message, and does not alter the weight of the terms (as expected)
After manually reordering the terms to be out of order, clicking the button and confirming the form resets the terms to be in the correct order (as expected)

Status:Needs review» Reviewed & tested by the community

Thanks @tim.plunkett!

Status:Reviewed & tested by the community» Needs work
Issue tags:+Needs reroll

Needs a reroll

git ac https://drupal.org/files/taxo-1976296-22.patch
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
100  7835  100  7835    0     0   4498      0  0:00:01  0:00:01 --:--:--  5265
error: patch failed: core/modules/taxonomy/taxonomy.admin.inc:396
error: core/modules/taxonomy/taxonomy.admin.inc: patch does not apply

Status:Needs work» Reviewed & tested by the community
StatusFileSize
new7.66 KB
PASSED: [[SimpleTest]]: [MySQL] 57,635 pass(es).
[ View ]

Rerolled.

Issue tags:-Needs reroll

Tags.

Status:Reviewed & tested by the community» Fixed

Committed 29907b0 and pushed to 8.x. Thanks!

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