From 7680c88adf7d5161908d630a51a83cde603e5f6d Mon Sep 17 00:00:00 2001
From: Robert
Date: Fri, 28 Jul 2017 17:52:17 +0200
Subject: Refs: #4176@1h; Specing Referential Creation on NetexImport API call

---
 spec/requests/api/v1/netex_import_spec.rb | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/spec/requests/api/v1/netex_import_spec.rb b/spec/requests/api/v1/netex_import_spec.rb
index ab1e7f6ae..e92901d83 100644
--- a/spec/requests/api/v1/netex_import_spec.rb
+++ b/spec/requests/api/v1/netex_import_spec.rb
@@ -32,8 +32,9 @@ RSpec.describe "NetexImport", type: :request do
       let( :authorization ){ authorization_token_header( get_api_key.token ) }
 
 
-      it 'succeeds' do
-        post_request.(netex_import: legal_attributes)
+      it 'succeeds', :wip do
+        legal_attributes # force object creation for correct to change behavior
+        expect{post_request.(netex_import: legal_attributes)}.to change{Referential.count}.by(1)
         expect( response ).to be_success
         expect( json_response_body ).to eq({'id' => NetexImport.last.id, 'type' => 'NetexImport'})
       end
@@ -47,7 +48,8 @@ RSpec.describe "NetexImport", type: :request do
       let( :authorization ){ authorization_token_header( "#{referential.id}-incorrect_token") }
 
       it 'does not succeed' do
-        post_request.(netex_import: legal_attributes)
+        legal_attributes # force object creation for correct to change behavior
+        expect{ post_request.(netex_import: legal_attributes) }.not_to change{Referential.count}
         expect( response.status ).to eq(401)
       end
 
-- 
cgit v1.2.3


From 0b198fa8dece545f7cfd282d9a10d0638b0491b4 Mon Sep 17 00:00:00 2001
From: Robert
Date: Fri, 28 Jul 2017 18:21:48 +0200
Subject: Refs: #4176@2h; Request Spec for NetexImport adapted and implemented

- Specing that, in case of a correct request  Referential & NetexImport instances are created and a 201 is returned
- Specing that, in case of an incorrect request,  no instance is created and a 406 with a correct error message is returned

- Implemented

MISSING:

Refactor controller implmentation
---
 app/controllers/api/v1/netex_imports_controller.rb | 15 +++++--
 spec/requests/api/v1/netex_import_spec.rb          | 52 ++++++++++++++++------
 2 files changed, 50 insertions(+), 17 deletions(-)

diff --git a/app/controllers/api/v1/netex_imports_controller.rb b/app/controllers/api/v1/netex_imports_controller.rb
index d67d121c0..e9b8242bd 100644
--- a/app/controllers/api/v1/netex_imports_controller.rb
+++ b/app/controllers/api/v1/netex_imports_controller.rb
@@ -5,9 +5,18 @@ module Api
       def create
         respond_to do | format |
           format.json do 
-            @import = NetexImport.create(netex_import_params)
-            unless @import.valid?
-              render json: {errors: @import.errors}, status: 406
+            workbench = Workbench.where(id: netex_import_params['workbench_id']).first
+            if workbench
+              @referential = Referential.new(name: netex_import_params['name'], organisation_id: workbench.organisation_id, workbench_id: workbench.id)
+              @import = NetexImport.new(netex_import_params)
+              if @import.valid? && @referential.valid?
+                @import.save!
+                @referential.save!
+              else
+                render json: {errors: @import.errors.to_h.merge( @referential.errors.to_h )}, status: 406
+              end
+            else
+              render json: {errors: {'workbench_id' => 'missing'}}, status: 406
             end
           end
         end
diff --git a/spec/requests/api/v1/netex_import_spec.rb b/spec/requests/api/v1/netex_import_spec.rb
index e92901d83..da42f8e19 100644
--- a/spec/requests/api/v1/netex_import_spec.rb
+++ b/spec/requests/api/v1/netex_import_spec.rb
@@ -3,6 +3,8 @@ RSpec.describe "NetexImport", type: :request do
   describe 'POST netex_imports' do
 
     let( :referential ){ create :referential }
