Do not throw not found error when resource relationship is null (#69)

* Do not throw not found error when resource relationship is null * Move tests to tricky operations * Add updates accoring to comments * Fix code style in spec * Add @jpalumickas as a contributor
venuu · May 30, 2017 · 95c9693 · 95c9693
1 parent 2f4e8b0
commit 95c9693
Show file tree

Hide file tree

Showing 6 changed files with 92 additions and 2 deletions.
diff --git a/.all-contributorsrc b/.all-contributorsrc
@@ -104,6 +104,17 @@
         "code",
         "test"
       ]
+    },
+    {
+      "login": "jpalumickas",
+      "name": "Justas Palumickas",
+      "avatar_url": "https://avatars0.githubusercontent.com/u/2738630?v=3",
+      "profile": "https://jpalumickas.com",
+      "contributions": [
+        "bug",
+        "code",
+        "test"
+      ]
     }
   ]
 }
diff --git a/README.md b/README.md
@@ -179,7 +179,7 @@ Thanks goes to these wonderful people ([emoji key](https://github.com/kentcdodds
 <!-- ALL-CONTRIBUTORS-LIST:START - Do not remove or modify this section -->
 | [<img src="https://avatars.githubusercontent.com/u/482561?v=3" width="100px;"/><br /><sub>Vesa Laakso</sub>](http://vesalaakso.com)<br />[💻](https://github.com/Venuu/jsonapi-authorization/commits?author=valscion) [📖](https://github.com/Venuu/jsonapi-authorization/commits?author=valscion) 🚇 [⚠️](https://github.com/Venuu/jsonapi-authorization/commits?author=valscion) [🐛](https://github.com/Venuu/jsonapi-authorization/issues?q=author%3Avalscion) 💬 👀 | [<img src="https://avatars.githubusercontent.com/u/562204?v=3" width="100px;"/><br /><sub>Emil Sågfors</sub>](https://github.com/lime)<br />[💻](https://github.com/Venuu/jsonapi-authorization/commits?author=lime) [📖](https://github.com/Venuu/jsonapi-authorization/commits?author=lime) 🚇 [⚠️](https://github.com/Venuu/jsonapi-authorization/commits?author=lime) [🐛](https://github.com/Venuu/jsonapi-authorization/issues?q=author%3Alime) 💬 👀 | [<img src="https://avatars.githubusercontent.com/u/1591161?v=3" width="100px;"/><br /><sub>Matthias Grundmann</sub>](https://github.com/matthias-g)<br />[💻](https://github.com/Venuu/jsonapi-authorization/commits?author=matthias-g) [📖](https://github.com/Venuu/jsonapi-authorization/commits?author=matthias-g) [⚠️](https://github.com/Venuu/jsonapi-authorization/commits?author=matthias-g) 💬 | [<img src="https://avatars.githubusercontent.com/u/1322?v=3" width="100px;"/><br /><sub>Thibaud Guillaume-Gentil</sub>](http://thibaud.gg)<br />[💻](https://github.com/Venuu/jsonapi-authorization/commits?author=thibaudgg) | [<img src="https://avatars.githubusercontent.com/u/71660?v=3" width="100px;"/><br /><sub>Daniel Schweighöfer</sub>](http://netsteward.net)<br />[💻](https://github.com/Venuu/jsonapi-authorization/commits?author=acid) | [<img src="https://avatars.githubusercontent.com/u/5076967?v=3" width="100px;"/><br /><sub>Bruno Sofiato</sub>](https://github.com/bsofiato)<br />[💻](https://github.com/Venuu/jsonapi-authorization/commits?author=bsofiato) | [<img src="https://avatars.githubusercontent.com/u/1896026?v=3" width="100px;"/><br /><sub>Adam Robertson</sub>](https://github.com/arcreative)<br />[📖](https://github.com/Venuu/jsonapi-authorization/commits?author=arcreative) |
 | :---: | :---: | :---: | :---: | :---: | :---: | :---: |
-| [<img src="https://avatars3.githubusercontent.com/u/4742306?v=3" width="100px;"/><br /><sub>Greg Fisher</sub>](https://github.com/gnfisher)<br />[💻](https://github.com/Venuu/jsonapi-authorization/commits?author=gnfisher) [⚠️](https://github.com/Venuu/jsonapi-authorization/commits?author=gnfisher) | [<img src="https://avatars3.githubusercontent.com/u/370182?v=3" width="100px;"/><br /><sub>Sam</sub>](http://samlh.com)<br />[💻](https://github.com/Venuu/jsonapi-authorization/commits?author=handlers) [⚠️](https://github.com/Venuu/jsonapi-authorization/commits?author=handlers) |
+| [<img src="https://avatars3.githubusercontent.com/u/4742306?v=3" width="100px;"/><br /><sub>Greg Fisher</sub>](https://github.com/gnfisher)<br />[💻](https://github.com/Venuu/jsonapi-authorization/commits?author=gnfisher) [⚠️](https://github.com/Venuu/jsonapi-authorization/commits?author=gnfisher) | [<img src="https://avatars3.githubusercontent.com/u/370182?v=3" width="100px;"/><br /><sub>Sam</sub>](http://samlh.com)<br />[💻](https://github.com/Venuu/jsonapi-authorization/commits?author=handlers) [⚠️](https://github.com/Venuu/jsonapi-authorization/commits?author=handlers) | [<img src="https://avatars0.githubusercontent.com/u/2738630?v=3" width="100px;"/><br /><sub>Justas Palumickas</sub>](https://jpalumickas.com)<br />[🐛](https://github.com/Venuu/jsonapi-authorization/issues?q=author%3Ajpalumickas) [💻](https://github.com/Venuu/jsonapi-authorization/commits?author=jpalumickas) [⚠️](https://github.com/Venuu/jsonapi-authorization/commits?author=jpalumickas) |
 <!-- ALL-CONTRIBUTORS-LIST:END -->
 
 This project follows the [all-contributors](https://github.com/kentcdodds/all-contributors) specification. Contributions of any kind welcome!
diff --git a/lib/jsonapi/authorization/authorizing_processor.rb b/lib/jsonapi/authorization/authorizing_processor.rb
@@ -269,6 +269,8 @@ def related_models_with_context
           data[rel_type].flat_map do |assoc_name, assoc_value|
             related_models =
               case assoc_value
+              when nil
+                nil
               when Hash # polymorphic relationship
                 resource_class = @resource_klass.resource_for(assoc_value[:type].to_s)
                 resource_class.find_by_key(assoc_value[:id], context: context)._model

diff --git a/lib/jsonapi/authorization/default_pundit_authorizer.rb b/lib/jsonapi/authorization/default_pundit_authorizer.rb
@@ -260,7 +260,11 @@ def authorize_related_records(source_record, related_records_with_context)
           when :to_many
             replace_to_many_relationship(source_record, records, relation_name)
           when :to_one
-            replace_to_one_relationship(source_record, records, relation_name)
+            if records.nil?
+              remove_to_one_relationship(source_record, relation_name)
+            else
+              replace_to_one_relationship(source_record, records, relation_name)
+            end
           end
         end
       end

diff --git a/spec/jsonapi/authorization/default_pundit_authorizer_spec.rb b/spec/jsonapi/authorization/default_pundit_authorizer_spec.rb
@@ -207,6 +207,52 @@
       end
     end
 
+    describe 'with "relation_type: :to_one" and records is nil' do
+      let(:related_records_with_context) do
+        [{
+          relation_name: :author,
+          relation_type: :to_one,
+          records: nil
+        }]
+      end
+
+      subject(:method_call) do
+        -> { authorizer.replace_fields(source_record, related_records_with_context) }
+      end
+
+      context 'authorized for remove_<type>? and authorized for update? on source record' do
+        before { stub_policy_actions(source_record, remove_author?: true, update?: true) }
+        it { is_expected.not_to raise_error }
+      end
+
+      context 'unauthorized for remove_<type>? and authorized for update? on source record' do
+        before { stub_policy_actions(source_record, remove_author?: false, update?: true) }
+        it { is_expected.to raise_error(::Pundit::NotAuthorizedError) }
+      end
+
+      context 'authorized for remove_<type>? and unauthorized for update? on source record' do
+        before { stub_policy_actions(source_record, remove_author?: true, update?: false) }
+        it { is_expected.to raise_error(::Pundit::NotAuthorizedError) }
+      end
+
+      context 'unauthorized for remove_<type>? and unauthorized for update? on source record' do
+        before { stub_policy_actions(source_record, remove_author?: false, update?: false) }
+        it { is_expected.to raise_error(::Pundit::NotAuthorizedError) }
+      end
+
+      context 'where remove_<type>? is undefined' do
+        context 'authorized for update? on source record' do
+          before { stub_policy_actions(source_record, update?: true) }
+          it { is_expected.not_to raise_error }
+        end
+
+        context 'unauthorized for update? on source record' do
+          before { stub_policy_actions(source_record, update?: false) }
+          it { is_expected.to raise_error(::Pundit::NotAuthorizedError) }
+        end
+      end
+    end
+
     describe 'with "relation_type: :to_many"' do
       let(:related_records) { Array.new(3) { Comment.new } }
       let(:related_records_with_context) do

diff --git a/spec/requests/tricky_operations_spec.rb b/spec/requests/tricky_operations_spec.rb
@@ -168,4 +168,31 @@
       it { is_expected.to be_forbidden }
     end
   end
+
+  describe 'PATCH /articles/:id (nullifying to-one relationship)' do
+    let(:article) { articles(:article_with_author) }
+    let(:json) do
+      <<-EOS.strip_heredoc
+      {
+        "data": {
+          "id": "#{article.external_id}",
+          "type": "articles",
+          "relationships": { "author": null }
+        }
+      }
+      EOS
+    end
+    let(:policy_scope) { Article.all }
+    subject(:last_response) { patch("/articles/#{article.external_id}", json) }
+
+    before do
+      allow_operation(
+        'replace_fields',
+        article,
+        [{ relation_type: :to_one, relation_name: :author, records: nil }]
+      )
+    end
+
+    it { is_expected.to be_successful }
+  end
 end