I was going to add test coverage over in #2046993: Inject dependencies into ViewsExposedFilterBlock derivative class, but in starting to do so realised that exposed blocks don't actually work at all at the moment. This is not a problem with the deriver but with the actual ViewsExposedBlock block plugin.

Test coverage should be added here instead.

Steps to reproduce:
- Create view, add page display
- Under 'advanced' change 'Exposed form in block' setting to yes
- Save
- Try to place this new exposed block
- Watch things break

CommentFileSizeAuthor
#8 2057831-8.patch15.3 KBdamiankloip
#8 interdiff-2057831-8.txt595 bytesdamiankloip
#2 2057831-2-tests-only.patch4.72 KBdamiankloip
#2 2057831-2.patch15.29 KBdamiankloip

Comments

damiankloip’s picture

damiankloip commented
Issue tags: +Needs tests

.

damiankloip’s picture

damiankloip commented
Status: Active » Needs review
StatusFileSize
new 2057831-2.patch15.29 KB
new 2057831-2-tests-only.patch4.72 KB
dawehner’s picture

dawehner
Primary language German
commented
Issue tags: -Needs tests

I really like to split up the functionality of the two kind of views blocks!

+++ b/core/modules/views/lib/Drupal/views/Tests/Plugin/ExposedFormTest.phpundefined
@@ -102,11 +104,45 @@ public function testExposedFormRender() {
+   * @param ViewStorageInterface $view
...
+  protected function getExpectedExposedFormId(ViewExecutable $view) {

The signature and documentation seem to disagree.

dawehner’s picture

dawehner
Primary language German
commented

I really like to split up the functionality of the two kind of views blocks!

+++ b/core/modules/views/lib/Drupal/views/Tests/Plugin/ExposedFormTest.phpundefined
@@ -102,11 +104,45 @@ public function testExposedFormRender() {
+   * @param ViewStorageInterface $view
...
+  protected function getExpectedExposedFormId(ViewExecutable $view) {

The signature and documentation seem to disagree.

Status: Needs review » Needs work
Issue tags: -VDC

The last submitted patch, 2057831-2.patch, failed testing.

damiankloip’s picture

damiankloip commented
Status: Needs work » Needs review
Issue tags: +VDC

#2: 2057831-2.patch queued for re-testing.

dawehner’s picture

dawehner
Primary language German
commented 
+++ b/core/modules/views/lib/Drupal/views/Tests/Plugin/ExposedFormTest.php
@@ -102,11 +104,45 @@ public function testExposedFormRender() {
+   * @param ViewStorageInterface $view
...
+  protected function getExpectedExposedFormId(ViewExecutable $view) {

The @param talks about ViewStorageInterface, but the function has ViewExecutable!

damiankloip’s picture

damiankloip commented
StatusFileSize
new interdiff-2057831-8.txt595 bytes
new 2057831-8.patch15.3 KB

Good call, that line just doesn;t match :)

dawehner’s picture

dawehner
Primary language German
commented
Status: Needs review » Reviewed & tested by the community

I really like having a new test for that bit.

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
commented
Priority: Normal » Critical

That sounds much worse than a "normal" bug. :)

I'll need some help walking through this patch on IRC or whatever, since after 3 months off I'm not up to speed on the new way blocks are defined and stuff. Else, we can just leave it for Alex/catch/Dries.

dawehner’s picture

dawehner
Primary language German
commented

Exposed filters as a block is certainly not a often used functionality. I guess most of us will be online later anyway.

catch’s picture

catch
he/him
Primary language English
commented
Status: Reviewed & tested by the community » Fixed

Reviewed this, looks like good cleanup as well as the bug fix.
Committed/pushed to 8.x, thanks!

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

Anonymous’s picture

(not verified) commented
Issue summary: View changes

Updated issue summary.