Fire hooks before validation and rename it into after_graph_persist

Author: Abderrahmane Boudi

lib/graphiti/resource.rb

@@ -112,8 +112,8 @@ def resolve(scope)
       adapter.resolve(scope)
     end
 
-    def before_validation(model, metadata)
-      hooks = self.class.config[:before_validation][metadata[:method]] || []
+    def after_graph_persist(model, metadata)
+      hooks = self.class.config[:after_graph_persist][metadata[:method]] || []
       hooks.each do |hook|
         instance_exec(model, metadata, &hook)
       end

lib/graphiti/resource/configuration.rb

@@ -186,7 +186,7 @@ def config
               sort_all: nil,
               sorts: {},
               pagination: nil,
-              before_validation: {},
+              after_graph_persist: {},
               before_commit: {},
               after_commit: {},
               attributes: {},

lib/graphiti/resource/dsl.rb

@@ -81,10 +81,10 @@ def default_filter(name = nil, &blk)
           }
         end
 
-        def before_validation(only: [:create, :update, :destroy], &blk)
+        def after_graph_persist(only: [:create, :update, :destroy], &blk)
           Array(only).each do |verb|
-            config[:before_validation][verb] ||= []
-            config[:before_validation][verb] << blk
+            config[:after_graph_persist][verb] ||= []
+            config[:after_graph_persist][verb] << blk
           end
         end
 

lib/graphiti/resource_proxy.rb

@@ -121,11 +121,10 @@ def destroy
         metadata = {method: :destroy}
         model = @resource.destroy(@query.filters[:id], metadata)
         model.instance_variable_set(:@__serializer_klass, @resource.serializer)
+        @resource.after_graph_persist(model, metadata)
         validator = ::Graphiti::Util::ValidationResponse.new \
           model, @payload
         validator.validate!
-        # FIXME: I don't know in which scenarios the following line is needed!
-        # @resource.before_validation(model, metadata)
         @resource.before_commit(model, metadata)
 
         {result: validator}
@@ -163,6 +162,7 @@ def persist
       transaction_response = @resource.transaction do
         ::Graphiti::Util::TransactionHooksRecorder.record do
           model = yield
+          ::Graphiti::Util::TransactionHooksRecorder.run_graph_persist_hooks
           validator = ::Graphiti::Util::ValidationResponse.new \
             model, @payload
           validator.validate!

lib/graphiti/util/persistence.rb

@@ -60,8 +60,8 @@ def run
 
     post_process(persisted, parents)
     post_process(persisted, children)
-    before_validation = -> { @resource.before_validation(persisted, metadata) }
-    add_hook(before_validation, :before_validation)
+    after_graph_persist = -> { @resource.after_graph_persist(persisted, metadata) }
+    add_hook(after_graph_persist, :after_graph_persist)
     before_commit = -> { @resource.before_commit(persisted, metadata) }
     add_hook(before_commit, :before_commit)
     after_commit = -> { @resource.after_commit(persisted, metadata) }

lib/graphiti/util/transaction_hooks_recorder.rb

@@ -8,7 +8,6 @@ class TransactionHooksRecorder
       #
       # ```ruby
       # TransactionHooksRecorder.record do
-      #   TransactionHooksRecorder.add(->{ do_some_stuff() }, :before_validation)
       #   TransactionHooksRecorder.add(->{ do_stuff() }, :before_commit)
       #   TransactionHooksRecorder.add(->{ do_more_stuff() }, :after_commit)
       #   {
@@ -28,7 +27,6 @@ def record
 
           begin
             result = yield
-            run(:before_validation)
             run(:before_commit)
 
             unless result.is_a?(::Hash)
@@ -43,6 +41,10 @@ def record
           end
         end
 
+        def run_graph_persist_hooks
+          run(:after_graph_persist)
+        end
+
         # Because hooks will be added from the outer edges of
         # the graph, working inwards
         def add(prc, lifecycle_event)
@@ -61,7 +63,7 @@ def _hooks
 
         def reset_hooks
           Thread.current[:_graphiti_hooks] = {
-            before_validation: [],
+            after_graph_persist: [],
             before_commit: [],
             after_commit: [],
           }

spec/integration/rails/persistence_spec.rb

@@ -276,6 +276,94 @@
       end
     end
 
