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
Comment | File | Size | Author |
---|---|---|---|
#52 | empty_rounded_numeric-1232920-52.patch | 11.51 KB | Pavan B S |
#48 | interdiff-1232920-41-46.txt | 1.86 KB | wturrell |
#46 | empty_rounded_numeric-1232920-46.patch | 11.12 KB | wturrell |
#45 | 2017-02-13_125849.png | 33.1 KB | Anonymous (not verified) |
#41 | patch_39_1232920-41-pass.patch | 11.06 KB | Anonymous (not verified) |
Issue fork drupal-1232920
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:
Comments
Comment #1
dawehnerHere is a patch which allows to do it.
Assign to merlinofchaos for a logical review.
Comment #2
jdleonardUnfortunately this isn't working for me.
Comment #3
drupik CreditAttribution: drupik commentedIts 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:)
Comment #4
dawehner@jdleonard with which field did you tested it?
Comment #5
jdleonardI 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).
Comment #6
dawehnerThere 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.
Comment #7
jdleonardI 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.
Comment #8
Junro CreditAttribution: Junro commentedSame 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
Comment #9
Junro CreditAttribution: Junro commentedEmpty text (Provide text to display if this field returns no results.) doesn't work as well when "Round" numbers option is check.
Comment #10
jdleonardComment #11
dawehnerCan you please tell some more words what you did to test the patch?
Comment #12
Junro CreditAttribution: Junro commentedhandlers/views_handler_field.inc
I took of
and replaced by
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....
Comment #13
dawehnerWell 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.
Comment #14
sdsheridanI did a simple fix for this in views_handler_field_numeric.inc (D6 version). The problem I discovered is in this line:
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: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
Comment #15
tancThank 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.
Comment #16
chiddicks CreditAttribution: chiddicks commentedI'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.
Comment #17
Anybody#16 works great for me and since we don't have any broken tests or negative feedback, let's set this RTBC!
Comment #19
chiddicks CreditAttribution: chiddicks commented16: views-rounded-empty-values-1232920.patch queued for re-testing.
Comment #20
fonant CreditAttribution: fonant commentedPatch #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).
Comment #21
akadimi CreditAttribution: akadimi commentedPatch #16 didn't work fo me. If it can help the field is coming from a relationship to a field collection items
Comment #22
ctrlADelThe 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.
Comment #23
kristiaanvandeneyndePatch in #16 worked flawlessly for me, RTBC here as well.
Comment #24
GuyPaddock CreditAttribution: GuyPaddock commentedCuriously, 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.Comment #25
GuyPaddock CreditAttribution: GuyPaddock commentedSee #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.
Comment #26
colanWe'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.
Comment #27
AnybodyRe-uploaded #16 as of #26.
Comment #28
AnybodyComment #30
colanThanks!
Comment #33
asirjacques CreditAttribution: asirjacques at Annertech for Society of Actuaries in Ireland commentedHello,
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.
Comment #34
asirjacques CreditAttribution: asirjacques at Annertech for Society of Actuaries in Ireland commentedComment #35
dawehnerThank you for porting the patch.
We would have some tests as well to ensure this bug never appears again.
Comment #37
ChristianAdamski CreditAttribution: ChristianAdamski as a volunteer and at Wolters Kluwer commentedComment #38
joelpittet@ChristianAdamski this was at "needs work" to add test coverage to prevent this from regressing in the future.
Comment #39
Anonymous (not verified) CreditAttribution: Anonymous commented#33 not work for me. Steps for reproduce:
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 (''), becausenumber_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.
Comment #40
Anonymous (not verified) CreditAttribution: Anonymous commentedComment #41
Anonymous (not verified) CreditAttribution: Anonymous commentedComment #44
wturrell CreditAttribution: wturrell as a volunteer commentedCan 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…
Comment #45
Anonymous (not verified) CreditAttribution: Anonymous commentedComment #46
wturrell CreditAttribution: wturrell as a volunteer commentedReview:
- 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)
Comment #48
wturrell CreditAttribution: wturrell as a volunteer commentedUpload interdiff with correct extension (sorry…)
Comment #49
LendudeHow is this related?
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.
@vaplas the decimal field uses the generic EntityField plugin, that why that never gets called.
Comment #50
Anonymous (not verified) CreditAttribution: Anonymous commented@wturrell, thanks for help with it!
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:
/core/modules/views/tests/src/Kernel/Handler/FieldKernelTest.php::_testHideIfEmpty()
, and it is not clear their relationship, location, messages for asserts?round()
is equials with number bynumber_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:
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. :)
Comment #51
LendudeGiving the decimal field a special handler would require #2337515: Allow @FieldType to customize views data I feel.
Comment #52
Pavan B S CreditAttribution: Pavan B S at Valuebound commentedRerolled the patch.
Comment #53
shaunole CreditAttribution: shaunole as a volunteer commentedI 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 anIF
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 thisIF
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
Comment #54
morbiD CreditAttribution: morbiD commented@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
Comment #60
pameeela CreditAttribution: pameeela commentedUpdated 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!
Comment #63
mohit_aghera CreditAttribution: mohit_aghera at QED42 commentedComment #65
quietone CreditAttribution: quietone at PreviousNext commentedThis 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.