Why Rails' try() and rspec's allow(x).to receive(:bla) can bite you in the butt:
When actions are taken by superusers, we're supposed to append admin_ to the SystemLog event names. For example: when one of us logs in, there should be a SystemLog with admin_log_in event. When normal users log in, there's a SystemLog with log_in event.
If you check your SystemLog table, you'll see that's not currently true ;) And it hasn't been for quite a while. This is surprising, since all of our SystemLog specs are passing!
Digging deeper, we find self.prefixed_event_name(event_name) in SystemLog, which is supposed to add the admin_ prefix. That method checks if the current_user is an admin with: current_user.try(:admin?). current_user could be nil, which is why we use try here. But guess what: the method User#admin? does not exist! We recently renamed admins to superusers, so the correct call should be current_user.try(:superuser?).
But then, how can our tests be passing??
In system_log_spec.rb we find the culprit: allow(user).to receive(:admin?).and_return(true).
That innocent-looking line creates the admin? method on user, so then the prefixed_event_name method correctly adds the admin_ prefix... but ONLY in our tests!
Lessons learned:
- Use
try!instead oftry.try!will actually call the method even if it doesn't exist. We would have caught this error much earlier, since we would have seenNoMethodError: undefined method admin? for #<User:0x00000003b74a78>all over the place. - Don't use
allow(x).to receive(:bla)if you don't need it. In this case, making the user an actual superuser withuser.superuser!is much shorter and clearer. Also, usingallow(x).to receive(:bla)tests the implementation, not the behavior.