+    describe "after graph persisted validation" do
+      subject(:make_request) { do_update(payload) }
+
+      let(:klass) do
+        Class.new(EmployeeResource) do
+          self.validate_endpoints = false
+
+          after_graph_persist do |model|
+            model.valid?(:after_graph_persisted)
+          end
+        end
+      end
+
+      let(:polyvalentEmployee) do
+        Class.new(Employee) do
+          def self.model_name
+            ActiveModel::Name.new(self, nil, 'PolyvalentEmployee')
+          end
+          validates :positions, length: { minimum: 2, too_short: 'too short, minimum is 2' }, on: :after_graph_persisted
+        end
+      end
+
+      let(:employee) { Employee.create!(first_name: "Jane") }
+
+      let(:payload) do
+        {
+          data: {
+            type: "employees",
+            id: employee.id.to_s,
+            relationships: {
+              positions: {
+                data: [
+                  { 'temp-id': "pos1", type: "positions", method: "create" },
+                  { 'temp-id': "pos2", type: "positions", method: "create" },
+                ],
+              },
+            },
+          },
+          included: [
+            {
+              'temp-id': "pos1",
+              type: "positions",
+              attributes: {title: "foo"},
+            },
+            {
+              'temp-id': "pos2",
+              type: "positions",
+              attributes: {title: "bar"},
+            },
+          ],
+        }
+      end
+
+      before do
+        klass.model = polyvalentEmployee
+        allow(controller).to receive(:resource) { klass }
+      end
+
+      context "when valid" do
+        it "responds with the persisted data" do
+          make_request
+          expect(jsonapi_included.count).to eq(2)
+          expect(jsonapi_included.map { |inc| inc.attributes["title"] }).to eq(["foo", "bar"])
+        end
+      end
+
+      context "when validation error" do
+        before do
+          payload[:data][:relationships][:positions][:data].pop
+        end
+
+        it "returns validation error response" do
+          make_request
+          expect(json["errors"].first).to match(
+            "code" => "unprocessable_entity",
+            "status" => "422",
+            "source" => {"pointer" => "/data/relationships/positions"},
+            "detail" => "Positions too short, minimum is 2",
+            "title" => "Validation Error",
+            "meta" => hash_including(
+              "attribute" => "positions",
+              "message" => "too short, minimum is 2"
+            )
+          )
+        end
+      end
+    end
+
     describe "non-writable foreign keys" do
       subject(:make_request) { do_update(payload) }
 

spec/integration/rails/transaction_hooks_spec.rb

@@ -1,7 +1,7 @@
 # rubocop: disable Style/GlobalVars
 
 if ENV["APPRAISAL_INITIALIZED"]
-  RSpec.describe "before_validation, before_ & after_commit hooks", type: :controller do
+  RSpec.describe "after_graph_persist, before_ & after_commit hooks", type: :controller do
     class Callbacks
       class << self
         attr_accessor :fired, :in_transaction_during, :entities
@@ -27,7 +27,7 @@ def in_transaction?
       Callbacks.fired = {}
       Callbacks.in_transaction_during = {}
       Callbacks.entities = []
-      $raise_on_before_validation = {employee: false}
+      $raise_on_after_graph_persist = {employee: false}
       $raise_on_before_commit = {employee: true}
     end
 
@@ -76,9 +76,9 @@ class EmployeeResource < ApplicationResource
 
         attribute :first_name, :string
 
-        before_validation do |employee|
-          Callbacks.add(:before_validation, employee)
-          if $raise_on_before_validation[:employee]
+        after_graph_persist do |employee|
+          Callbacks.add(:after_graph_persist, employee)
+          if $raise_on_after_graph_persist[:employee]
             raise "rollitback"
           end
         end
@@ -226,7 +226,7 @@ def json
           expect {
             post :create, params: payload
           }.to raise_error("whoops")
-          expect(Callbacks.entities).to be_empty
+          expect(Callbacks.entities).to include(:after_graph_persist)
         end
       end
 
@@ -238,7 +238,7 @@ def json
         it "fires all before and after_commit hooks" do
           post :create, params: payload
           expect(Callbacks.entities).to eq([
-            :before_validation,
+            :after_graph_persist,
             :before_create,
             :stacked_before_create,
             :employee_after_create,
@@ -247,20 +247,20 @@ def json
         end
       end
 
-      context "when an error is raised before_validation" do
+      context "when an error is raised after_graph_persist" do
         before do
-          $raise_on_before_validation = {employee: true}
+          $raise_on_after_graph_persist = {employee: true}
         end
 
         it "does not run before_commit callbacks" do
           expect_any_instance_of(Graphiti::Util::ValidationResponse)
-            .to receive(:validate!)
+            .to_not receive(:validate!)
           expect {
             post :create, params: payload
           }.to raise_error("rollitback")
           expect(Employee.count).to be_zero
           expect(Callbacks.entities.length).to eq(1)
-          expect(Callbacks.fired[:before_validation]).to be_a(Employee)
+          expect(Callbacks.fired[:after_graph_persist]).to be_a(Employee)
         end
       end
     end
@@ -307,7 +307,7 @@ def json
         post :create, params: payload
 
         expect(Callbacks.entities).to eq([
-          :before_validation,
+          :after_graph_persist,
           :before_create,
           :stacked_before_create,
           :before_position,
@@ -337,7 +337,7 @@ def json
         expect(Callbacks.fired[:employee_after_create_eval_test]).to be_a(IntegrationHooks::EmployeeResource)
       end
 
-      it "can access children resources from before_validation" do
+      it "can access children resources from after_graph_persist" do
         post :create, params: payload
         expect(Callbacks.fired[:employee_after_create].positions.length).to eq(1)
         expect(Callbacks.fired[:employee_after_create].positions[0]).to be_a(Position)