Check out this post I wrote for Corgibytes about balancing maintenance work with new features when modernizing software. Special thanks go out to my fellow Corgis for their constructive feedback. They convinced me keep the overall idea but re-write my first draft. Plus they fixed my spelling and grammar errors that regular readers of this site are used too.
A client has an online store that is powered by an older version of Spree. I’m in the process of upgrading it and adding features to it at the same time. It’s a slow process as upgrading to newer versions of Spree, which also requires upgrading Ruby and Rails, is no easy task.
One customization the customer has is a delayed job that fires when a order is complete. The delayed job preforms some tasks that can take a while hence why they are done in a separate process after the order is completed.
Recently there was some issues with the delayed job task that forced me look how the delayed jobs where setup and managed. I found a couple things should be fixed, such as not deleting failed jobs. Not being able to find the error message for failed job was pain.
The most serious issue I discovered was in the page to view the delayed jobs, which used the Delayed Job Web gem. Access to the page was restricted by a password (good) but was only done use HTTP basic auth (bad) and had a hard coded password from a previous developer (bad).
Reviewing the Delayed Job Web documentation I found that it does support authenticating with Devise. That was good as Spree also uses Devise for authentication. After some research, trail and error I found that the following will work only allow Spree Admin’s to access the delayed job page:
# config/routes.rb
Spree::Core::Engine.routes.prepend do
authenticated :spree_user, -> spree_user { spree_user.admin? } do
mount DelayedJobWeb, at: "/delayed_job"
end
end
Notice instead of using just :user we need to use :spree_user. Not if someone tries to view the delayed job page when not logged in or as an non-admin Spree user then a 404 error is returned.
If logged in as a Spree admin then you can view the page as normal.
I struggled to create unit tests for the above. At first I just created some RSpec route tests:
# spec/routing/delayed_job_spec.rb
require 'rails_helper'
describe 'routes for delayed jobs', type: :routing do
routes { Spree::Core::Engine.routes }
context 'user not logged in' do
it 'they cannot see the route' do
expect(:get => "/delayed_job").to_not be_routable
expect(:post => "/delayed_job").to_not be_routable
end
end
context 'user logged in' do
before(:each) do
login_user
end
it 'they cannot see the route' do
expect(:get => "/delayed_job").to_not be_routable
expect(:post => "/delayed_job").to_not be_routable
end
end
context 'user logged in as admin' do
before(:each) do
login_admin
end
it 'they can see the route' do
expect(:get => "/delayed_job").to be_routable
expect(:post => "/delayed_job").to be_routable
end
end
end
Unfortunately that failed with an error:
NoMethodError: undefined method 'authenticate?' for nil:NilClass
Turns out this is a known issue with Rails 3 and Devise as outlined here. So instead I created integration tests for the delayed job security using Cucumber.
# features/delayed_job.feature
@javascript
Feature: Delayed Job
@allow-rescue
Scenario: I can't view Delayed Job page if I'm not logged in
When I visit the delayed job page
Then I get a 404 error
@allow-rescue
Scenario: I can't view Delayed Job page if I'm not an admin
Given I am logged in as a user
When I visit the delayed job page
Then I get a 404 error
Scenario: I can view Delayed Job page if I'm logged in as admin
Given I am logged in as an administrator
When I visit the delayed job page
Then I can see the delayed job page
# features/step_definitions/delayed_job_steps.rb
When(/^I visit the delayed job page$/) do
visit "/delayed_job"
end
Then(/^I get a 404 error$/) do
expect(page).to have_text('Routing Error No route matches [GET] "/delayed_job"')
end
Then(/^I can see the delayed job page$/) do
expect(page).to have_text('The list below shows an overview of the jobs in the delayed_job queue')
end
While not unit tests having integration tests is better then nothing.
As I continue to upgrade the client’s Spree store I’ll eventually replace the unsupported delayed_job gem with something that is. Perhaps Active Job or Sidekiq.
P.S. – One of the first songs that come up when I searched for songs about jobs. Never heard it before but it made me chuckle.
Take this job and shove it I ain’t workin’ here no more My woman done left and took all the reasons I was working for Ya better not try to stand in my way As I’m walkin’, out the door Take this job and shove it I ain’t workin’ here no more
The subtitle for the book is “Underdogs, Misfits, and the Art of Battling Giants” and as you would expect it has many examples of an underdog beating a giant. Often the underdogs have to use unconventional approaches as facing the giant head one would result in their defeat.
An item that almost became my takeaway is that sometimes the underdog is not really the underdog. Instead sometimes the underdog is more powerful then they appear. A good example of this is the title story of David and Goliath as examined by Gladwell. He argues that David was armed with a superior weapon, the sling. The sling with it’s much longer range and stopping power then the sword wielded by Goliath. Basically David brought a gun to a knife fight and unsurprisingly won.
My actual takeaway from this book is to watch for the inverted U curve.
If you are at one end of the curve, say the right side, the decreasing whatever the amount is by a little gets you better results. The problem is if you decrease the amount too much you actually get just as bad results as if you had too much.
The example in David and Goliath is classroom size. Large class sizes, say greater then 30, resulted in poor grades (results) and when the class size was reduced the students grades got better. The problem is if you make the class size too small, say less then 18, the students grades are just a worse if the class is too large. What you ideally want is the optimal class size of somewhere around 18 students. Enough for students to group and and interact but not too many to overwhelm the teacher.
My takeaway is to keep an eye out for the inverted U curve in my own life. Pay attention when I’ve reached the optimal amount of work, play, exercise, vegetables, etc. Also for my professional life. Like the optimal amount of blog posts per month?
The older I get the more I appreciate code linters. Something that can detect and often correct my formatting errors? Great! One less thing I have to worry about. Then I can spend my time on more important tasks such fixing the bug, adding a new feature, or uniting/conquering the world.
The default linter for Ruby is Rubocop. It works great but I find it’s default rules to restrictive. You can change the defaults but that is time consuming and confusing. At least confusing to me. While searching for a Rubocop config that I liked I came across Standard.
Standard is a wrapper for Rubocop but had defaults that like. It’s goal is to remove thinking about linting. Just install it and it works. No configuration setup and reasonable defaults. Great! Looks like everything I wanted.
Well, almost everything. Standard did not have a way to generate a Todo file. A file that lists all the errors in an existing project that we want to ignore until we get a chance to fix them.
Why would you want this? Well if you are working with Corgibytes you end up working on legacy projects. Projects with poorly written code with little to no tests and no automated build.
The first thing we do when inheriting a legacy code base is to baseline it. Take note of code coverage, which automated tests are failing, known bugs, and of course what linting errors exist. Once we have the baseline numbers we make sure any changes only improve the code, not make things worse. For example, we always want the code coverage number to stay the same or go up, it should never go down. For linting errors the number should always be decreasing.
Now Standard 0.4.0 has a way to generate a baseline in the form of a Todo file. Which means you can now incorporate Standard into the you build procedure. For example if you run Standard on my old website it will spit out lots of errors:
To create the baseline for the linter generate the Todo file run Standard with the following command:
standardrb --generate-todo
This will generate a .standard_todo.yml file that contains a list of all the files with errors in them. For my old website there are lots of errors.
Now when we run Standard we don’t get any errors. That said we do still get a nice message reminding us to remove files from the Todo file.
So go ahead and try it out. If you have any feedback please let me know by opening an issue. Special thank to TestDouble and Searls for creating Standard and working with me on the pull request.
P.S. – Below song is not related to the post but the current state of Alberta. Rough couple of months for many due to the economic fallout from the virus shutdown and the price of oil. That I, like many, are missing travelling. Who would have thought I would miss driving?
Hurtin’ albertan with nothing more to lose Too much oil money, not enough booze East of the rockies and west of the rest Do my best to do my damnedest and that’s just about all I guess
With the current virus issues I forgot to publicize that my presentation about creating pleasant development environments was posted along with an example of Dockerizing an existing Rails application. You can find the slides for the talk here. Finally you can find a ready to use existing Rails template that uses Docker here.
Thanks to YEGRB for allowing me to present and to Will for his awesome editing skills. Also thank you to Corgibytes for feedback on draft presentation.
I appreciate people posting getting started examples and templates online. They are good for getting started and playing with a new technology. The problem is the templates are usually not production ready. They are also missing a bunch of the developer tools and best practices, such as linters and automated builds, that every project will eventually need.
So I created a Rails 6 template that includes many of the tools I want on a Rails project. I wanted the template to be easy to use but also contain all the tools I wanted. This includes a Docker container, a linter, a type checker, and a build scripts. Actually the template has two build scripts, one for GitHub Actions and one for GitLab CI. The Docker script will build both development and production images.
Finally the template is deployed with production settings to the new Render hosting. You can see it here.
Let me know what you think, suggest improvements, or report a bug by opening an issue. Pull requests are also accepted.
This one I actually already knew but temporarily forgot about it so got to relearn it. In MySQL, and many other databases, updating the column with an ALTER statement will remove any properties not explicitly listed.
For example, say you want to update an MySQL database to support utf8mb4. This requires updating the existing string columns to utf8mb4 which my co-worker did in a script that looked like:
ALTER TABLE #{table} CHANGE #{column_name} VARCHAR CHARACTER SET utf8mb4 COLLATE utf8mb4_unicode_ci
I reviewed the pull request and didn’t catch the error even thought I been burnt by this alter column issue in the past. I guess my scar must have healed, or at least faded enough that I forgot about it.
The problem was some columns had default values set. Since the default values were not specified when redefining the column they got lost. The script should have looked like the below which I stole from here.
SELECT
CONCAT(
COLUMN_NAME,
' @new_type',
IF(IS_NULLABLE='NO', ' NOT NULL ', ' '),
IF(COLUMN_DEFAULT IS NOT NULL, CONCAT(' DEFAULT ', QUOTE(c.COLUMN_DEFAULT), ' '), ' '),
IF(COLUMN_COMMENT IS NOT NULL AND COLUMN_COMMENT != '', CONCAT(' COMMENT ', QUOTE(c.COLUMN_COMMENT), ' '), ' '),
EXTRA
) AS s
FROM
INFORMATION_SCHEMA.COLUMNS c
WHERE
TABLE_SCHEMA=#{database}
AND
TABLE_NAME=#{table}
Luckily we had a bunch of automated tests that caught the problem. Actually, we didn’t catch it right away. There was another issue where our automated CI tests where running the tests against the database without the utf8mb4 migrations. Fortunately we noticed the automated CI test issue, and the missing default values, just before doing the production release.
Lesson learned? Luck comes to those who are prepared (i.e. have automated tests) and who pair when doing complicated risky releases.
P.S. – No lyrical reason for picking this song. It just has the word lucky in it and it’s catchy. I had trouble finding a song about plain old luck and not lucky in love/lust.
