Response to @ScotiabankHelps

So, yesterday, I tweeted the following:

Someone manning the @ScotiabankHelps Twitter account responded:

While they would like to keep this a private, meandering conversation on Twitter, I figured I’d rather clarify my situation more publicly than that.

Firstly, I would like a spokesperson of Scotiabank to let me know how to do the following things without having to go to a branch or the home branch (those are different things, by the way) or to suffer through the endless, soul-sucking experience that is the call centre:

  1. Close an account whose balance has been $0 for 2 years now
  2. Move money between my own accounts by cheque without having a 5 business day hold on it
  3. Receive money from the same US source by cheque and not have a 20 business day hold on it
  4. Find out what my account number is.

To expand on #3, I’m a freelance web developer who has had a long-term contract with a company in the US. I know, for a fact, that 1 business day after I deposit a cheque from the company into my business account at Scotiabank, the money is withdrawn from theirs. Why am I then unable to have the full balance of that cheque available to me for the next 19 business days AKA a whole month? This is very much an anti-small-business policy.

Secondly, I would like to know why there is a distinction between a “home” branch and a regular one. Why am I unable to go to any branch of Scotiabank and do anything I want with my own accounts? Now, I don’t necessarily want to go to any branch, but if I must, why must I go to the one branch I happened to be near by when I opened an account? That is ridiculous.

So, a Scotiabank spokesperson, please feel free to answer these questions either in a comment on this post or in an e-mail to me. The address is srdjanpejic.scotiabankATgmailDOTcom. I trust you will know how to parse that. 

adailyriot: lifeoncinnabarisland: If you were touched or outraged by the Troy Davis case, don’t allow the same thing to happen to another man. This is Rodney Reed and like Troy Davis there is no physical evidence tying Rodney to the crime that he was accused of committing. In fact, there is very real evidence leading to another suspect all together. Rodney was tried by an all white jury in southern Texas and sentenced to death. Let’s show the murderers that took the life of Troy Davis that we are not going anywhere anytime soon. This is a movement, not a moment. Here is a link to the free, streaming documentary regarding Rodney and his story: http://video.google.com/videoplay?docid=-4864052717720140330 auto reblog for SIGNAL BOOST! Again, ugh
View Separately

adailyriot:

lifeoncinnabarisland:

If you were touched or outraged by the Troy Davis case, don’t allow the same thing to happen to another man. This is Rodney Reed and like Troy Davis there is no physical evidence tying Rodney to the crime that he was accused of committing. In fact, there is very real evidence leading to another suspect all together. Rodney was tried by an all white jury in southern Texas and sentenced to death. Let’s show the murderers that took the life of Troy Davis that we are not going anywhere anytime soon. This is a movement, not a moment.

Here is a link to the free, streaming documentary regarding Rodney and his story: http://video.google.com/videoplay?docid=-4864052717720140330

auto reblog for SIGNAL BOOST!

Again, ugh

(via jsmooth995)

Source: lifeoncinnabarisland

Adding test coverage on an existing model, part 4 - preset fields

As the last post in which I will discuss covering a part of the model with tests, I will look at a method the original developer called preset_fields. To give you a bit of a background, in order to start and complete any job, there needs to be a way for the company to quote parts of that job to different subcontractors. To make this easier, when a job is created, the application creates an empty quote section or sections, based on the job category selected. This lowers the mental load on the user and lets them move just a bit quicker.

Hopefully this makes the following code easier to digest:

The usage as it is right now:

I’m unsure whether this could be stuck into an after_create filter or left as is or if it’s even a good idea to do this in this way. However, I can deal with that later. Right now, I need to make sure that all of the logic branches in this method are covered. I also do not like to hang logic on strings. For one, this logic only works if the site is in English. For another, in this app, category names are configurable by the user. ‘Repair’ might be named ‘Improvement’, for example. Another thing to look at during the refactoring stage.

Anyhow, here are the specs:

Ugh, each spec has an awkward setup phase. The code looks fairly simple, but it relies on whole lot of state being present. Replicating this state in order to make sure each logic branch does what it’s supposed to is not easy. I am unsure what to do here, but I don’t like how these specs look.

So, now that these methods are more or less covered with tests, I’m going to start refactoring. That’s left for another day.

Adding test coverage on an existing model, part 3 - Dynamic search thingamabob

