Adding a new client - part two

So we started building our first feature in tiny little steps. And we've not achieved very much so far - we know we can't do anything if not logged in, nor can we do anything if we're not an admin.

But today we're going to put some big foundations in place. As I mentioned earlier, I've implemented a version of this before so I have a good idea of how I think it's going to end up. However, we're going to let the specifications drive us forward - it might turn out that my ideas weren't the best (or simplest) way of building this.

Creating the organisation

Our final scenario is when our admin logs in, creates a new organisation which results in a new user-group being created and some pre-existing folders being made available, read-only, to this new organisation.

So let's get started with something simple - creating the organisation:

  context "when I am logged in as an admin" do
    it "creates the organisation" do
      user = a_user status: "admin"
      access_token = an_access_token_for user: user

      post "/api/organisations", params: {name: "My organisation"}, headers: auth_headers_for(access_token)

      expect(response).to be_created

      get "/api/organisations", headers: auth_headers_for(access_token)
      expect(response).to be_successful
      data = JSON.parse(response.body)
      expect(data.size).to eq 1
      expect(data.first["name"]).to eq "My organisation"
    end
  end

As expected, when we run this, it fails. And we don't even have an organisation model yet. So let's create one - using bin/rails g model Organisation. At the moment, it only needs a primary key, a name field (t.string :name, null: false, default: "", index: true) and I'm going to leap ahead and add in a status field (t.integer :status, null: false, default: 0, index: true). We don't strictly need it yet, but I know from experience that deleting records when you have an audit trail can cause problems - so, at the very least, we can use it to implement a "soft-delete" function.

Our organisation model looks like this:

class Organisation < ApplicationRecord
  validates :name, presence: true 
  enum status: {active: 0, inactive: -1}
end

Take note that the status field in the database has a default value of 0 and the status enum's active is represented by 0^[1] - so every organisation is active unless we specify otherwise^[2].

And our organisations controller becomes:

class Api::OrganisationsController < ApplicationController  
  def create
    raise NotAuthorised unless current_user.admin?
    render json: Organisation.create!(organisation_params), status: :created
  rescue NotAuthorised
    head :unauthorized
  end

  class NotAuthorised < StandardError
  end

  private

  def organisation_params
    params.permit(:name)
  end
end

Firstly, if we're not doing concurrent programming^[3], I like to use exceptions. Rather than using lots of nested if statements, it keeps the "happy path" and the "error path" separate. The first line checks for permissions, then we have the normal case, followed by a separate section dealing with when things go wrong.

Secondly, to create our organisation, we sanitise the incoming parameters, build a record and return a status of :created. We render the results as :json - as this is purely an API call - which uses ActiveRecord's default JSON serialisation. It's not perfect, but it does the job.

Thirdly, we define our NotAuthorised exception. This means that there's a lot going on here - it's a lot of stuff for one small controller file - and I can say for a fact that we're going to move things around later. But, for the moment, it's the simplest thing that works.

Now when we run the spec, it fails with at the next stage - when we read the organisations back via the API. Which makes sense as we have no read API for organisations.

Reading organisations

You can implement a simple index action on the organisations controller. To get started, we add the index action into our config/routes.rb file.

  namespace :api do
    resources :organisations, only: [:create, :index]
  end

But before we go any further, we'll need to go off on a slight tangent.

In my Gemfile I have included CanCanCan. This gem allows you to define a set of access rules for your various models, it adds scopes to your models to govern read access and a helper method to your controllers that trigger those rules.

The ability.rb file is the traditional way to define our CanCanCan rules. I like to add it to my user model like so:

class User < ApplicationRecord
  delegate :can?, to: :ability
  delegate :cannot?, to: :ability

  def ability
    @ability ||= User::Ability.new(self)
  end
end

We'll come back to the delegate calls later. But for now, we'll define a very basic set of permissions, in app/models/user/ability.rb like so:

class User::Ability
  include CanCan::Ability

  def initialize user
    if user.admin? 
      can :create, Organisation
      can :read, Organisation, status: "active"  
    end
  end
end

It's a bit messy, but it enough for now. With that in place, we can return to our organisations controller. We add in an index action and simplify the create call.

class Api::OrganisationsController < ApplicationController
  def index
    render json: Organisation.accessible_by(current_ability)
  end

  def create
    authorize! :create, Organisation
    render json: Organisation.create!(organisation_params), status: :created
  rescue CanCan::AccessDenied
    head :unauthorized
  end

  private

  def organisation_params
    params.permit(:name)
  end
end

Our controller is looking much simpler already^[4] and the specs all pass.

CanCanCan does the heavy lifting for us.

In our index action, we use the accessible_by scope, passing in current_ability. These are supplied by CanCanCan. current_ability builds an ability object for the current user and accessible_by adds a extra where clauses to our database queries, which filter out any organisations this user is not allowed to see. In this case, the ability class's can :read, Organisation, status: "active" rule filters on the status field. Later on, we'll add some much more sophisticated clauses into the ability class, and vary those rules based on the access levels that our user has. But, for now, remember that, whenever you read from the database, add in the accessible_by scope and you will never leak data to the outside world.

