Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
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.
Comment | File | Size | Author |
---|---|---|---|
#28 | taxonomy.oop-reset.1976296.28.patch | 7.66 KB | Gaelan |
#22 | taxo-1976296-22.patch | 7.65 KB | tim.plunkett |
#22 | interdiff.txt | 3.59 KB | tim.plunkett |
#20 | 1976296-20.patch | 7.4 KB | jibran |
#20 | interdiff.txt | 5.56 KB | jibran |
Comments
Comment #1
larowlanComment #2
andypostRelated issues #1978112: Convert taxonomy admin path to follow other core entity patterns and #1946456: Convert taxonomy_term_confirm_delete() and taxonomy_vocabulary_confirm_delete() to the new form interface
Comment #3
sandhya.m CreditAttribution: sandhya.m commentedComment #4
andypostSuppose this should be fine!
PS: currently ordering of terms does not work for me
Comment #5
andypostFiled separate issue #2020841: Order of terms broken after 'Reset to alphabetical' and 'Save Order'
Comment #6
andypostOrdering would be fixed in #2009680: Replace theme() with drupal_render() in taxonomy module
Comment #7
dawehnerI think we need something like vocabulary or terms update, as this permission is certainly open for way to many people.
Comment #8
andypost@dawehner There was a issue to change list permission so this one should inherit listing access
This should be the same
Comment #9
jibran#4: tax-alpha-1976296-4.patch queued for re-testing.
Comment #11
pcambraJust a plain reroll of #4
Comment #13
dawehnerThat was easy to fix.
Comment #15
jibranvendor namespaces should be at the end.
Last I know this doesn't mix.
This also needs $request as param @see ConfirmFormBase::buildForm
Comment #16
jibranFixed #15.
Comment #18
jibran#16: 1976296-16.patch queued for re-testing.
Comment #19
tim.plunkettLooking 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.
Comment #20
jibranNow with
EntityConfirmFormBase
.Comment #21
andypostthis access permission still wrong - see #7 & #8
Comment #22
tim.plunkettRerolled with some fixes. Manually tested as well.
Comment #23
andypostGreat!
Comment #24
xjm#20 demonstrates that this does have test coverage, but let's please test manually at least once and document it on the issue? Thanks!
Comment #25
tim.plunkettI 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)
Comment #26
xjmThanks @tim.plunkett!
Comment #27
alexpottNeeds a reroll
Comment #28
Gaelan CreditAttribution: Gaelan commentedRerolled.
Comment #29
star-szrTags.
Comment #30
alexpottCommitted 29907b0 and pushed to 8.x. Thanks!