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.
To reproduce:
- Go to admin/structure/block/list/block_plugin_ui%3Abartik/add
- Fill in a search word in and hit 'Next'
You get a notice and are also (second bug?) redirected to the wrong page.
Comment | File | Size | Author |
---|---|---|---|
#14 | 1880378-14.patch | 4.73 KB | EclipseGc |
#10 | 1880378-tests-only-10.patch | 1.86 KB | swentel |
#10 | 1880378-10.patch | 4.27 KB | swentel |
#7 | 1880378-7.patch | 2.41 KB | EclipseGc |
#2 | 1880378-2.patch | 1.7 KB | swentel |
Comments
Comment #1
swentel CreditAttribution: swentel commentedComment #2
swentel CreditAttribution: swentel commentedBrings back the autocomplete (code by EclipseGc) and fixes the redirect as well.
Comment #3
swentel CreditAttribution: swentel commentedJust a question: should we validate the machine name on submit, because otherwise, you get a big error message like:
"Drupal\Component\Plugin\Exception\PluginException: The plugin (recent_comments_) did not specify an instance class. in Drupal\Component\Plugin\Factory\DefaultFactory->getPluginClass() (line 60 of /Users/drupal/drupal/core/lib/Drupal/Component/Plugin/Factory/DefaultFactory.php)"
Comment #4
aspilicious CreditAttribution: aspilicious commentedthe machine name has an issue alrdy to turn it into an autocompleted field (like we have with other machine names). We should add more validation there. (and add some tests)
Comment #5
aspilicious CreditAttribution: aspilicious commentedWhats the point of a string argument if it gets overriden anyway?
Comment #6
EclipseGc CreditAttribution: EclipseGc commentedIn the code I sent to swentel the first time I had removed that, he probably just missed it when he added it to his code.
I would like a validation on this autocomplete to make sure that we're requesting a valid plugin_id. I think that makes a lot of good sense.
Comment #7
EclipseGc CreditAttribution: EclipseGc commentedok, this fixes the issue aspilicious pointed out, and adds some validation to the form to prevent the issues swentel mentioned.
Eclipse
Comment #8
tim.plunkettTagging
Comment #9
swentel CreditAttribution: swentel commentedWill add tests
Comment #10
swentel CreditAttribution: swentel commentedTests only + Tests and fix patch.
Comment #12
tim.plunkett#10: 1880378-10.patch queued for re-testing.
Comment #13
tim.plunkettThe fix looks good, but a couple nitpicks.
Missing a docblock
Should be protected.
This seems like a good time to fix this todo.
Comment #14
EclipseGc CreditAttribution: EclipseGc commentedOk, attempted to take care of tim's comments. setUp() is now protected, we discussed the test method and agreed that didn't need to be protected. The variable that he wanted docs for was completely unnecessary. I removed it. More extensive docs added for system_plugin_autocomplete().
Eclipse
Comment #15
swentel CreditAttribution: swentel commentedLooks good to me.
Comment #16
webchickCommitted and pushed to 8.x. Thanks!