The preview screen when working with views does not process a single value of '0'. Since arguments/context filters could have a value of zero, this is problematic.

the steps to reproduce:

1) enable the 'show the query' setting in views
2) create a standard content view
3) add a context filter for a numeric field - such as node nid
4) type the number 1 into the the "preview with contextual filters" box then click update

at this point, note that the query has the expected "where node.nid=1"

5) type the number 0 into the "preview with contextual filters" box, then click update

now note that the entire condition is gone, when it should say "where node.nid=0"

This is a result of the following code in viewPreviewForm.php which uses empty() to test the input:

    if (!empty($form_state['values']['view_args'])) {
      $args = explode('/', $form_state['values']['view_args']);
    }

Empty, unfortunately doesn't mean "empty", it means without value, so the values of 0 and "0" are considered empty. A simple resolution is to add a test for "$form_state['values']['view_args']==0".

Comments

netw3rker’s picture

Status: Active » Needs review
StatusFileSize
new648 bytes

Here's a patch for this.

Status: Needs review » Needs work

The last submitted patch, 1: 2313073-1_views-preview-ignores-zero-value.patch, failed testing.

tim.plunkett’s picture

Version: 8.x-dev » 8.0.x-dev

Version: 8.0.x-dev » 8.1.x-dev

Drupal 8.0.6 was released on April 6 and is the final bugfix release for the Drupal 8.0.x series. Drupal 8.0.x will not receive any further development aside from security fixes. Drupal 8.1.0-rc1 is now available and sites should prepare to update to 8.1.0.

Bug reports should be targeted against the 8.1.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.2.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.1.x-dev » 8.2.x-dev

Drupal 8.1.9 was released on September 7 and is the final bugfix release for the Drupal 8.1.x series. Drupal 8.1.x will not receive any further development aside from security fixes. Drupal 8.2.0-rc1 is now available and sites should prepare to upgrade to 8.2.0.

Bug reports should be targeted against the 8.2.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.3.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.2.x-dev » 8.3.x-dev

