workarea-commerce / workarea-api

Adds JSON REST APIs to the Workarea Commerce Platform
https://www.workarea.com
Other
7 stars 1 forks source link

Recommendations endpoints suggestion #19

Closed GesJeremie closed 4 years ago

GesJeremie commented 4 years ago

Is your feature request related to a problem? Please describe.

Based on test/integration/workarea/api/admin/recommendation_settings_integration_test.rb:

require 'test_helper'

module Workarea
  module Api
    module Admin
      class RecommendationSettingsIntegrationTest < IntegrationTest
        include Workarea::Admin::IntegrationTest

        setup :set_product

        def set_product
          @product = create_product
        end

        def create_recommendation_settings
          Recommendation::Settings.create!(id: @product.id)
        end

        def test_shows_recommendation_settings
          recommendation_settings = create_recommendation_settings
          get admin_api.product_recommendation_settings_path(@product.id)

          result = JSON.parse(response.body)['recommendation_settings']
          assert_equal(
            recommendation_settings,
            Recommendation::Settings.new(result)
          )
        end

        def test_updates_recommendation_settings
          recommendation_settings = create_recommendation_settings

          patch admin_api.product_recommendation_settings_path(@product.id),
            params: { recommendation_settings: { product_ids: ['foo'] } }

          assert_equal(['foo'], recommendation_settings.reload.product_ids)
        end
      end
    end
  end
end

it seems that a Recommendation::Settings should be created before to hit the different endpoints. Not doing so would return a similar error to:

{
    "params": [
        "S20BRK1389"
    ],
    "problem": "Document(s) not found for class Workarea::Recommendation::Settings with id(s) S20BRK1389."
}

However we don't currently have an endpoint to do so (create a Workarea::Recommendation::Setting for the product_id given).

After some digging in the core it seems that this creation is implicit anyway app/controllers/workarea/admin/recommendations_controller.rb:

module Workarea
  module Admin
    class RecommendationsController < Admin::ApplicationController
      required_permissions :catalog

      before_action :find_product
      before_action :find_settings

      def edit
      end

      def update
        # ...
      end

      private

      def find_product
        # ...
      end

      def find_settings
        # Note that we use find_or_initialize_by
        model = Recommendation::Settings.find_or_initialize_by(id: @product.id)
        @settings = RecommendationsViewModel.new(model, view_model_options)
      end
    end
  end
end

Possible solutions to fix this weirdness

  1. We could automatically create a Workarea::Recommendation::Settings in workarea-core when a new product is created (worker, inline, ...) to always have the 1:1 relationship.
  2. We could add an endpoint in workarea-api to programmatically create the relationship.
  3. Tweak /workarea-api-4.5.3/admin/app/controllers/workarea/api/admin/recommendation_settings_controller.rb:

    module Workarea
    module Api
    module Admin
      class RecommendationSettingsController < Admin::ApplicationController
        before_action :find_recommendation_settings
    
        swagger_path '/products/{product_id}/recommendation_settings' do
          # ...
        end
    
        def show
          respond_with recommendation_settings: @recommendation_settings
        end
    
        def update
          @recommendation_settings.update_attributes!(params[:recommendation_settings])
          respond_with recommendation_settings: @recommendation_settings
        end
    
        private
    
        def find_recommendation_settings
          # Use find_or_initialize_by here
          @recommendation_settings = Recommendation::Settings.find(params[:product_id])
        end
      end
    end
    end
    end

Personally I'm rolling with the find_or_initialize_by (solution 3) for the current build I'm working on.

Cheers 🍻

bencrouse commented 4 years ago

Thanks @GesJeremie , this is a good idea. Especially without an endpoint to create one!