Allow unwritable PK when associating on create

Imagine updating a `Position`, changing the `Employee` it is associated
with. Since we're justassociating, we don't need `Employee`s PK to be
writable. But because our internals end up saving the `Employee`
regardless - we just walk the tree, and `id` is part of the attributes
considered - so we'd normally validate the PK, even though we really
don't need to.

We already avoided validating the PK when *updating* the `Position`,
this PR also considers *creating* the `Position` and associating to a
pre-existing `Employee`. We do this by looking at the `method` of
each sidepost, instad of the overall action being called.

Author: Lee Richmond

lib/graphiti/request_validators/validator.rb

@@ -25,7 +25,7 @@ def validate
             end
           end
 
-          typecast_attributes(resource, deserialized_payload.attributes, deserialized_payload.meta[:payload_path])
+          typecast_attributes(resource, deserialized_payload.attributes, @action, deserialized_payload.meta[:payload_path])
           process_relationships(resource, deserialized_payload.relationships, deserialized_payload.meta[:payload_path])
         else
           errors.add(:"data.type", :missing)
@@ -65,15 +65,20 @@ def process_relationships(resource, relationships, payload_path)
             next
           end
 
-          typecast_attributes(x[:resource], x[:attributes], x[:meta][:payload_path])
-          process_relationships(x[:resource], x[:relationships], x[:meta][:payload_path])
+          resource = x[:resource]
+          attributes = x[:attributes]
+          relationships = x[:relationships]
+          payload_path = x[:meta][:payload_path]
+          action = x[:meta][:method]
+          typecast_attributes(resource, attributes, action, payload_path)
+          process_relationships(resource, relationships, payload_path)
         end
       end
 
-      def typecast_attributes(resource, attributes, payload_path)
+      def typecast_attributes(resource, attributes, action, payload_path)
         attributes.each_pair do |key, value|
           # Only validate id if create action, otherwise it's only used for lookup
-          next if @action != :create &&
+          next if action != :create &&
             key == :id &&
             resource.class.config[:attributes][:id][:writable] == false
 

spec/integration/rails/persistence_spec.rb

@@ -404,8 +404,6 @@ def self.model_name
     end
 
     describe "non-writable foreign keys" do
-      subject(:make_request) { do_update(payload) }
-
       context "when belongs_to" do
         let(:klass) do
           Class.new(EmployeeResource) do
@@ -414,14 +412,16 @@ def self.model_name
           end
         end
 
-        let(:employee) { Employee.create!(first_name: "Jane") }
-        let(:classification) { Classification.create!(description: "foo") }
+        let!(:employee) { Employee.create!(first_name: "Jane") }
+        let!(:classification) { Classification.create!(description: "foo") }
 
         let(:payload) do
           {
             data: {
               type: "employees",
-              id: employee.id.to_s,
+              attributes: {
+                first_name: "foo"
+              },
               relationships: {
                 classification: {
                   data: {
@@ -436,12 +436,39 @@ def self.model_name
 
         before do
           allow(controller).to receive(:resource) { klass }
+
+          classification_resource = Class.new(ClassificationResource) do
+            def self.name
+              "ClassificationResource"
+            end
+            attribute :id, :integer_id, writable: false
+          end
+          klass.belongs_to :classification, resource: classification_resource
         end
 
-        it "does not require the FK to be a writable attribute" do
-          make_request
-          employee = Employee.last
-          expect(employee.classification_id).to eq(classification.id)
+        context "and overall action is create" do
+          subject(:make_request) { do_create(payload) }
+
+          it "does not require the FK to be a writable attribute" do
+            expect {
+              make_request
+            }.to change { Employee.count }.by(1)
+            employee = Employee.last
+            expect(employee.classification_id).to eq(classification.id)
+          end
+        end
+
+        context "and overall action is update" do
+          subject(:make_request) { do_update(payload) }
+
+          before do
+            payload[:data][:id] = employee.id.to_s
+          end
+
+          it "does not require the FK to be a writable attribute" do
+            make_request
+            expect(employee.reload.classification_id).to eq(classification.id)
+          end
         end
       end
 
@@ -465,7 +492,9 @@ def self.model_name
           {
             data: {
               type: "employees",
-              id: employee.id.to_s,
+              attributes: {
+                first_name: "foo"
+              },
               relationships: {
                 positions: {
                   data: [{
@@ -491,10 +520,29 @@ def self.model_name
           allow(controller).to receive(:resource) { klass }
         end
 
-        it "does not require the FK to be a writable attribute" do
-          make_request
-          employee = Employee.last
-          expect(employee.positions.map(&:title)).to eq(["foo"])
+        context "and the overall request is create" do
+          subject(:make_request) { do_create(payload) }
+
+          it "does not require the FK to be a writable attribute" do
+            expect {
+              make_request
+            }.to change { Employee.count }.by(1)
+            employee = Employee.last
+            expect(employee.positions.map(&:title)).to eq(["foo"])
+          end
+        end
+
+        context "and the overall request is update" do
+          subject(:make_request) { do_update(payload) }
+
+          before do
+            payload[:data][:id] = employee.id.to_s
+          end
+
+          it "does not require the FK to be a writable attribute" do
+            make_request
+            expect(employee.reload.positions.map(&:title)).to eq(["foo"])
+          end
         end
       end
     end