Drupal 8.2.6 was released on February 1, 2017 and is the final full bugfix release for the Drupal 8.2.x series. Drupal 8.2.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.3.0 on April 5, 2017. (Drupal 8.3.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.3.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.4.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.3.x-dev » 8.4.x-dev

Drupal 8.3.6 was released on August 2, 2017 and is the final full bugfix release for the Drupal 8.3.x series. Drupal 8.3.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.4.0 on October 4, 2017. (Drupal 8.4.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.4.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.5.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

skylord’s picture

Rerolled for 8.3.x

jwkovell’s picture

Using Drupal 8.3.7

Patch applied cleanly and fixed the issue for me.

jwkovell’s picture

Status: Needs work » Needs review

Also applies and works on Drupal 8.5.x-dev.

I'm not familiar with the automated testing system or how to read the results. The failures seem... unrelated to the patch?

I'm hesitant to switch to "RTBC" unless someone more familiar with the testing system can confirm those errors aren't a problem. Switching to "Needs (further) Review" instead.

lendude’s picture

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

The fails in #8 look related since they are working with arguments, so this fix is not right (didn't dive into why).
@jkovell if you are in doubt about whether or not a fail is related, try applying the patch to a local Drupal 8 checkout and running the same tests that failed on the testbot locally.

Also, this needs tests.

Version: 8.4.x-dev » 8.5.x-dev

Drupal 8.4.4 was released on January 3, 2018 and is the final full bugfix release for the Drupal 8.4.x series. Drupal 8.4.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.5.0 on March 7, 2018. (Drupal 8.5.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.5.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.6.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.5.x-dev » 8.6.x-dev

Drupal 8.5.6 was released on August 1, 2018 and is the final bugfix release for the Drupal 8.5.x series. Drupal 8.5.x will not receive any further development aside from security fixes. Sites should prepare to update to 8.6.0 on September 5, 2018. (Drupal 8.6.0-rc1 is available for testing.)

Bug reports should be targeted against the 8.6.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.7.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.6.x-dev » 8.8.x-dev

Drupal 8.6.x will not receive any further development aside from security fixes. Bug reports should be targeted against the 8.8.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.9.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 8.8.x-dev » 8.9.x-dev

Drupal 8.8.7 was released on June 3, 2020 and is the final full bugfix release for the Drupal 8.8.x series. Drupal 8.8.x will not receive any further development aside from security fixes. Sites should prepare to update to Drupal 8.9.0 or Drupal 9.0.0 for ongoing support.

Bug reports should be targeted against the 8.9.x-dev branch from now on, and new development or disruptive changes should be targeted against the 9.1.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 8.9.x-dev » 9.2.x-dev

Drupal 8 is end-of-life as of November 17, 2021. There will not be further changes made to Drupal 8. Bugfixes are now made to the 9.3.x and higher branches only. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.2.x-dev » 9.3.x-dev

Version: 9.3.x-dev » 9.4.x-dev

Drupal 9.3.15 was released on June 1st, 2022 and is the final full bugfix release for the Drupal 9.3.x series. Drupal 9.3.x will not receive any further development aside from security fixes. Drupal 9 bug reports should be targeted for the 9.4.x-dev branch from now on, and new development or disruptive changes should be targeted for the 9.5.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

smustgrave’s picture

Status: Needs work » Needs review
StatusFileSize
new2.18 KB

Could the test be like this?

lendude’s picture

Status: Needs review » Needs work
Issue tags: -Needs tests
  1. +++ b/core/modules/views_ui/src/ViewPreviewForm.php
    @@ -49,7 +49,7 @@ public function form(array $form, FormStateInterface $form_state) {
    +    if (!$form_state->isValueEmpty('view_args') || ($form_state->getValue('view_args') !== NULL && $form_state->getValue('view_args') == 0)) {
    

    ($form_state->getValue('view_args') !== NULL && $form_state->getValue('view_args') == 0)) could just be $form_state->getValue('view_args') === 0 right?

  2. +++ b/core/modules/views_ui/tests/src/Functional/PreviewTest.php
    @@ -30,7 +30,7 @@ public function testPreviewContextual() {
    +    $this->submitForm([], 'Update preview');
    

    I get it, but strictly speaking an unrelated change

mrinalini9’s picture

Status: Needs work » Needs review
StatusFileSize
new1.63 KB
new1.27 KB

Updated patch #19 by addressing #20, please review it.

Status: Needs review » Needs work

The last submitted patch, 21: 2313073-21.patch, failed testing. View results

smustgrave’s picture

Status: Needs work » Needs review
StatusFileSize
new569 bytes
new1.68 KB

Sorry about the $edit = [] change just was annoying to look at.

So don't think === 0 can be used. If you test the patch in #21 manually it doesn't work. (So the test cases worked!)
Adding back 20.1

lendude’s picture

Status: Needs review » Needs work

If === 0 doesn't work, but == 0 does, then we seem to be depending on loose typing, which is a bad idea. So this probably gets passed as a string, so it should be === '0', does that make it pass?

smustgrave’s picture

StatusFileSize
new569 bytes
new1.64 KB

That works too

smustgrave’s picture

Status: Needs work » Needs review
abhijith s’s picture

StatusFileSize
new37.13 KB
new144.92 KB

Applied patch #25 and the query is showing correctly after applying this patch.Attaching screen recordings for reference.

smustgrave’s picture

Status: Needs review » Reviewed & tested by the community

Moving to RTBC based on comment #27

alexpott’s picture

Status: Reviewed & tested by the community » Needs work
+++ b/core/modules/views_ui/src/ViewPreviewForm.php
@@ -49,7 +49,7 @@ public function form(array $form, FormStateInterface $form_state) {
-    if (!$form_state->isValueEmpty('view_args')) {
+    if (!$form_state->isValueEmpty('view_args') || $form_state->getValue('view_args') === '0') {

I think this should be if($form_state->hasValue('view_args')) { - simpler logic and more correct.

asad_ahmed’s picture

Assigned: Unassigned » asad_ahmed
asad_ahmed’s picture

Assigned: asad_ahmed » Unassigned
Status: Needs work » Needs review
StatusFileSize
new1.63 KB
new517 bytes

Made changes as per #29

asad_ahmed’s picture

StatusFileSize
new517 bytes

Please ignore the above interdiff, I have mistakenly mentioned 24 as a old patch number which is 25 actually, uploading the right one.

asad_ahmed’s picture

Assigned: Unassigned » asad_ahmed
asad_ahmed’s picture

StatusFileSize
new2.02 KB
new848 bytes

Fixed the above patches and made changes as per #29

asad_ahmed’s picture

Assigned: asad_ahmed » Unassigned

Status: Needs review » Needs work

The last submitted patch, 34: 2313073-34.patch, failed testing. View results

alexpott’s picture

Ah interesting... the form API's ::hasValue and isValueEmpty just seem dangerous...

I think the most elegant and logically correct formulation is then:

    if ($form_state->getValue('view_args', '') !== '') {
      $args = explode('/', $form_state->getValue('view_args'));
    }
Ratan Priya’s picture

Assigned: Unassigned » Ratan Priya
smustgrave’s picture

Status: Needs work » Needs review
Issue tags: +Bug Smash Initiative
StatusFileSize
new813 bytes
new1.99 KB
Munavijayalakshmi’s picture

Assigned: Ratan Priya » Munavijayalakshmi
Status: Needs review » Reviewed & tested by the community
StatusFileSize
new85.34 KB
new85.53 KB

Patch #39 resolved the issue.

Munavijayalakshmi’s picture

Version: 9.4.x-dev » 9.5.x-dev
Assigned: Munavijayalakshmi » Unassigned

Patch #39 applied successfully in 9.5.x and resolved the issue.

Munavijayalakshmi’s picture

StatusFileSize
new48.9 KB

On branch 9.5.x.

alexpott’s picture

Version: 9.5.x-dev » 9.4.x-dev
Status: Reviewed & tested by the community » Fixed

@Munavijayalakshmi thanks for the before and after screenshots proving the patch works - that is useful. The screenshot of the patch being applied in #42 is not.

Committed and pushed 43ea903a40 to 10.1.x and b5faa5d0e9 to 10.0.x and 453e372351 to 9.5.x and ac62c5e5e6 to 9.4.x. Thanks!

  • alexpott committed 43ea903 on 10.1.x
    Issue #2313073 by smustgrave, asad_ahmed, mrinalini9, Munavijayalakshmi...

  • alexpott committed b5faa5d on 10.0.x
    Issue #2313073 by smustgrave, asad_ahmed, mrinalini9, Munavijayalakshmi...

  • alexpott committed 453e372 on 9.5.x
    Issue #2313073 by smustgrave, asad_ahmed, mrinalini9, Munavijayalakshmi...

  • alexpott committed ac62c5e on 9.4.x
    Issue #2313073 by smustgrave, asad_ahmed, mrinalini9, Munavijayalakshmi...

Status: Fixed » Closed (fixed)

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