Skip to content

Commit

Permalink
Do not try to coerce "" for numeric & date params
Browse files Browse the repository at this point in the history
When using an HTML form for searching or filtering and a field is left
blank, Rails puts an empty string in the params hash, and this raised
an unexpected InvalidParameterError when we defined the param as a
non-required Integer, Float, Date, etc.

Now we will treat that empty string as an un-supplied param and only
reject it if the param is flagged as required.

- added shared examples for Float tests
- nested shared examples in the appropriate scope
  • Loading branch information
jlw committed Jan 15, 2024
1 parent 15d3cc8 commit a4fc380
Show file tree
Hide file tree
Showing 9 changed files with 130 additions and 22 deletions.
2 changes: 2 additions & 0 deletions lib/rails_param/coercion/big_decimal_param.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@ class BigDecimalParam < VirtualParam
DEFAULT_PRECISION = 14

def coerce
return nil if param == '' # e.g. from an empty field in an HTML form

stripped_param = if param.is_a?(String)
param.delete('$,').strip.to_f
else
Expand Down
2 changes: 2 additions & 0 deletions lib/rails_param/coercion/float_param.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@ module RailsParam
class Coercion
class FloatParam < VirtualParam
def coerce
return nil if param == '' # e.g. from an empty field in an HTML form

Float(param)
end
end
Expand Down
2 changes: 2 additions & 0 deletions lib/rails_param/coercion/integer_param.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@ module RailsParam
class Coercion
class IntegerParam < VirtualParam
def coerce
return nil if param == '' # e.g. from an empty field in an HTML form

Integer(param)
end
end
Expand Down
2 changes: 2 additions & 0 deletions lib/rails_param/coercion/time_param.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@ module RailsParam
class Coercion
class TimeParam < VirtualParam
def coerce
return nil if param == '' # e.g. from an empty field in an HTML form

return type.strptime(param, options[:format]) if options[:format].present?

type.parse(param)
Expand Down
12 changes: 12 additions & 0 deletions spec/rails_param/coercion/big_decimal_param_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,12 @@
end
end

shared_examples "returns nil" do
it "returns the param as a nil value" do
expect(subject.coerce).to be nil
end
end

it_behaves_like "returns BigDecimal with default precision"

context "given a precision option" do
Expand All @@ -35,6 +41,12 @@
expect(subject.coerce).to eq 1.50
end
end

context "param is blank (e.g. empty field in an HTML form)" do
let(:param) { "" }

it_behaves_like "returns nil"
end
end
end
end
40 changes: 34 additions & 6 deletions spec/rails_param/coercion/float_param_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -6,20 +6,48 @@
let(:options) { {} }
subject { described_class.new(param: param, type: type, options: options) }

context "value is valid" do
let(:param) { "12.34" }
shared_examples "does not raise an error" do
it "does not raise an error" do
expect { subject.coerce }.to_not raise_error
end
end

shared_examples "raises ArgumentError" do
it "raises ArgumentError" do
expect { subject.coerce }.to raise_error ArgumentError
end
end

it "returns a Float" do
shared_examples "returns a Float" do
it "returns the param as a Float" do
expect(subject.coerce).to eq 12.34
end
end

shared_examples "returns nil" do
it "returns the param as a nil value" do
expect(subject.coerce).to be nil
end
end

context "value is valid" do
let(:param) { "12.34" }

it_behaves_like "does not raise an error"
it_behaves_like "returns a Float"
end

context "param is blank (e.g. empty field in an HTML form)" do
let(:param) { "" }

it_behaves_like "does not raise an error"
it_behaves_like "returns nil"
end

context "value is invalid" do
let(:param) { "foo" }

it "raises ArgumentError" do
expect { subject.coerce }.to raise_error ArgumentError
end
it_behaves_like "raises ArgumentError"
end
end
end
45 changes: 29 additions & 16 deletions spec/rails_param/coercion/integer_param_spec.rb
Original file line number Diff line number Diff line change
@@ -1,28 +1,34 @@
require 'spec_helper'

