My Question:
What should the correct structure be? I have tried redesign this several times but keep getting tight-coupling issues.
Relevant Info:
I am writing a new endpoint for some third party software.
It will receive a payload that I am then passing into a static method of my Subscription class.
From there I want to do a look-up of the payload related subscription, establish an instance of it, then perform an update based off the rest of my information on my class. I am running into an error that is saying the following: undefined method update for ActiveRecording::Association::HasOneAssociation
EndPointController:
class Api::V2::EndpointController < Api::V5::BaseController
def connect
data = decode(request.body.read)
Subscription.static_method(data)
end
end
Subscription Model:
class Subscription < ActiveRecord::Base
def self.static_method(data)
@subscription = Subscription.find_by_id(data.subscription_id)
@subscription.update(data)
end
end
Subscription Controller:
class SubscriptionsController < ApplicationController
def update
#execute update
end
end
Moving the code into the model was not a very good idea to begin with. It's the controller that needs to check if the update is successful and respond accordingly. Model also have no concept of context besides what you pass in and are already overburdoned with responsibilites.
I would just write this code in the controller in the most straight forward way possible.
# @todo setup the correct inflections in config/initializers/inflections.rb
# @see https://github.com/rubocop/ruby-style-guide#camelcase-classes
# @see https://github.com/rubocop/ruby-style-guide#namespace-definition
module API
module V2
# @todo Write a high level description of the what the responsibility of this class is.
# the name itself is pretty smelly.
# "Endpoint" is very vague - every controller method is an endpoint.
# Why is this inheriting from a different version of the API? Very smelly.
class EndpointController < API::V5::BaseController
# @todo write a description of the purpose of this method
# @todo document the route that this method corresponds to
def connect
data = decode(request.body.read) # WTF is this?
# .find will raise an exception if the id is not valid
# Rails will catch this and return a 404.
subscription = Subscription.find(data.subscription_id)
# When you assume, you make an ass out of you and me.
# Always check if the update is successful
if subscription.update(data)
head :ok # or provide some other response
else
head :unprocessable_entity
end
end
end
end
end
If you later find that you're repeating this code more than twice then add an abstraction such as a service object.