+    let( :workbench ){ referential.workbench }
+
 
     let( :file_path ){ fixtures_path 'single_reference_import.zip' }
     let( :file ){ fixture_file_upload( file_path ) }
@@ -20,21 +22,16 @@ RSpec.describe "NetexImport", type: :request do
         name: 'hello world',
         file: file,
         referential_id: referential.id,
-        workbench_id:   referential.workbench_id
+        workbench_id:   workbench.id
       }
     end 
 
-    let( :illegal_attributes ) do
-      { referential_id: referential.id }
-    end 
 
     context 'with correct credentials and correct request' do
       let( :authorization ){ authorization_token_header( get_api_key.token ) }
 
-
-      it 'succeeds', :wip do
-        legal_attributes # force object creation for correct to change behavior
-        expect{post_request.(netex_import: legal_attributes)}.to change{Referential.count}.by(1)
+      it 'succeeds' do
+        post_request.(netex_import: legal_attributes)
         expect( response ).to be_success
         expect( json_response_body ).to eq({'id' => NetexImport.last.id, 'type' => 'NetexImport'})
       end
@@ -42,6 +39,15 @@ RSpec.describe "NetexImport", type: :request do
       it 'creates a NetexImport object in the DB' do
         expect{ post_request.(netex_import: legal_attributes) }.to change{NetexImport.count}.by(1)
       end
+
+      it 'creates a correct Referential' do
+        legal_attributes # force object creation for correct to change behavior
+        expect{post_request.(netex_import: legal_attributes)}.to change{Referential.count}.by(1)
+        Referential.last.tap do | ref |
+          expect( ref.workbench_id ).to eq(workbench.id)
+          expect( ref.organisation_id ).to eq(workbench.organisation_id)
+        end
+      end
     end
 
     context 'with incorrect credentials and correct request' do
@@ -61,14 +67,32 @@ RSpec.describe "NetexImport", type: :request do
     context 'with correct credentials and incorrect request' do
       let( :authorization ){ authorization_token_header( get_api_key.token ) }
 
-      it 'does not succeed' do
-        post_request.(netex_import: illegal_attributes)
-        expect( response.status ).to eq(406)
-        expect( json_response_body['errors']['file'] ).not_to be_empty
+      shared_examples_for 'illegal attributes' do |bad_attribute, illegal_value=nil|
+        context "missing #{bad_attribute}" do
+          let!( :illegal_attributes ){ legal_attributes.merge( bad_attribute => illegal_value ) }
+          it 'does not succeed' do
+            post_request.(netex_import: illegal_attributes)
+            expect( response.status ).to eq(406)
+            expect( json_response_body['errors'][bad_attribute.to_s] ).not_to be_empty
+          end
+
+          it 'does not create an Import object' do
+            expect{ post_request.(netex_import: illegal_attributes) }.not_to change{Import.count}
+          end
+
+          it 'does not create a new Referential' do
+            expect{ post_request.(netex_import: illegal_attributes) }.not_to change{Referential.count}
+          end
+        end
       end
 
-      it 'does not create an Import object' do
-        expect{ post_request.(netex_import: illegal_attributes) }.not_to change{Import.count}
+      it_behaves_like 'illegal attributes', :file
+      it_behaves_like 'illegal attributes', :workbench_id
+      context 'name already taken' do
+        before do
+          create :referential, name: 'already taken'
+        end
+        it_behaves_like 'illegal attributes', :name, 'already taken'
       end
     end
   end
-- 
cgit v1.2.3


From 9387e31a76d9bae4cdb94622081246eea6d6c8d7 Mon Sep 17 00:00:00 2001
From: Robert
Date: Tue, 1 Aug 2017 10:49:35 +0200
Subject: Refs: #4176@3h; NetexImportController refactored with help of
 ControlFlow

