There are some distinct anti-patterns emerging in the code as we built features quickly, and I would like to do a focussed refactor sprint, mostly focussed on consolidating business logic and validation at the model layer of the code.
Currently, all the business logic is handled in our routes (the controller in a Model-View-Controller paradigm), which violates the principle of fat models, thin controllers - encapsulating all logic related to the data in the models layer offers distinct benefits, and I think solves a lot of anti-patterns that are emerging in the code, enumerating some here:
Different views that handle saving / updating of models each being responsible for doing data validation.
A sprawling helpers folder with a random assortment of helper functions, most of which are more appropriate as model methods, making it easier to reason about the data model they are connected to.
Related to above, logging actions related to data updates being handled in the routes, when it should be an afterSave or beforeSave method on models.
Some proposals, which I will ticket separately and link here:
Rename database folder to models. Make individual folders for each model - item, tag, etc.
Move all validation logic to model. https://github.com/osmlab/to-fix-backend/issues/238 -- essentially, the route should just call something like Item.save(postData) - have all validation and saving logic handled in the model, and the view be responsible only for catching and conveying any errors to the user.
Move query validation to the model layer: create a class method like Item.search() or so, that handles validation of query parameters and constructing the object to pass to findAll() - again the route just being responsible for passing parameters to the model, and sending data or errors back to the user.
Move logging to the model layer: when we are logging actions that change models, that behaviour should be represented at the model layer - ideally, use the beforeSave or afterSave model event to trigger logging, and all logging for a model encapsulated in a single file like models/item/logging.js.
What we should end up with is a folder structure roughly like:
- models
- item
- index.js
- db.js <-- contains the actual db definition
- validation.js <---- contains all the validation rules for `item`
- search.js <----- contains all the code to handle querying / search
- logging.js <------- contains all code to handle logging
^^ and similarly for other models
This should make a lot of the code much clearer and easier to reason about, with concerns separated. Also, this will allow us to start writing more granular unit tests at the model / data layer.
There are some distinct anti-patterns emerging in the code as we built features quickly, and I would like to do a focussed refactor sprint, mostly focussed on consolidating business logic and validation at the
model
layer of the code.Currently, all the business logic is handled in our
routes
(thecontroller
in a Model-View-Controller paradigm), which violates the principle offat models, thin controllers
- encapsulating all logic related to the data in themodels
layer offers distinct benefits, and I think solves a lot of anti-patterns that are emerging in the code, enumerating some here:helpers
folder with a random assortment of helper functions, most of which are more appropriate as model methods, making it easier to reason about the data model they are connected to.routes
, when it should be anafterSave
orbeforeSave
method on models.Some proposals, which I will ticket separately and link here:
database
folder tomodels
. Make individual folders for eachmodel
-item
,tag
, etc.route
should just call something likeItem.save(postData)
- have all validation and saving logic handled in the model, and the view be responsible only for catching and conveying any errors to the user.Item.search()
or so, that handles validation of query parameters and constructing the object to pass tofindAll()
- again theroute
just being responsible for passing parameters to the model, and sending data or errors back to the user.beforeSave
orafterSave
model event to trigger logging, and all logging for a model encapsulated in a single file likemodels/item/logging.js
.What we should end up with is a folder structure roughly like:
^^ and similarly for other models
This should make a lot of the code much clearer and easier to reason about, with concerns separated. Also, this will allow us to start writing more granular unit tests at the model / data layer.
cc @kepta @emilymdubois @coxchapman