Up tonight is an attempt to test a class method named search. This class method enables the user to search for existing active jobs using a few different properties like name, ID and PO # given to the user by the client. It’s a nice piece of user functionality as it lowers the mental load for the user.

Personally, I would implement this specific requirement with an existing gem, like searchlogic or ransack, or implement a proper full-text search engine like sphinx. These are still options available, but firstly, I need to understand what exactly this method does. The implementation is as follows:

Hrm, that’s a pretty confusing piece of code. I’m not exactly sure how this is used, so let me ack this method call quickly and see what comes up. This is what I find:

Hrm, I don’t like that at all. I see a weirdly named method and I see strings that look like SQL being passed in. Let me look at the weirdly named method.

Yep, that’s what I thought. OK, I think I know how to test this now. Here are the specs that exercise both branches of that method.

Good. I know what and how. Searching is messy and I understand now why it’s best to implement an already existing solution rather than attempting to tackle one yourself. I am putting the replacement of this code with a gem high on my TODO list, which is growing by leaps and bounds.

This was a thorny one. Providing a simple and intuitive UI for users, especially non-technical ones, is hard and it leads to the mess of code like the above. Lesson learned.

Adding test coverage on an existing model, part 2 - display methods

Now that there is some semblance of test coverage on this model, I’m moving on to other low-hanging fruit. I noticed that there are two methods named display_name and display_shortname.

My immediate thought is that these methods should probably be in a presenter, but since right now we’re concerned with getting the model covered with tests, I’ll just work out the logic and see if I can’t maybe simplify them a touch.

Here are the implementations of the methods:

Hrm, there’s a few logic branches there, so there’ll be a few specs, I guess:

OK, I’m reasonably happy with that. I’ve covered the possibility that the name might somehow be nil, although the validations should catch that. I don’t like concatenation of strings using + as it looks ugly in the source and I’ve heard that the performance of string concatenation in that way is worse than the way I normally do it. So let’s fix that:

OK, that looks better to me. I’m not sure how I can pretty up the i18n call. Any suggestions would be welcome here. I run the tests and find them passing. Moving on.

Next up, let me take a look at the display_work_window method. From the looks of it, it checks the job’s start and end time and displays a message based on the state of those two fields. I’m guessing it’s being used in the display on the dashboard as an activity notification. A quick ack command confirms my guess. Its only invocation is in a view that’s called from the user’s dashboard.

OK, since there are a few branches, let me make sure that I exercise them all. Here’s the method’s implementation:

As you can see there’s a default message and then a message for each state. I won’t try to refactor this now, but I’ll try to test it all to enable the refactoring in the future.

OK, that makes me feel good and I’m building up a momentum here. I’ve knocked off a few low-hanging things and I’ve gotten a feel for the system.

There are definitively things to improve here. The + string concatenation is everywhere, annoying me. The messages are hard coded into the model, begging to be broken into a view or a presenter. Anyway, enough for now.

Adding test coverage on an existing model, part 1 - Validations

So, true to my word, but almost 3 months later :/, I’m going to detail some of my techniques for adding test coverage to an app that’s already in production paying bills.

After thinking about it for a while, I came to the common sense conclusion that before trying to fix things, I should cover the existing code with as many tests as possible. So, here we go.

When I first started on this task, I noticed that the model central to the operation of the entire app didn’t even have field validations on it. The fact that this app operated fine without what everyone knows as the basic building block of a Rails app was an illuminating moment for me. I asked my boss, the original developer and business stakeholder, what fields the Job model needed. I took the list and implemented the simplest validation ever.

This caused the following change to the model, as expected:

I’m omitting the rest of the model here for clarity.

I looked over the Admin::JobsController to make sure that this requirement didn’t break anything. Since the app was scaffolded, the requisite error handling parts were already in place. 

An important note: I was only focusing on testing the models initially. I fixed the generated controller specs where I needed to and added extra tests while I was there. However, most of my focus was to be in the models.

Exactly what I was thinking

From Issue #23 - SOLID design principles:

The mistake I made was thinking that splitting different aspects of functionality into modules was a valid way of respecting the single responsibility principle. But this is deeply flawed thinking, because the end result of pulling in roughly 50 methods into a single object by mixing in 8 modules results in a single object, Prawn::Document having 60+ public methods all sharing the same state and namespace. Any illusion of a physical separation of concerns is all smoke and mirrors here.

