Since #1995048: EntityListController::getOperations() should respect access checks went in, all entity types with list controllers need an access controller.
FilterFormat doesn't have one.

More importantly, no tests failed.

To reproduce, go to admin/config/content/formats and notice you have no links to edit the formats.

Uploading a patch from that issue, but it needs tests first anyway.

CommentFileSizeAuthor
#8 filter-access-controller-2030129-8.patch3.92 KBa_c_m
#5 filter-access-controller-2030129-5.patch2.61 KBa_c_m
#4 filter-configure-link-check-2030129-4.patch1.19 KBa_c_m
#3 filter-configure-link-check-2030129-3.patch1.12 KBa_c_m
filter-access-controller-do-not-test.patch2.73 KBtim.plunkett

Comments

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented

We need to add a test using clickLink() so that we actually test the page

a_c_m’s picture

a_c_m commented

Ok, been digging about.

I see the error. But don't understand why its not failing as is ?

I've viewed source on admin/config/content/formats and i can't find the link the test asserts should be there ()... so i'm confused as to how the $this->assertLinkByHref isn't failing...

Also clickLink (https://api.drupal.org/api/drupal/modules!simpletest!drupal_web_test_cas...) doesn't appear to have a way to select which version of the link you want, as there may be more than one "Configure" link on the page, i wanted to be sure we were clicking the right one.

As i'm new to all this i expect i'm missing the point somewhere.

a_c_m’s picture

a_c_m commented
StatusFileSize
new filter-configure-link-check-2030129-3.patch1.12 KB

So yeah, turns out its sort of an issue with assertLinkByHref which checks for contains, instead of an exact match.

ContactSiteWideTest.php has a fix, which i will copy. But, it looks like that code had its own issue ;) So will fix that as well in another issue (#2031211: Check for URL's absence will always pass due to wrong var name)

a_c_m’s picture

a_c_m commented
StatusFileSize
new filter-configure-link-check-2030129-4.patch1.19 KB

Updated patch to reference parent issue : #2031223: Add linkByHrefExistsExact and linkByHrefNotExistsExact for matching links by href exactly

a_c_m’s picture

a_c_m commented
Status: Active » Needs review
StatusFileSize
new filter-access-controller-2030129-5.patch2.61 KB

Putting both patches together.

The last submitted patch, filter-access-controller-2030129-5.patch, failed testing.

a_c_m’s picture

a_c_m commented
Status: Needs work » Needs review
Issue tags: +Needs tests

#5: filter-access-controller-2030129-5.patch queued for re-testing.

a_c_m’s picture

a_c_m commented
StatusFileSize
new filter-access-controller-2030129-8.patch3.92 KB

adding missing file.

Status: Needs review » Needs work

The last submitted patch, filter-access-controller-2030129-8.patch, failed testing.

berdir’s picture

berdir
Primary language German
Location Switzerland
commented

Looks like a random fail: Last checked is updated Other LocaleUpdateCronTest.php 120 Drupal\locale\Tests\LocaleUpdateCronTest->testUpdateCron().

berdir’s picture

berdir
Primary language German
Location Switzerland
commented
Status: Needs work » Needs review

#8: filter-access-controller-2030129-8.patch queued for re-testing.

berdir’s picture

berdir
Primary language German
Location Switzerland
commented
Status: Needs review » Reviewed & tested by the community
Issue tags: -Needs tests

This looks good, thank you.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Status: Reviewed & tested by the community » Fixed

Noticed a comment error... did the following change on change (thanks @berdir)

-    // Cannot use ::assertNoLinkByHref as it does partial url matching and with
-    // field_ui enabled 'admin/config/content/formats/manage/' . $format_id . '/disable' may exist.
+    // Cannot use the assertNoLinkByHref method as it does partial url matching
+    // and 'admin/config/content/formats/manage/' . $format_id . '/disable'
+    // exists.

Committed 165ec2c and pushed to 8.x. Thanks!

Status: Fixed » Closed (fixed)

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