Merge pull request #4358 from solidusio/waiting-for-dev/with_model_and_cache_classes

Readd `config.cache_classes` on test env and remove `with_model` dep

Author: waiting-for-dev

Gemfile

@@ -40,7 +40,6 @@ group :backend, :frontend, :core, :api do
   gem 'rspec-activemodel-mocks', '~> 1.1', require: false
   gem 'rspec-rails', '~> 4.0.1', require: false
   gem 'simplecov', require: false
-  gem 'with_model', require: false
   gem 'rails-controller-testing', require: false
   gem 'puma', require: false
 

api/app/controllers/spree/api/resource_controller.rb

@@ -1,8 +1,17 @@
 # frozen_string_literal: true
 
+# @deprecated Inherit directly from Spree::Api::BaseController
 class Spree::Api::ResourceController < Spree::Api::BaseController
   before_action :load_resource, only: [:show, :update, :destroy]
 
+  def self.inherited(klass)
+    Spree::Deprecation.warn <<~MSG
+      Spree::Api::ResourceController is deprecated. Please, copy any logic you
+      need and inherit directly from Spree::Api::BaseController.
+    MSG
+    super
+  end
+
   def index
     collection_scope = model_class.accessible_by(current_ability)
     if params[:ids]

api/app/controllers/spree/api/users_controller.rb

@@ -1,8 +1,10 @@
 # frozen_string_literal: true
 
-class Spree::Api::UsersController < Spree::Api::ResourceController
+class Spree::Api::UsersController < Spree::Api::BaseController
+  before_action :load_resource, only: [:show, :update, :destroy]
+
   def index
-    user_scope = model_class.accessible_by(current_ability, :show)
+    user_scope = user_class.accessible_by(current_ability, :show)
     if params[:ids]
       ids = params[:ids].split(",").flatten
       @users = user_scope.where(id: ids)
@@ -14,20 +16,71 @@ def index
     respond_with(@users)
   end
 
-  private
+  def show
+    respond_with(@user)
+  end
+
+  def new
+    authorize! :new, user_class
+    respond_with(user_class.new)
+  end
+
+  def create
+    authorize! :create, user_class
+
+    @user = user_class.new(permitted_user_params)
 
-  attr_reader :user
+    if @user.save
+      respond_with(@user, status: 201, default_template: :show)
+    else
+      invalid_resource!(@user)
+    end
+  end
+
+  def update
+    authorize! :update, @user
 
-  def model_class
+    if @user.update(permitted_user_params)
+      respond_with(@user, status: 200, default_template: :show)
+    else
+      invalid_resource!(@user)
+    end
+  end
+
+  def destroy
+    authorize! :destroy, @user
+
+    destroy_result = if @user.respond_to?(:discard)
+      @user.discard
+    else
+      @user.destroy
+    end
+
+    if destroy_result
+      respond_with(@user, status: 204)
+    else
+      invalid_resource!(@user)
+    end
+  rescue ActiveRecord::DeleteRestrictionError
+    render "spree/api/errors/delete_restriction", status: 422
+  end
+
+  private
+
+  def user_class
     Spree.user_class
   end
 
-  def user_params
-    permitted_resource_params
+  def load_resource
+    @user = user_class.accessible_by(current_ability, :show).find(params[:id])
+  end
+
+  def permitted_user_params
+    params.require(:user).permit(permitted_user_attributes)
   end
 
-  def permitted_resource_attributes
-    if action_name == "create" || can?(:update_email, user)
+  def permitted_user_attributes
+    if action_name == "create" || can?(:update_email, @user)
       super | [:email]
     else
       super

api/spec/controllers/spree/api/resource_controller_spec.rb

@@ -2,190 +2,10 @@
 
 require 'spec_helper'
 
-module Spree
-  module Api
-    class WidgetsController < Spree::Api::ResourceController
-      prepend_view_path('spec/test_views')
+describe Spree::Api::ResourceController, type: :controller do
+  it "is deprecated" do
+    expect(Spree::Deprecation).to receive(:warn).with(/deprecated/)
 