describe RailsParam::Coercion::IntegerParam do
shared_examples "does not raise an error" do
it "does not raise an error" do
expect { subject.coerce }.to_not raise_error
describe "#coerce" do
let(:type) { Integer }
let(:options) { {} }
subject { described_class.new(param: param, type: type, options: options) }

shared_examples "does not raise an error" do
it "does not raise an error" do
expect { subject.coerce }.to_not raise_error
end
end
end

shared_examples "raises ArgumentError" do
it "raises ArgumentError" do
expect { subject.coerce }.to raise_error ArgumentError
shared_examples "raises ArgumentError" do
it "raises ArgumentError" do
expect { subject.coerce }.to raise_error ArgumentError
end
end
end

shared_examples "returns an Integer" do
it "returns the param as an Integer" do
expect(subject.coerce).to eq 19
shared_examples "returns an Integer" do
it "returns the param as an Integer" do
expect(subject.coerce).to eq 19
end
end
end

describe "#coerce" do
let(:type) { Integer }
let(:options) { {} }
subject { described_class.new(param: param, type: type, options: options) }
shared_examples "returns nil" do
it "returns the param as a nil value" do
expect(subject.coerce).to be nil
end
end

context "param is a valid value" do
let(:param) { "19" }
Expand All @@ -31,6 +37,13 @@
it_behaves_like "returns an Integer"
end

context "param is blank (e.g. empty field in an HTML form)" do
let(:param) { "" }

it_behaves_like "does not raise an error"
it_behaves_like "returns nil"
end

context "param is invalid value" do
let(:param) { "notInteger" }

Expand Down
28 changes: 28 additions & 0 deletions spec/rails_param/coercion/time_param_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,12 @@
end
end

shared_examples "returns nil" do
it "returns the param as a nil value" do
expect(subject.coerce).to be nil
end
end

let(:options) { {} }
subject { described_class.new(param: param, type: type, options: options) }

Expand Down Expand Up @@ -195,5 +201,27 @@
end
end
end

context "param is blank (e.g. empty field in an HTML form)" do
let(:param) { "" }

context "type is Date" do
let(:type) { Date }

it_behaves_like "returns nil"
end

context "type is Time" do
let(:type) { Time }

it_behaves_like "returns nil"
end

context "type is DateTime" do
let(:type) { DateTime }

it_behaves_like "returns nil"
end
end
end
end
19 changes: 19 additions & 0 deletions spec/rails_param/param_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,12 @@ def params;
expect(controller.params["foo"]).to eql(42)
end

it "will allow a nil value (e.g. from an empty field in an HTML form)" do
allow(controller).to receive(:params).and_return({ "foo" => "" })
controller.param! :foo, Integer
expect(controller.params["foo"]).to be nil
end

it "will raise InvalidParameterError if the value is not valid" do
allow(controller).to receive(:params).and_return({ "foo" => "notInteger" })
expect { controller.param! :foo, Integer }.to(
Expand All @@ -151,6 +157,12 @@ def params;
expect(controller.params["foo"]).to eql(42.22)
end

it "will allow a nil value (e.g. from an empty field in an HTML form)" do
allow(controller).to receive(:params).and_return({ "foo" => "" })
controller.param! :foo, Float
expect(controller.params["foo"]).to be nil
end

it "will raise InvalidParameterError if the value is not valid" do
allow(controller).to receive(:params).and_return({ "foo" => "notFloat" })
expect { controller.param! :foo, Float }.to(
Expand Down Expand Up @@ -604,6 +616,13 @@ def params;
)
end

it "raises on a nil value (e.g. from an empty field in an HTML form)" do
allow(controller).to receive(:params).and_return({ "foo" => "" })
expect { controller.param! :foo, BigDecimal, required: true }.to(
raise_error(RailsParam::InvalidParameterError, "Parameter foo is required")
)
end

it "raises custom message" do
allow(controller).to receive(:params).and_return({})
expect { controller.param! :price, Integer, required: true, message: "No price specified" }.to(
Expand Down

0 comments on commit a4fc380

Please sign in to comment.