Problem/Motivation

I have a numeric field that holds pricing data. If the pricing data in the database is 0.00 or NULL and the field is set to round (I'm rounding to 2 decimal places) and the "Count the number 0 as empty" and "Hide if empty" boxes are checked, then the field is output as 0.00 (expected: hidden). The field is successfully hidden if rounding is disabled.

Probably unnecessary info:
- price column in MySQL is of type decimal(6,2)
- Views integration via Entity API (7.x-1.x-dev)

Steps to reproduce

  • Add a Number (Decimal) field to a content type
  • Create a view of fields as an unformatted list with this content type and add the Number field
  • In the field display settings, make sure Scale is set to 2
  • In 'No results behaviour', tick 'Hide if empty' and 'Count the number 0 as empty'
  • Create nodes with differing numeric values: e.g. NULL (unfilled), 0, 0.0, 0.01 (give the node a corresponding title so you can tell which is which)
  • See that the field is displayed for nodes with 0.00
  • Update the field display settings in the view to set Scale to 0
  • See that the field no longer appears

Proposed resolution

Remaining tasks

Answer #49.1 and #492.

User interface changes

API changes

Data model changes

Release notes snippet

Issue fork drupal-1232920

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

dawehner’s picture

Assigned: Unassigned » merlinofchaos
Status: Active » Needs review
FileSize
1.21 KB

Here is a patch which allows to do it.

Assign to merlinofchaos for a logical review.

jdleonard’s picture

Unfortunately this isn't working for me.

drupik’s picture

Its working for me on comment count field :) but i still looking for solution for last comment link, when no comment it still showing :(

Thanks for patch:)

dawehner’s picture

@jdleonard with which field did you tested it?

jdleonard’s picture

Assigned: merlinofchaos » Unassigned
Status: Needs review » Needs work

I figured out why the patch in #1 isn't working for me. In my case, $this->last_render equals "$0.00" because there's a prefix (it's a price). I'm not sure the right way to get the original value. It does appear to be available in $values (I see it as NULL).

dawehner’s picture

Status: Needs work » Needs review

There is already another isse for the prefix/suffix empty problem #1239522: Prefix causes empty on 0 fields to not be empty
As this is about the value itself, it's another issue.

jdleonard’s picture

I believe that's a different problem. Unlike in that issue, my prefix is set in Views. I'm not using the Field API. It seems to me that this should be fixed as part of this issue. I'm super happy to help, but I'm not sure the appropriate way and place to check the original value.

Junro’s picture

Same problem, with D6 and Views 3.x.

When using round numbers, empty values are displayed : 0.0

Codes from Patch #1 doesn't work for me.

#1163412: Count number 0 as empty / vote results

Junro’s picture

Empty text (Provide text to display if this field returns no results.) doesn't work as well when "Round" numbers option is check.

jdleonard’s picture

Status: Needs review » Needs work
dawehner’s picture

Can you please tell some more words what you did to test the patch?

Junro’s picture

handlers/views_handler_field.inc

I took of