---
 app/controllers/api/v1/netex_imports_controller.rb | 52 +++++++++++++++-------
 app/controllers/application_controller.rb          |  1 +
 app/controllers/concerns/control_flow.rb           | 14 ++++++
 app/views/api/v1/netex_imports/create.json.rabl    |  4 +-
 spec/requests/api/v1/netex_import_spec.rb          | 25 ++++++-----
 5 files changed, 66 insertions(+), 30 deletions(-)
 create mode 100644 app/controllers/concerns/control_flow.rb

diff --git a/app/controllers/api/v1/netex_imports_controller.rb b/app/controllers/api/v1/netex_imports_controller.rb
index e9b8242bd..17eec2ef8 100644
--- a/app/controllers/api/v1/netex_imports_controller.rb
+++ b/app/controllers/api/v1/netex_imports_controller.rb
@@ -1,34 +1,54 @@
 module Api
   module V1
     class NetexImportsController < ChouetteController
+      include ControlFlow
 
       def create
         respond_to do | format |
-          format.json do 
-            workbench = Workbench.where(id: netex_import_params['workbench_id']).first
-            if workbench
-              @referential = Referential.new(name: netex_import_params['name'], organisation_id: workbench.organisation_id, workbench_id: workbench.id)
-              @import = NetexImport.new(netex_import_params)
-              if @import.valid? && @referential.valid?
-                @import.save!
-                @referential.save!
-              else
-                render json: {errors: @import.errors.to_h.merge( @referential.errors.to_h )}, status: 406
-              end
-            else
-              render json: {errors: {'workbench_id' => 'missing'}}, status: 406
-            end
-          end
+          format.json(&method(:create_models))
         end
       end
 
 
       private
 
+      def find_workbench
+        @workbench = Workbench.find(netex_import_params['workbench_id'])
+      rescue ActiveRecord::RecordNotFound
+        render json: {errors: {'workbench_id' => 'missing'}}, status: 406
+        finish_action!
+      end
+
+      def create_models
+        find_workbench
+        create_referential
+        create_netex_import
+      end
+
+      def create_netex_import
+        @netex_import = NetexImport.new(netex_import_params.merge(referential_id: @new_referential.id))
+        @netex_import.save!
+      rescue ActiveRecord::RecordInvalid 
+        render json: {errors: @netex_import.errors}, status: 406
+        finish_action!
+      end
+
+      def create_referential
+        @new_referential =
+          Referential.new(
+            name: netex_import_params['name'],
+            organisation_id: @workbench.organisation_id,
+            workbench_id: @workbench.id)
+        @new_referential.save!
+      rescue ActiveRecord::RecordInvalid
+        render json: {errors: @new_referential.errors}, status: 406
+        finish_action!
+      end
+
       def netex_import_params
         params
           .require('netex_import')
-          .permit(:file, :name, :referential_id, :workbench_id)
+          .permit(:file, :name, :workbench_id)
       end
     end
   end
diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb
index 8fcaa3b1b..d15aa336d 100644
--- a/app/controllers/application_controller.rb
+++ b/app/controllers/application_controller.rb
@@ -37,6 +37,7 @@ class ApplicationController < ActionController::Base
     current_organisation
   end
 
+
   # Overwriting the sign_out redirect path method
   def after_sign_out_path_for(resource_or_scope)
     new_user_session_path
diff --git a/app/controllers/concerns/control_flow.rb b/app/controllers/concerns/control_flow.rb
new file mode 100644
index 000000000..0f41a1cda
--- /dev/null
+++ b/app/controllers/concerns/control_flow.rb
@@ -0,0 +1,14 @@
+module ControlFlow
+  FinishAction = Class.new RuntimeError
+
+  def self.included into
+    into.rescue_from FinishAction, with: :catch_finish_action
+  end
+
+  # Allow to exit locally inside an action after rendering (especially in error cases)
+  def catch_finish_action; end
+
+  def finish_action! msg = 'finish action'
+    raise FinishAction, msg
+  end
+end
diff --git a/app/views/api/v1/netex_imports/create.json.rabl b/app/views/api/v1/netex_imports/create.json.rabl
index 1361cdb80..f37703349 100644
--- a/app/views/api/v1/netex_imports/create.json.rabl
+++ b/app/views/api/v1/netex_imports/create.json.rabl
@@ -1,3 +1,3 @@
 