This is exactly my argument whenever anyone fields the argument that modules can be used to make a Ruby object conform to the Single Responsibility Principle. Well done, Greg.

Imitation is sincerest form of flattery

Inspired by Chris Strom’s chain of posts on the SPDY protocol, I’ve decided to quite literally rip off that format and apply it to my current topic of interest.

So, starting tomorrow, I will start a chain of posts chronicling my attempts to get an existing Rails 3 app under good test coverage. I hope to turn this into a presentation of some form later on. I’d also love feedback on the chain, as I’m looking to learn here, as well.

I’m excited to get going

Stupid rules

The FAA’s website indicates the agency is not fully sure how electronic waves might interfere with airline avionics, but they want to make sure passengers remain safe. According to the site, “there are still unknowns about the radio signals that portable electronic devices and cell phones give off.”

From: http://news.travel.aol.com/2011/03/25/do-electronic-gadgets-really-affect-an-airplanes-instruments/


In other words, “We haven’t investigated if electronic devices affect airplane electronics, but after thinking for a minute, it sounds like that should be the case. Also, we don’t plan to investigate, but since we have so much power when it comes to air travel and we can kick off any passenger for any reason whatsoever, we don’t need to. So, fuck you and you can’t use your mobile device, haha.”


Stupid rules are stupid.

Twitter dev fuckup

From the twitter-api-announce mailing list:

“Still, our user research shows that consumers continue to be confused by the different ways that a fractured landscape of third-party Twitter clients display tweets and let users interact with core Twitter functions.  For example, people get confused by websites or clients that display tweets in a way that doesn’t follow our design guidelines, or when services put their own verbs on tweets instead of the ones used on Twitter.  Similarly, a number of third-party consumer clients use their own versions of suggested users, trends, and other data streams, confusing users in our network even more.  Users should be able to view, retweet, and reply to @nytimes’ tweets the same way; see the same profile information about @whitehouse; and be able to join in the discussion around the same trending topics as everyone else across Twitter.”

I read this as “Why in the hell are you people writing and using unofficial Twitter clients?”

A question to my Cucumber-using peeps

How do you deal with ever-expanding step definition files?

From everything I’ve read, it seems that it’s acceptable to have multiple scenarios per feature file. However, this means that each scenario will have steps that are identical to other scenarios in that feature. This also means that there’ll be steps unique to a scenario and all these steps go into the same step definition file. Well, at least, I put them in the same file.

So, this brings up the question above. How does one manage a situation like this? Is it acceptable to have a huge step definition file? If not, how do I break it up?

MongoDB map/reduce

Note to self: Map/Reduce in MongoDB relies on having documents with the same key/value as one of the properties. It will not total up numeric values, if the documents holding these values have no K/V pairs in common.

Examples of what I’m on about

This will work:

This will not:

Repeat to remember

Documents must share a K/V pair in order to execute MapReduce in Mongo.

I’ve been told…by myself.

Authentication with Rails 3, Devise & MongoMapper

This is just a quick post about combining the above three libraries and one specific challenge I encountered. This mostly for my own documentation than anything else.

Overview

I am writing an app running on MongoDB. I chose to interface with the database through MongoMapper and decided to use Devise as my authentication library. The latter two interface well, but that interaction is poorly documented. Most documentation focuses on another popular ODM for Mongo called Mongoid.

The installation and setup procedures for all these libraries are well-documented elsewhere, I will only mention that MongoMapper should be installed from source, using the rails3 branch.

The gotcha

I followed all the instructions included in the Github wikis of each library above and ended up with a nice User model set up with all of the Devise macros for the model, which is cool.

The Devise generator also set up attr_accessible for me, which is one of the best practices of Rails development as anyone experienced will tell you.

This is where I ran into a gotcha.

Having created the first user through the console and having set up the password for that account, I proceeded to create the New User form. I included all the fields needed for the creation of a user, including the password and password confirmation fields.

I drove through the UI and noticed that Devise was throwing an error message saying ‘Password cannot be blank’. This was weird as I was passing the values for the two fields in from the form.

Experienced Rails devs will probably shout at their screens right about now saying, “attr_accessible, check attr_accessible!!”. They’re right, of course. Adding password and password_confirmation to attr_accessible fixes the problem.

Now that I think about it

It’s kinda weird that Devise did not add the two fields to attr_accessible, even though it clearly needed them to successfully save a document. Maybe this is an issue with the generator provided with the library and it’s something to look into.