BenMorganIO / solidus_json_api

A JSON API for Solidus.
https://benmorganio.github.io/solidus_json_api/
BSD 3-Clause "New" or "Revised" License
21 stars 8 forks source link

Remove usage of the BaseController #4

Closed BenMorganIO closed 8 years ago

BenMorganIO commented 9 years ago

From the Slack chats, it looks like there's a concern with inheriting the Spree::Api::V2::BaseController from Spree::Api::BaseController. See: https://github.com/spree-contrib/spree_api_v2/blob/8dc3da5ae3f77188bef6479a61a092f19a83011e/app/controllers/spree/api/v2/base_controller.rb#L4

The reasoning was to be able to steal little helper methods such as the #api_key method.

We could move some of these to a concern, make a Spree::Api::V1::BaseController, or maybe just :scissors: & paste the code.

hhff commented 9 years ago

What's the actual concern?

braidn commented 8 years ago

definitely thumbs up for the concern, but why throw this into a V1::BaseController? Just for clarity and backward compatibilities sake?

BenMorganIO commented 8 years ago

@hhff there's some code in the Spree::Api::BaseController that's V1 specific. I'm really thinking of making a Spree::Api::V1::BaseController and moving everything there in Spree. This way a Spree::Api::BaseController hands out some useful helper methods to both V1 and V2.

BUT, would these helper methods be used in a V3? Maybe, maybe not. Also, and more importantly, if you're making your own API implementation, maybe you have your own base controller and you don't want to switch over to Sprees. Therefore, making an API controller helper would suite these needs fine.

Next steps I'm thinking:

BenMorganIO commented 8 years ago

Sine we're on Solidus now, this isn't a concern anymore. Closing...