-    if (empty($this->last_render)) {
-      if (($this->last_render !== 0 && $this->last_render !== '0') || !empty($this->options['empty_zero'])) {
-        $alter = $this->options['alter'];
-        $alter['alter_text'] = 1;
-        $alter['text'] = $this->options['empty'];
-        $this->last_render = $this->render_text($alter);
-      }

and replaced by

+    $is_number_zero = is_numeric($this->last_render) && $this->last_render == 0;
+    // If the user has selected to handle zero as empty, check it numerical.
+    // Else check whether it's not numerical empty.
+    if ((empty($this->last_render) && !$is_number_zero) || (!empty($this->options['empty_zero']) && $is_number_zero)) {
+      $alter = $this->options['alter'];
+      $alter['alter_text'] = 1;
+      $alter['text'] = $this->options['empty'];
+      $this->last_render = $this->render_text($alter);

But as I said, I'm using D6. Even if Views 6.3 looks like the same than d7 version, maybe 6.3 needs adjustements. I don't know....

dawehner’s picture

Well everyone want's to fix this bug. So people have to be able to understand how you used views (provide an export) etc.

Just imagine that we don't have your full drupal configuration.

sdsheridan’s picture

I did a simple fix for this in views_handler_field_numeric.inc (D6 version). The problem I discovered is in this line:

if ($this->options['hide_empty'] && empty($value) && ($value !== 0 || $this->options['empty_zero'])) {
  return '';
}

After all the manipulation before this test, if you set a precision to the number, what you get is a string that is something like '0.00', which is not empty. So the condition fails at the empty($value) bit. Also, I wondered why all the above work was being done if we would get here and possibly not need it. So, I changed the function as follows:

  function render($values) {
    $value = $values->{$this->field_alias};
    // PATCHED to test if the possible string coming in as the value is in reality really zero, and
    // moved this code up to avoid doing the rest of the work if it is
    // Check to see if hiding should happen before doing the rest of the work.
    $fvalue = ( is_null($value) ? $value : (float) $value );
    if ($this->options['hide_empty'] && empty($fvalue) && ($fvalue !== 0 || $this->options['empty_zero'])) {
      return '';
    }

    if (!empty($this->options['set_precision'])) {
      $value = number_format($value, $this->options['precision'], $this->options['decimal'], $this->options['separator']);
    }
    else {
      $remainder = abs($value) - intval(abs($value));
      $value = $value > 0 ? floor($value) : ceil($value);
      $value = number_format($value, 0, '', $this->options['separator']);
      if ($remainder) {
        // The substr may not be locale safe.
        $value .= $this->options['decimal'] . substr($remainder, 2);
      }
    }

    // Should we format as a plural.
    if (!empty($this->options['format_plural'])) {
      $value = format_plural($value, $this->options['format_plural_singular'], $this->options['format_plural_plural']);
    }

    return filter_xss($this->options['prefix']) . check_plain($value) . filter_xss($this->options['suffix']);
  }

I moved the test higher up to avoid doing unnecessary work, and then cast the incoming value to a float, and tested the float (leaving it as null it it came in as null, being the representation of 'empty' in the 'hide if empty' case). Regardless of how the string $value comes in (and assuming it will always be a string representing a number if it gets this far), casting anything that looks like zero to a float will make it indeed zero. So, if value comes in as '0.0000', $fvalue will be 0, unless of course it was NULL, in which case it remains NULL. This solved the problem for me.

Shawn

tanc’s picture

Thank you sdsheridan, your approach worked nicely for a custom handler where I needed this functionality. I think it makes a lot of sense for the views_handler_field_numeric handler to do this check early and bail.

chiddicks’s picture

Issue summary: View changes
Status: Needs work » Needs review
FileSize
1.27 KB

I've made a patch with sdsheridan's logic reordering for Views' core views_handler_field_numeric. It fixed the issue for me. Please post back reviews as I think we can get this fixed.

Anybody’s picture

Status: Needs review » Reviewed & tested by the community

#16 works great for me and since we don't have any broken tests or negative feedback, let's set this RTBC!

The last submitted patch, 1: 1232920-empty-0.0.patch, failed testing.

chiddicks’s picture

fonant’s picture

Patch #16 works nicely here :) Rounded numeric fields with suffix now hide properly when the field value is zero.

I'm using this for a view with a geofield proximity filter, so the proximity distance field output is shown when the filter is in use, but is hidden when there is no location to filter by (resulting in all values being NULL).

akadimi’s picture

Patch #16 didn't work fo me. If it can help the field is coming from a relationship to a field collection items

ctrlADel’s picture

The patch in #16 didn't work for me either.

I did find a workaround though. If you to set the rounding to 0 and then rewrite the field as the token [field_name_here-value] it works. It removes your ability to round but I didn't need that anyways.

kristiaanvandeneynde’s picture

Patch in #16 worked flawlessly for me, RTBC here as well.

GuyPaddock’s picture

Curiously, this doesn't seem to fix issues with precision. I have a Float field that has a column with a precision set to 2 (since when it *does* have data, we want it to be a number like 2.50 or 6.75), and I have both "Hide if empty" and "Count 0 as empty" set. If the precision is 0, it works fine. If it's not 0, then it doesn't work even with this patch.

Even stranger is that I added a Devel print statement above the code this patch affects in render() and it's not called for the columns in question. I wonder what's overriding it.

GuyPaddock’s picture

See #2450401: Multiple issues with is_value_empty() if you have the related issue, like I did, of fields with precision (not rounding) not being hidden properly.

colan’s picture

We've recently switched our testing from the old qa.drupal.org to DrupalCI. Because of a bug in the new system, #2623840: Views (D7) patches not being tested, older patches must be re-uploaded. On re-uploading the patch, please set the status to "Needs Review" so that the test bot will add it to its queue.

If all tests pass, change the Status back to "Reviewed & tested by the community". We'll most likely commit the patch immediately without having to go through another round of peer review.

We apologize for the trouble, and appreciate your patience.

Anybody’s picture

Status: Reviewed & tested by the community » Needs review
FileSize
1.27 KB

Re-uploaded #16 as of #26.

Anybody’s picture

Status: Needs review » Reviewed & tested by the community

  • colan committed 94efb78 on 7.x-3.x authored by chiddicks
    Issue #1232920 by dawehner, Anybody, chiddicks: Hide earlier in the...
colan’s picture

Project: Views (for Drupal 7) » Drupal core
Version: 7.x-3.x-dev » 8.0.x-dev
Component: Code » views.module
Status: Reviewed & tested by the community » Patch (to be ported)

Thanks!

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.

asirjacques’s picture

Hello,

I've ported the patch to D8.2.x following the latest patch logic.

Let me know if there something else I should do as this is new to me. Thanks.

asirjacques’s picture

dawehner’s picture

Status: Patch (to be ported) » Needs work
Issue tags: +Needs tests

Thank you for porting the patch.

We would have some tests as well to ensure this bug never appears again.

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.

ChristianAdamski’s picture

Status: Needs work » Needs review
joelpittet’s picture

Status: Needs review » Needs work

@ChristianAdamski this was at "needs work" to add test coverage to prevent this from regressing in the future.

Anonymous’s picture

Status: Needs work » Needs review
FileSize
1.86 KB

#33 not work for me. Steps for reproduce:

  1. Create type with float field
  2. Create node with field value = 0.0
  3. Create views with display fields and set options "Count the number 0 as empty", "Hide if empty", "Hide rewriting if empty".

Looks like NumericField::render() just not called (full clear cache not helps)

+1 to check the empty of value before rendering.

Also scale options should not empty input (''), because number_format() not works correctly with this.

Attached patch works for me. Set NR just for Bot. I will try to write the tests later. It would be nice to know when #33 is works, it helps to do test for that case too.

Anonymous’s picture

Status: Needs review » Needs work
Anonymous’s picture

The last submitted patch, 41: 1232920-41-test-only.patch, failed testing.

The last submitted patch, 41: patch_33_1232920-41-fail.patch, failed testing.

wturrell’s picture

Can we improve steps to reproduce in the issue summary?

I'm sure I'm missing something obvious, but I can't find "Count the number 0 as empty" and "Hide is empty" in the field settings or display widget for Number (float) and Number (decimal).

[EDIT] Never mind, figured it out (Views, add a Number (decimal) field, No results behaviour).
Able to reproduce and patch worked. Will attempt to review properly shortly…

Anonymous’s picture

FileSize
33.1 KB

view field config settings

wturrell’s picture

Review:

- Able to reproduce problem if Number (decimal) = 0.00, but not for NULL (i.e. "Hide if empty" works for the latter)
- Added steps to reproduce to issue summary
- Fixed by patch, applies cleanly to 8.3.x
- Uploaded new patch - only modifications are some line length fixes in the test
- The code change to renderText() makes sense, don't think it needs extra comments.
- In scope.
- No UI/CSS/JS changes
- No translatable strings.

More tests?
- Should we have tests on 0.00 (or 0.0, 0) as well as 0.01 to be on the safe side?
- Likewise, do we need tests on field type float, not just decimal? (#24, #39)

The last submitted patch, 46: interdiff-1232920-41-46.patch, failed testing.

wturrell’s picture

FileSize
1.86 KB

Upload interdiff with correct extension (sorry…)

Lendude’s picture

Status: Needs review » Needs work
  1. +++ b/core/lib/Drupal/Core/Field/Plugin/Field/FieldFormatter/DecimalFormatter.php
    @@ -52,6 +52,7 @@ public function settingsForm(array $form, FormStateInterface $form_state) {
    +      '#required' => TRUE,
    

    How is this related?

  2. +++ b/core/lib/Drupal/Core/Field/Plugin/Field/FieldFormatter/DecimalFormatter.php
    --- a/core/modules/views/src/Plugin/views/field/FieldPluginBase.php
    +++ b/core/modules/views/src/Plugin/views/field/FieldPluginBase.php
    

    I don't like the fix being here at all. We are messing with Field UI decimal plugin specific settings in the Views FieldPluginBase.

    At the very least it should be in EntityField, but even that sounds bad. If we want decimal specific logic, we need a decimal specific handler I would think.

Looks like NumericField::render() just not called (full clear cache not helps)

@vaplas the decimal field uses the generic EntityField plugin, that why that never gets called.

Anonymous’s picture

@wturrell, thanks for help with it!

More tests?

More tests have sense for me too: integer, float, decimal, string ("0" should pass, "00" should not pass, because we cann't sure that this is number data), and with possitive and negative values.

Also I have few discontents to my patch. Example:

  • test is plagitom with /core/modules/views/tests/src/Kernel/Handler/FieldKernelTest.php::_testHideIfEmpty(), and it is not clear their relationship, location, messages for asserts?
  • number by round() is equials with number by number_format()?

But after #49 i think #41 patch is not actual.

@Lendude, thanks for the clarification of the situation. We already have issues about Decimal Handler:

  1. #2085361: Provide a field handler for serial columns that do not have decimal or separator formatting
  2. #2735997: Decimal separator and decimals settings ignored when aggregating decimal fields

Should we make progress one of them?
Also we have many issues about problem works with decimal/float. Hence create special handlers have big sense.

And #33 looks like #1952926: NumericField.php does not support negative value rendering in range -0.xx. :)

Lendude’s picture

Giving the decimal field a special handler would require #2337515: Allow @FieldType to customize views data I feel.

Pavan B S’s picture

shaunole’s picture

I recently ran across this issue none of the above patches seemed to work. I believe this is my first post in the forum, so hopefully, this proves useful to others after a little scrutiny from other devs here.

Within the handlers/views_handler_field_math.inc (see line 71), there is an IF statement that evaluates whether the field should be hidden and if it is empty. However, this if statement appears AFTER the numeric value is converted to a string which will not evaluate as empty with the decimal. By moving this IF block up the output formatting lines (around line 54) , it correctly evaluates as 0/empty.

Is there a logical reason that the check for empty should be performed after the string conversion? If not, simply moving that block up a bit appears to resolve this issue for me. I don't want to provide a patch that could cause others problems if my logic is flawed here, but if it is an acceptable change, I'll happily provide one.

In case it's relevant, I'm using Drupal Version: 7.54 with Views Version: 7.x-3.15.

Thanks for your help and feedback!

Shaun

morbiD’s picture

@shaunole: This issue only has a patch for numeric fields while you're referring to math fields. I already created a related issue/patch for (D7) math fields: #2741953: Empty rounded math expression fields not hidden properly

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.

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.

pameeela’s picture

Issue summary: View changes
Issue tags: +Bug Smash Initiative

Updated issue summary with more detail on the steps, took a while to figure out exactly how to reproduce since there are about a million ways to do this!

mohit_aghera made their first commit to this issue’s fork.

mohit_aghera’s picture

Status: Needs work » Needs review
  • Fixing code formatting and indentation issues.
  • Update naming convention for test case file and class.
  • Update the patch for 9.3.x
  • Fixing test case failures.

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.

quietone’s picture

Version: 9.2.x-dev » 9.4.x-dev
Issue summary: View changes
Status: Needs review » Needs work
Issue tags: +Needs issue summary update

This needs an issue summary update. As a reviewer, it really helps to have that up to date. I'll add what I can during this review.

I tested on Drupal 9.4.x, standard install. I can confirm that without the patch the field is displayed when the value is 0 and scale is 2 but not when the patch is applied.

All the points in #49 need to be addressed. Setting NW for that. Since #49.2 advises against the current solution, I am skipping reviewing the patch or test.

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

Drupal 9.4.0-alpha1 was released on May 6, 2022, which means new developments and disruptive changes should now 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.

Version: 9.5.x-dev » 10.1.x-dev

Drupal 9.5.0-beta2 and Drupal 10.0.0-beta2 were released on September 29, 2022, which means new developments and disruptive changes should now be targeted for the 10.1.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 10.1.x-dev » 11.x-dev

Drupal core is moving towards using a “main” branch. As an interim step, a new 11.x branch has been opened, as Drupal.org infrastructure cannot currently fully support a branch named main. New developments and disruptive changes should now be targeted for the 11.x branch, which currently accepts only minor-version allowed changes. For more information, see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.