In our create action, we use the authorize! :create, Organisation call to block access. If the current user does not have the :create permission defined in the ability file, a CanCan::AccessDenied exception is raised, which we trap. This means we can drop the NotAuthorised exception from our controller.

Time to come clean

I have to admit that the code I have in front of me does not look like this. The organisations controller does not have an index action and my routes.rb only includes the :create action on the resources :organisations line.

As I mentioned before, I've implemented much of this before. And one of the things I have written is a "generic resources API" - inspired by Supabase. The code is pretty complex (as is the spec file) but it means that I do not need to write a controller per model - resulting in a smaller routes file and fewer controllers. I'll go into how this works later on (it also depends heavily on actions^[5] which we'll come to soon).

As a sneak preview, here's some (but not all) of the generic API in my routes file:

  namespace :api do
    resources :organisations, only: [:create]

	# generic API
    get ":class_name", to: "resources#index", as: :resources
    get ":class_name/:id", to: "resources#show", as: :resource
    # more generic API comes here but we'll come to that later
  end

The idea here is that you can call GET /api/organisations which Rails will route to the Api::ResourcesController#index action - and this will then use CanCanCan to return a JSON list of all the organisations you are allowed to see. Or you can call GET /api/things which will route to the same controller but return all the "things" you are allowed to see. And if you want to see get an individual resource you can call GET /api/organisations/123 or GET /api/things456 - again, you will only see the resources you're allowed to see.

User groups

The next step is to add a new clause to our specification. If we create a new organisation, we want to trigger an automation that creates a user-group specific to that organisation. At this point in time, I don't really know what a user-group looks like, but that's the entire point of this process. We're going to do the bare minimum to make the specification pass and revisit it if and when more requirements become apparent.

So let's add a new clause to the specification. Remember, this is our opportunity to define the "perfect" API - the one we wish we had when interacting with this system. It may turn out to be impossible, but at this point in time, with the wide open future in front of us, we might as well do things the best way we can.

First thing is to add the new clause - and move some of the setup code into RSpec's let statements^[6]. This means we're sharing some of the basic record creation stuff when it's needed across multiple tests.

  context "when I am logged in as an admin" do
    let(:user) { a_user status: "admin" }
    let(:access_token) { an_access_token_for user: user }

    it "creates the organisation" do
      post "/api/organisations", params: {name: "My organisation"}, headers: auth_headers_for(access_token)
      # (snip) all the stuff we saw before
    end

    it "automatically creates a user group for the new organisation" do 

    end
  end

Next, we write out the stuff we're actually testing. We're going to create an organisation, but this time, instead of testing that the organisation was created^[7], we're going to test that a user-group was created.

    it "automatically creates a user group for the new organisation" do
      post "/api/organisations", params: {name: "Megacorp"}, headers: auth_headers_for(access_token)

      get "/api/user_groups", headers: auth_headers_for(access_token)
      expect(response).to be_successful
      data = JSON.parse(response.body)
      expect(data.size).to eq 1
      expect(data.first["name"]).to eq "Megacorp staff"
    end

Obviously this is going to fail. Firstly, we don't have a user-group model, or controller. Secondly, we haven't defined this automation.

Let's start with the user-group.

Generate a new model (called UserGroup in a table user_groups). Keep it simple and make it identical to the organisation with a name and a status (active/inactive). Add clauses to the user's ability class - can :read UserGroup and can :create, UserGroup. I don't need to add in a controller, as my generic resources stuff will handle it, but if you're following along, you will just need to add the route for user_groups#index with a controller that looks much like the organisations one we defined earlier.

Next up is the automation. But, first, a quick digression.

Ensuring our code does not end up a tangled mess

The absolute simplest thing we can do to make the automations work is simply attach an after_create callback to our Organisation model. In a small application, we can do that with few repercussions. But this is going to be the heart of my clients' systems, probably for at least a decade^[8]. So I'm going to avoid the standard Rails way of doing things - for a number of reasons.

Firstly, I know that this system will have multiple automations and they will vary based upon the configuration. The health and safety company we're building rules for will work differently to the retailer we'll be looking at next. So whatever the callback does will need to have some configuration options. More importantly, it's likely that there will be cascades of automations, where automations perform actions which trigger further automations. I know, from experience, that this can create an untraceable mess, where it's impossible to debug or improve the code once a certain level of complexity is reached.

Secondly, ActiveRecord callbacks are, necessarily linked to ActiveRecord models. Which means they are intrinsically tied to the database and the database schema. As the system grows and changes, the "surface area" of each model will increase (as more and more fields and behaviours thrown together) and it will become harder to isolate individual behaviours to debug or change them. It's not inevitable that our code will become a mess but if we follow the standard Rails patterns it takes discipline in order to keep things clean^[9]. Instead, I want to make it easy to do the right thing and hard to do the wrong thing - so the code naturally stays separated and isolated. Ultimately, the goal here is, no matter how large the codebase grows, it remains easy to understand and easy to change.

Thirdly, I've just finished listening to "Clean Architecture" and "Clean Agile" by Bob Martin^[10]. There are many things within these books that I take issue with^[11] but one idea that really stuck with me was that your source code should reflect what your application does. If someone looks at your top-level "app" folder for the very first time, all they will see is that "this is a Ruby on Rails project". It provides no insight into what the project actually does and how it works. What if your top-level "app" folder actually described the functionality? With folders like create_organisation, start_shift, receive_incoming_order and publish_job. When a new developer joins your team and you ask them to fix a bug with receiving incoming orders, they know exactly where to start - it's right there in front of them in the app/receive_incoming_order folder.

Fourthly, and this is related to the above, Rails does a lot of work to autoload your code. If you learn only ever build Rails projects and never write "pure" ruby you will never have to use a require or require_relative statement. Whilst this is convenient, it hides something really important. Dependencies. Keeping track of your dependencies, and more importantly, keeping track of the direction of your dependencies, is essential for ensuring that your code remains easy to change.

Event Handlers and Locators

What we're trying to avoid is "coupling" - that is tying big chunks of code together. Coupled code is difficult to understand and difficult to change because so much disparate stuff is tied together^[12]. Or, to repeat the point above, the first bit of code depends on the second bit of code and the second bit of code depends on the first bit of code. A double-sided dependency - often hard to spot and which can lead to unexpected bugs.

How do we prevent this?

By forcing a separation between the chunks of code and making the dependencies explicit, so we can easily see when we end up tying things together that shouldn't be tied together.

How do we separate our code?

By broadcasting events and locating objects.

In our feature, we want a user-group to be created and folders made available when an organisation is created. So, when an organisation is created, we trigger an event that notifies any registered observers^[13]. Actually, ActiveRecord callbacks are an event notification mechanism. They just happen to be a very tightly coupled mechanism and we're looking for something a bit looser.

The other cause of coupling is class names. We could easily write an Organisation class that creates a UserGroup and Folder when needed. But what if, at some point in the future, it's not enough to just create a UserGroup but you also have to create several other inter-connected objects at the same time?

To deal with this, we use locators. The locator is a level of indirection - instead of referring to organisations or folders or user-groups by their class, we tell an intermediary what we are looking for and it locates the relevant class for us. Couple that with the "factory pattern" and we can build objects without a direct dependency on the classes that do the work. All we need is a working knowledge of which methods to call and which parameters to pass.

Currently, our OrganisationsController has the line render json: Organisation.create!(organisation_params), status: :created - it is tied directly to the Organisation ActiveRecord model. Which means if we want to add extra behaviours to the creation of an organisation, we either need to add them into the controller or add them into the model. Adding them to the controller means if we want to create an organisation from anywhere else^[14] we need to repeat this logic again and again - a recipe for inconsistencies and bugs. Adding them to the model then ties them to the database (because ActiveRecord does not work without some form of storage), which will make our tests slower and more brittle, plus mean we cannot use this same logic outside of this particular application^[15].

So, instead, we create a factory for creating organisations - I'm going to call it CreateOrganisation and get that to do the work. We also load it into a locator, so our controller tells the locator it wants to create an organisation, the locator returns the factory, then the controller tells the factory to do the work.

A pre-built implementation

I started writing some classes to handle all of this, then realised I had seen it before. The dry-rb collection of gems has a whole collection of prebuilt classes and modules that do exactly this (plus a whole lot more). It took me a little while to get my head around how they are structured, but I'm going to start with this.

• We'll have a locator, that is an instance of Dry::System. This will be called StandardProcedure and we'll use a Rails initialiser to fill it with the classes and factories that the application needs.
• We'll have an event broadcaster, that is responsible for notifying any observers that something has happened elsewhere in the application. This will be using Dry::Events and will be installed in the locator as events so every part of the system can access it.
• We'll implement our factories using Dry::Transaction. We've already noticed that creating an organisation is a multi-step process, involving authorisation, creating the object and then notifying interested parties. Dry::Transaction is designed to handle multi-step processes; you give it an input, it passes that input along a "pipeline" of steps, eventually resulting in your desired output, or an error condition.

The plan is to reimplement our controller so it looks something like this^[16]

class Api::OrganisationsController < ApplicationController
  def index
    render json: StandardProcedure["fetch_organisations"].for(current_user)
  end

  def create
	render json: StandardProcedure["create_organisation"].with(organisation_params.merge(user: current_user)), status: :created
  rescue StandardProcedure::Unauthorised
    head :unauthorized
  # Probably more stuff will have to go here
  end

  private

  def organisation_params
    params.permit(:name)
  end
end

Now, the controller is isolated from both the database models and our authorisation framework, CanCanCan^[17]. So if we change how the models are accessed^[18] or we decide that CanCanCan isn't working out, we just change the relevant parts and the controller is unaffected. At first glance, all this indirection looks like we've made a load of work for ourselves, but, in reality, it's very simple^[19] and the implementation work will be isolated, with very few dependencies.

This gives us flexibility and options as to how we build the thing - not just today, but for evermore.

However, this has been a lot of words just to get here, so we'll run through the real work next time.