Dynamic parameter persistence #59
Conversation
| before(:each) { expect(parameters).to be_empty } | ||
| it "returns the persisted params" do | ||
| persisted_params = connection.parameters | ||
| persisted_params.each do |key, value| |
There was a problem hiding this comment.
Use each_key instead of each.
Unused block argument - value. If it's necessary, use _ or _value as an argument name to indicate that it won't be used.
| context "empty param values" do | ||
| let(:parameters) { {"a" => nil, "b" => 3, "c" => ""} } | ||
| it "removes them" do | ||
| expect(subject.send(:request_params)).to eq({"b" => 3}) |
There was a problem hiding this comment.
Space inside { missing.
Redundant curly braces around a hash parameter.
Space inside } missing.
| expect(subject.send(:request_params)).to eq({}) | ||
| end | ||
| context "empty param values" do | ||
| let(:parameters) { {"a" => nil, "b" => 3, "c" => ""} } |
There was a problem hiding this comment.
Space inside { missing.
Space inside } missing.
| end | ||
| it 'does not update the DB if there are no request params' do | ||
| expect(parameters).to be_empty | ||
| expect{ subject.call }.to_not change{ connection.reload.parameters } |
There was a problem hiding this comment.
Parenthesize the param change{ connection.reload.parameters } to make sure that the block will be associated with the change method call.
| end | ||
|
|
||
| def request_params | ||
| context.parameters.to_h.select{|k,v| v.present?} |
There was a problem hiding this comment.
Space between { and | missing.
Unused block argument - k. If it's necessary, use _ or _k as an argument name to indicate that it won't be used.
Space missing after comma.
Space missing inside }.
davidlaprade
force-pushed
the
dynamic-param-persistence
branch
2 times, most recently
from
7162aaa to
bef1882
Compare
| connection.reload.parameters | ||
| }.from({}) | ||
| end | ||
|
|
There was a problem hiding this comment.
Extra empty line detected at block body end.
| connection.reload.parameters | ||
| }.from({}) | ||
| end | ||
|
|
There was a problem hiding this comment.
Extra empty line detected at block body end.
| connection.reload.parameters | ||
| }.from({}) | ||
| end | ||
|
|
There was a problem hiding this comment.
Extra empty line detected at block body end.
davidlaprade
force-pushed
the
dynamic-param-persistence
branch
from
bef1882 to
9a5cdfd
Compare
spec/support/compatible_http.rb
Outdated
| def compatible_http(method, path, args) | ||
| case Rails::VERSION::MAJOR | ||
| when 4 then self.send(method, path, args[:params], args[:headers]) | ||
| when 5 then self.send(method, path, args) |
spec/support/compatible_http.rb
Outdated
| # See https://github.com/nebulab/cangaroo/pull/60#issue-175032794 | ||
| def compatible_http(method, path, args) | ||
| case Rails::VERSION::MAJOR | ||
| when 4 then self.send(method, path, args[:params], args[:headers]) |
| let(:parameters) { { "different" => "param" } } | ||
| it 'does not update the DB' do | ||
| expect(connection.parameters.keys).to_not include(parameters.keys.first) | ||
| expect{ subject.call }.to_not change{ connection.reload.parameters.with_indifferent_access } |
There was a problem hiding this comment.
Parenthesize the param change{ connection.reload.parameters.with_indifferent_access } to make sure that the block will be associated with the change method call.
davidlaprade
force-pushed
the
dynamic-param-persistence
branch
from
6377df2 to
5e38aff
Compare
|
Have you guys had a chance to look at this? I've had it running in production for a couple months now without issue. Anything I can do to help keep this moving along? |
|
@bricesanchez it seems ok for me, what do you think? |
Back in the days of Wombat, some endpoints depended on being able to pass parameters back to the system, e.g. a timestamp of the most recently updated item. This was the point behind the
#add_parametermethod in spree'sEndpointBase, which most of the endpoints have as a dependency. Many of them require the#add_parametermethod to function properly, for example:to name a few.
This attempts to add this functionality to cangaroo, so that anyone using these endpoints really would find cangaroo to be backwards compatible with Wombat.