-      def model_class
-        Widget
-      end
-
-      def permitted_widget_attributes
-        [:name]
-      end
-    end
-  end
-
-  describe Api::WidgetsController, type: :controller do
-    render_views
-
-    after(:all) do
-      Rails.application.reload_routes!
-    end
-
-    with_model 'Widget', scope: :all do
-      table do |widget|
-        widget.string :name
-        widget.integer :position
-        widget.timestamps null: false
-      end
-
-      model do
-        acts_as_list
-        validates :name, presence: true
-      end
-    end
-
-    before do
-      Spree::Core::Engine.routes.draw do
-        namespace :api do
-          resources :widgets
-        end
-      end
-    end
-
-    let(:user) { create(:user, :with_api_key) }
-    let(:admin_user) { create(:admin_user, :with_api_key) }
-
-    describe "#index" do
-      let!(:widget) { Widget.create!(name: "a widget") }
-
-      it "returns no widgets" do
-        get :index, params: { token: user.spree_api_key }, as: :json
-        expect(response).to be_successful
-        expect(json_response['widgets']).to be_blank
-      end
-
-      context "it has authorization to read widgets" do
-        it "returns widgets" do
-          get :index, params: { token: admin_user.spree_api_key }, as: :json
-          expect(response).to be_successful
-          expect(json_response['widgets']).to include(
-            hash_including(
-              'name' => 'a widget',
-              'position' => 1
-            )
-          )
-        end
-
-        context "specifying ids" do
-          let!(:widget2) { Widget.create!(name: "a widget") }
-
-          it "returns both widgets from comma separated string" do
-            get :index, params: { ids: [widget.id, widget2.id].join(','), token: admin_user.spree_api_key }, as: :json
-            expect(response).to be_successful
-            expect(json_response['widgets'].size).to eq 2
-          end
-
-          it "returns both widgets from multiple arguments" do
-            get :index, params: { ids: [widget.id, widget2.id], token: admin_user.spree_api_key }, as: :json
-            expect(response).to be_successful
-            expect(json_response['widgets'].size).to eq 2
-          end
-
-          it "returns one requested widgets" do
-            get :index, params: { ids: widget2.id.to_s, token: admin_user.spree_api_key }, as: :json
-            expect(response).to be_successful
-            expect(json_response['widgets'].size).to eq 1
-            expect(json_response['widgets'][0]['id']).to eq widget2.id
-          end
-
-          it "returns no widgets if empty" do
-            get :index, params: { ids: '', token: admin_user.spree_api_key }, as: :json
-            expect(response).to be_successful
-            expect(json_response['widgets']).to be_empty
-          end
-        end
-      end
-    end
-
-    describe "#show" do
-      let(:widget) { Widget.create!(name: "a widget") }
-
-      it "returns not found" do
-        get :show, params: { id: widget.to_param, token: user.spree_api_key }, as: :json
-        assert_not_found!
-      end
-
-      context "it has authorization read widgets" do
-        it "returns widget details" do
-          get :show, params: { id: widget.to_param, token: admin_user.spree_api_key }, as: :json
-          expect(response).to be_successful
-          expect(json_response['name']).to eq 'a widget'
-        end
-      end
-    end
-
-    describe "#new" do
-      it "returns unauthorized" do
-        get :new, params: { token: user.spree_api_key }, as: :json
-        expect(response).to be_unauthorized
-      end
-
-      context "it is allowed to view a new widget" do
-        it "can learn how to create a new widget" do
-          get :new, params: { token: admin_user.spree_api_key }, as: :json
-          expect(response).to be_successful
-          expect(json_response["attributes"]).to eq(['name'])
-        end
-      end
-    end
-
-    describe "#create" do
-      it "returns unauthorized" do
-        expect {
-          post :create, params: { widget: { name: "a widget" }, token: user.spree_api_key }, as: :json
-        }.not_to change(Widget, :count)
-        expect(response).to be_unauthorized
-      end
-
-      context "it is authorized to create widgets" do
-        it "can create a widget" do
-          expect {
-            post :create, params: { widget: { name: "a widget" }, token: admin_user.spree_api_key }, as: :json
-          }.to change(Widget, :count).by(1)
-          expect(response).to be_created
-          expect(json_response['name']).to eq 'a widget'
-          expect(Widget.last.name).to eq 'a widget'
-        end
-      end
-    end
-
-    describe "#update" do
-      let!(:widget) { Widget.create!(name: "a widget") }
-      it "returns unauthorized" do
-        put :update, params: { id: widget.to_param, widget: { name: "another widget" }, token: user.spree_api_key }, as: :json
-        assert_not_found!
-        expect(widget.reload.name).to eq 'a widget'
-      end
-
-      context "it is authorized to update widgets" do
-        it "can update a widget" do
-          put :update, params: { id: widget.to_param, widget: { name: "another widget" }, token: admin_user.spree_api_key }, as: :json
-          expect(response).to be_successful
-          expect(json_response['name']).to eq 'another widget'
-          expect(widget.reload.name).to eq 'another widget'
-        end
-      end
-    end
-
-    describe "#destroy" do
-      let!(:widget) { Widget.create!(name: "a widget") }
-
-      it "returns unauthorized" do
-        delete :destroy, params: { id: widget.to_param, token: user.spree_api_key }, as: :json
-        assert_not_found!
-        expect { Widget.find(widget.id) }.not_to raise_error
-      end
-
-      context "it is authorized to destroy widgets" do
-        it "hard-deletes the widget" do
-          delete :destroy, params: { id: widget.to_param, token: admin_user.spree_api_key }, as: :json
-          expect(response.status).to eq 204
-          expect { Widget.find(widget.id) }.to raise_error(ActiveRecord::RecordNotFound)
-        end
-      end
-    end
+    expect(Class.new(described_class))
   end
 end

api/spec/controllers/spree/api/soft_deletable_resource_controller_spec.rb

@@ -175,6 +175,18 @@ module Spree::Api
         expect(response.status).to eq(204)
       end
 
+      unless Spree.user_class.instance_methods.include?(:discard)
+        it "softs-deletes when user is soft-deletable" do
+          soft_deleted = false
+          Spree.user_class.define_method(:discard) { soft_deleted = true }
+          delete spree.api_user_path(user)
+          expect(response.status).to eq(204)
+          expect(soft_deleted).to be(true)
+        ensure
+          Spree.user_class.undef_method(:discard)
+        end
+      end
+
       it "cannot destroy user with orders" do
         create(:completed_order_with_totals, user: user)
         delete spree.api_user_path(user)