Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Handling Nulls in Currency Filter #477

Merged
merged 3 commits into from Apr 13, 2022
Merged

Handling Nulls in Currency Filter #477

merged 3 commits into from Apr 13, 2022

Conversation

AnushaKaraka
Copy link
Contributor

@AnushaKaraka AnushaKaraka commented Apr 7, 2022

This PR is to add a quick null check for input in the Currency filter and added an associated testcase.

Without this check a null reference exception is thrown whenever the input is null(which is a quite common scenario if we pass in a property of a model) and stopping the render of the entire template.

PS: Tried to create an issue first but couldn't for some reason. hence opened the PR directly. Please let me know if I have to follow a different process for the same. Here is my issue notes:

Dotliquid version 2.1.457

Expected behavior To render null if the input is null for a currency filter

Actual behavior It is throwing a null reference exception if I pass in a null property(which is highly possible)

Steps to reproduce the Problem (you can add files)

  1. Create a customer object with a property named Amount.
  2. Instantiate the object with a null value for Amount
  3. Use DotLiquid syntax with currency filter {{Customer.Amount | currency:'fr-CA'}}

or

Use DotLiquid syntax with currency filter {{null | currency:'fr-CA'}}

@codecov
Copy link

codecov bot commented Apr 7, 2022

Codecov Report

Merging #477 (9dc6dc5) into master (c9aff49) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##           master     #477   +/-   ##
=======================================
  Coverage   87.27%   87.28%           
=======================================
  Files          67       67           
  Lines        2633     2634    +1     
  Branches      634      635    +1     
=======================================
+ Hits         2298     2299    +1     
  Misses        212      212           
  Partials      123      123           
Impacted Files Coverage Δ
src/DotLiquid/StandardFilters.cs 93.62% <100.00%> (+0.01%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c9aff49...9dc6dc5. Read the comment docs.

Copy link
Contributor

@microalps microalps left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would like to see a test and handling for empty string once we are at it.

src/DotLiquid/StandardFilters.cs Show resolved Hide resolved
@microalps microalps merged commit 491f941 into dotliquid:master Apr 13, 2022
@AnushaKaraka AnushaKaraka deleted the Handle_Nulls_In_CurrencyFilter branch April 13, 2022 06:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants