Skip to content

Commit 98a11ad

Browse files
Seosamh Cahillrichmolj
authored andcommitted
Fix sidepost validation silently failing (#140)
* Fix sidepost validation silently failing Sidepost Child A and Child B on Parent, (method: ‘update’) If Child A is invalid and Child B is valid I get 200 and the fully patched response for all models, although Child A will not in fact be persisted. If I remove Child B (the valid one) and sidepost again I get the expected 422 and an errors hash. This fix is copied from graphiti-api/graphiti#38 * Add specs From https://github.com/graphiti-api/graphiti/blob/39913b107d6a0956ba10f5abd6d31018b6df91f4/spec/integration/rails/persistence_spec.rb#L395
1 parent 2567b45 commit 98a11ad

File tree

2 files changed

+55
-7
lines changed

2 files changed

+55
-7
lines changed

lib/jsonapi_compliable/util/validation_response.rb

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -45,26 +45,28 @@ def valid_object?(object)
4545
end
4646

4747
def all_valid?(model, deserialized_params)
48-
valid = true
49-
return false unless valid_object?(model)
48+
checks = []
49+
checks << valid_object?(model)
5050
deserialized_params.each_pair do |name, payload|
5151
if payload.is_a?(Array)
5252
related_objects = model.send(name)
53-
related_objects.each do |r|
53+
related_objects.each_with_index do |r, index|
5454
valid = valid_object?(r)
55+
checks << valid
5556

5657
if valid
57-
valid = all_valid?(r, deserialized_params[:relationships] || {})
58+
checks << all_valid?(r, payload[index][:relationships] || {})
5859
end
5960
end
6061
else
6162
related_object = model.send(name)
6263
valid = valid_object?(related_object)
64+
checks << valid
6365
if valid
64-
valid = all_valid?(related_object, deserialized_params[:relationships] || {})
66+
checks << all_valid?(related_object, payload[:relationships] || {})
6567
end
6668
end
6769
end
68-
valid
70+
checks.all? { |c| c == true }
6971
end
7072
end

spec/integration/rails/persistence_spec.rb

Lines changed: 47 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ def create
2626
errors: {
2727
employee: employee.errors,
2828
positions: employee.positions.map(&:errors),
29-
departments: employee.positions.map(&:department).map(&:errors)
29+
departments: employee.positions.map(&:department).compact.map(&:errors)
3030
}
3131
}
3232
end
@@ -509,6 +509,52 @@ def do_put(id)
509509
expect(positions[1].title).to eq('manager')
510510
expect(department.name).to eq('safety')
511511
end
512+
513+
context 'when a has_many relationship has validation error' do
514+
around do |e|
515+
begin
516+
Position.validates :title, presence: true
517+
e.run
518+
ensure
519+
Position.clear_validators!
520+
end
521+
end
522+
523+
before do
524+
payload[:included][0][:attributes].delete(:title)
525+
end
526+
527+
it 'rolls back the entire transaction' do
528+
expect {
529+
do_post
530+
}.to_not change { Employee.count+Position.count+Department.count }
531+
expect(json['errors']['positions'])
532+
.to eq([{ 'title' => ["can't be blank"] }, {}])
533+
end
534+
end
535+
536+
context 'when a belongs_to relationship has a validation error' do
537+
around do |e|
538+
begin
539+
Department.validates :name, presence: true
540+
e.run
541+
ensure
542+
Department.clear_validators!
543+
end
544+
end
545+
546+
before do
547+
payload[:included][1][:attributes].delete(:name)
548+
end
549+
550+
it 'rolls back the entire transaction' do
551+
expect {
552+
do_post
553+
}.to_not change { Employee.count+Position.count+Department.count }
554+
expect(json['errors']['departments'])
555+
.to eq([{ 'name' => ["can't be blank"] }])
556+
end
557+
end
512558

513559
context 'when associating to an existing record' do
514560
let!(:classification) { Classification.create!(description: 'senior') }

0 commit comments

Comments
 (0)