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

Do not try to coerce "" for numeric & date params #103

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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