-object @import
-attributes :id, :type
+object @netex_import
+attributes :id, :workbench_id, :referential_id
diff --git a/spec/requests/api/v1/netex_import_spec.rb b/spec/requests/api/v1/netex_import_spec.rb
index da42f8e19..fd5f6d497 100644
--- a/spec/requests/api/v1/netex_import_spec.rb
+++ b/spec/requests/api/v1/netex_import_spec.rb
@@ -11,7 +11,7 @@ RSpec.describe "NetexImport", type: :request do
 
     let( :post_request ) do
       -> (attributes) do
-        post "/api/v1/netex_imports.json",
+        post api_v1_netex_imports_path(format: :json),
           attributes,
           authorization
       end
@@ -21,8 +21,7 @@ RSpec.describe "NetexImport", type: :request do
       {
         name: 'hello world',
         file: file,
-        referential_id: referential.id,
-        workbench_id:   workbench.id
+        workbench_id: workbench.id
       }
     end 
 
@@ -33,7 +32,11 @@ RSpec.describe "NetexImport", type: :request do
       it 'succeeds' do
         post_request.(netex_import: legal_attributes)
         expect( response ).to be_success
-        expect( json_response_body ).to eq({'id' => NetexImport.last.id, 'type' => 'NetexImport'})
+        expect( json_response_body ).to eq(
+          'id'             => NetexImport.last.id,
+          'referential_id' => Referential.last.id,
+          'workbench_id'   => workbench.id
+        )
       end
 
       it 'creates a NetexImport object in the DB' do
@@ -50,24 +53,22 @@ RSpec.describe "NetexImport", type: :request do
       end
     end
 
+
     context 'with incorrect credentials and correct request' do
       let( :authorization ){ authorization_token_header( "#{referential.id}-incorrect_token") }
 
-      it 'does not succeed' do
+      it 'does not create any DB object and does not succeed' do
         legal_attributes # force object creation for correct to change behavior
         expect{ post_request.(netex_import: legal_attributes) }.not_to change{Referential.count}
         expect( response.status ).to eq(401)
       end
 
-      it 'does not create an Import object' do
-        expect{ post_request.(netex_import: legal_attributes) }.not_to change{Import.count}
-      end
     end
 
     context 'with correct credentials and incorrect request' do
       let( :authorization ){ authorization_token_header( get_api_key.token ) }
 
-      shared_examples_for 'illegal attributes' do |bad_attribute, illegal_value=nil|
+      shared_examples_for 'illegal attributes' do |bad_attribute, illegal_value=nil, referential_count: 0|
         context "missing #{bad_attribute}" do
           let!( :illegal_attributes ){ legal_attributes.merge( bad_attribute => illegal_value ) }
           it 'does not succeed' do
@@ -80,13 +81,13 @@ RSpec.describe "NetexImport", type: :request do
             expect{ post_request.(netex_import: illegal_attributes) }.not_to change{Import.count}
           end
 
-          it 'does not create a new Referential' do
-            expect{ post_request.(netex_import: illegal_attributes) }.not_to change{Referential.count}
+          it 'might create a referential' do
+            expect{ post_request.(netex_import: illegal_attributes) }.to change{Referential.count}.by(referential_count)
           end
         end
       end
 
-      it_behaves_like 'illegal attributes', :file
+      it_behaves_like 'illegal attributes', :file, referential_count: 1
       it_behaves_like 'illegal attributes', :workbench_id
       context 'name already taken' do
         before do
-- 
cgit v1.2.3