Some refactoring to make pullreview.com happier.

app/controllers/admin/addresses_controller.rb

@@ -1,3 +1,4 @@
+# Shared code for Aliases and Mailboxes controller.
 class Admin::AddressesController < AdminController
 
   inherit_resources

app/controllers/admin/domains_controller.rb

@@ -1,3 +1,4 @@
+# Domains controller.
 class Admin::DomainsController < AdminController
 
   load_and_authorize_resource

app/controllers/admin/permissions_controller.rb

@@ -1,3 +1,4 @@
+# Permissions controller.
 class Admin::PermissionsController < AdminController
 
   inherit_resources

app/controllers/admin/versions_controller.rb

@@ -1,3 +1,4 @@
+# Versions controller.
 class Admin::VersionsController < AdminController
 
   load_and_authorize_resource

app/controllers/admin_controller.rb

@@ -1,3 +1,4 @@
+# Shared code for all admin controllers.
 class AdminController < ApplicationController
 
   authorize_resource

app/controllers/application_controller.rb

@@ -1,3 +1,4 @@
+# Application controller.
 class ApplicationController < ActionController::Base
 
   check_authorization unless: :devise_controller?

app/controllers/development_login_controller.rb

@@ -1,3 +1,4 @@
+# DevelopmentLogin controller for quick login in development environment.
 class DevelopmentLoginController < ApplicationController
 
   skip_authorization_check

app/controllers/mailboxes_controller.rb

@@ -1,3 +1,4 @@
+# Mailboxes controller.
 class MailboxesController < InheritedResources::Base
 
   load_and_authorize_resource

app/controllers/sessions_controller.rb

@@ -1,3 +1,4 @@
+# Sessions controller.
 class SessionsController < Devise::SessionsController
 
   before_filter :prepare_params, only: :create

app/helpers/addresses_helper.rb

@@ -1,30 +1,56 @@
+# Shared helpers for Alias and Mailbox models.
 module AddressesHelper
 
+  class DomainNotFound < StandardError
+  end
+
+  class AddressNotFound < StandardError
+  end
+
+
   def parent
     Domain.where(id: params[:domain_id]).first
   end
 
   def link_to_alias_or_mailbox(email_or_object)
     return nil if email_or_object.blank?
 
-    if email_or_object.is_a? Alias or email_or_object.is_a? Mailbox
-      o = email_or_object
-      email = o.email
-    else
-      username, domain = email_or_object.split('@')
-      domain = Domain.where(name: domain).first
+    begin
+      email, o = email_and_object(email_or_object)
+    rescue DomainNotFound, AddressNotFound
+      return email_or_object
+    end
 
-      return email_or_object if domain.nil?
+    link_to email, [:edit, :admin, o.domain, o]
+  end
 
-      o   = domain.aliases.where(username: username).first
-      o ||= domain.mailboxes.where(username: username).first
 
-      return email_or_object if o.nil?
+  private
 
-      email = email_or_object
+  def email_and_object(email_or_object)
+    case email_or_object.class.to_s
+      when 'Alias', 'Mailbox'
+        o = email_or_object
+        email = o.email
+      else
+        email, o = search_object_by_email(email_or_object)
     end
 
-    link_to email, [:edit, :admin, o.domain, o]
+    [email, o]
+  end
+
+  def search_object_by_email(email)
+    username, domain = email.split('@')
+    domain = Domain.where(name: domain).first
+
+    raise DomainNotFound if domain.nil?
+
+    o   = domain.aliases.where(username: username).first
+    o ||= domain.mailboxes.where(username: username).first
+
+    raise AddressNotFound if o.nil?
+
+    [email, o]
   end
 
 end

app/helpers/aliases_helper.rb

@@ -1,3 +1,4 @@
+# Helper for aliases views.
 module AliasesHelper
 
   include AddressesHelper

app/helpers/application_helper.rb

@@ -1,3 +1,4 @@
+# Helper methods available in any view.
 module ApplicationHelper
 
   def active_navigation(controller)

app/helpers/domains_helper.rb

@@ -1,3 +1,4 @@
+# Helper for domains views.
 module DomainsHelper
 
   def link_to_domain(domain)

app/helpers/mailboxes_helper.rb

@@ -1,3 +1,4 @@
+# Helper for mailboxes views.
 module MailboxesHelper
 
   include AddressesHelper

app/helpers/versions_helper.rb

@@ -1,3 +1,4 @@
+# Helper for versions views.
 module VersionsHelper
 
   include MailboxesHelper
@@ -10,6 +11,8 @@ def icon_for_event(event)
         icon :trash
       when 'update'
         icon :edit
+      else
+        raise "No icon for event '#{event}'."
     end
   end
 
@@ -62,6 +65,8 @@ def url_for_object(object)
         [:edit, :admin, object.domain, object]
       when 'Permission'
         [:edit, :admin, object.item, object]
+      else
+        raise "No URL for object class '#{object.class.to_s}'."
     end
   end
 

app/models/ability.rb

@@ -1,4 +1,6 @@
+# CanCan ability definitions.
 class Ability
+
   include CanCan::Ability
 
   def initialize(mailbox)
@@ -32,4 +34,5 @@ def initialize(mailbox)
       end
     end
   end
+
 end

app/models/alias.rb

@@ -1,5 +1,9 @@
+# Alias model.
 class Alias < ActiveRecord::Base
 
+  include AddressValidations
+
+
   belongs_to :domain
 
   attr_accessible :active, :description, :domain_id, :goto, :username
@@ -9,22 +13,6 @@ class Alias < ActiveRecord::Base
   has_paper_trail
 
 
-  validates :username,
-    presence: true,
-    uniqueness: {
-      scope: :domain_id,
-      message: 'Combination of username and domain is not unique.'
-    },
-    format: {
-      with: /\A[a-zA-Z0-9.\-_]+\z/,
-      message: 'Username contains invalid characters.'
-    },
-    exclusion: {
-      in: Settings.blocked_usernames,
-      message: 'Username is blocked.'
-    }
-
-  validates :domain_id, presence: true
   validates :goto, presence: true
 
 

app/models/domain.rb

@@ -1,3 +1,4 @@
+# Domain model.
 class Domain < ActiveRecord::Base
 
   include Permissionable

app/models/mailbox.rb

@@ -1,5 +1,8 @@
+# Mailbox model (also used for login to Møil).
 class Mailbox < ActiveRecord::Base
 
+  include AddressValidations
+
   belongs_to :domain
   has_many   :permissions, dependent: :destroy, as: :subject
   has_one    :relocation,  dependent: :destroy
@@ -18,21 +21,6 @@ class Mailbox < ActiveRecord::Base
   default_value_for :quota, Settings.default_quota
 
 
-  validates :username,
-    presence: true,
-    uniqueness: {
-      scope: :domain_id,
-      message: 'Combination of username and domain is not unique.'
-    },
-    format: {
-      with: /\A[a-zA-Z0-9.\-_]+\z/,
-      message: 'Username contains invalid characters.'
-    },
-    exclusion: {
-      in: Settings.blocked_usernames,
-      message: 'Username is blocked.'
-    }
-
   validates :password,
     presence: {
       if: :password_required?
@@ -45,7 +33,6 @@ class Mailbox < ActiveRecord::Base
       allow_blank: true
     }
 
-  validates :domain_id, presence: true
   validates :encrypted_password, presence: true
 
 

app/models/permission.rb

@@ -1,3 +1,4 @@
+# Permission model.
 class Permission < ActiveRecord::Base
 
   ROLES = %w(owner editor)

app/models/relocation.rb

@@ -1,3 +1,4 @@
+# Relocation model for moving of mailbox directories.
 class Relocation < ActiveRecord::Base
 
   belongs_to :mailbox

db/seeds.rb

@@ -30,6 +30,8 @@ def rand5; rand(5) + 1; end
     rand5.times { FactoryGirl.create :mailbox, domain: d }
     rand5.times { FactoryGirl.create :alias, domain: d }
   rescue
+    # Just ignore naming conflicts.
+    nil
   end
 end
 

lib/address_validations.rb

@@ -0,0 +1,25 @@
+# Shared validations for Alias and Mailbox models.
+module AddressValidations
+
+  extend ActiveSupport::Concern
+
+  included do
+    validates :username,
+      presence: true,
+      uniqueness: {
+        scope: :domain_id,
+        message: 'Combination of username and domain is not unique.'
+      },
+      format: {
+        with: /\A[a-zA-Z0-9.\-_]+\z/,
+        message: 'Username contains invalid characters.'
+      },
+      exclusion: {
+        in: Settings.blocked_usernames,
+        message: 'Username is blocked.'
+      }
+
+    validates :domain_id, presence: true
+  end
+
+end

lib/devise/encryptors/sha512_dovecot.rb

@@ -1,6 +1,10 @@
+# Devise encryptor for dovecot compatible SHA512 hash.
 module Devise
+  # Namespace for devise-encryptable gem.
   module Encryptable
+    # Namespace for custom devise encryptors.
     module Encryptors
+      # Devise encryptor for dovecot compatible SHA512 hash.
       class Sha512Dovecot < Base
         def self.digest(password, stretches, salt, pepper)
           password.crypt('$6$' + salt)

lib/password.rb

@@ -1,9 +1,11 @@
+# Password generation module.
 module Password
 
   module Sha512Crypt
 
     SEEDS = ('a'..'z').to_a + ('A'..'Z').to_a + (0..9).to_a + ['/', '.']
 
+    # Generate 16 characters long alphanumeric salt. 
     def self.generate_salt
       salt = ''
 

lib/permissionable.rb

@@ -1,4 +1,6 @@
+# Enables permission on models it is included into.
 module Permissionable
+
   extend ActiveSupport::Concern
 
   included do
@@ -34,4 +36,5 @@ def permission?(role, mailbox)
     # Deny otherwise.
     false
   end
+
 end

test/functional/admin/aliases_controller_test.rb

@@ -1,13 +1,13 @@
 require 'test_helper'
+require 'functional/admin/test_helper.rb'
 
+# Tests for app/controllers/admin/aliases_controller.rb
 class Admin::AliasesControllerTest < ActionController::TestCase
 
   context 'Admin namespace' do
     context 'as admin' do
       setup do
-        @mailbox = FactoryGirl.create :mailbox, admin: true
-        @domain_id = @mailbox.domain_id
-        sign_in @mailbox
+        create_and_sign_in_mailbox
       end
 
       context 'on GET to index' do

test/functional/admin/domains_controller_test.rb

@@ -1,5 +1,6 @@
 require 'test_helper'
 
+# Tests for app/controllers/admin/domains_controller.rb
 class Admin::DomainsControllerTest < ActionController::TestCase
 
   context 'Admin namespace' do

test/functional/admin/mailboxes_controller_test.rb

@@ -1,12 +1,12 @@
 require 'test_helper'
+require 'functional/admin/test_helper.rb'
 
+# Tests for app/controllers/admin/mailboxes_controller.rb
 class Admin::MailboxesControllerTest < ActionController::TestCase
 
   context 'As admin' do
     setup do
-      @mailbox = FactoryGirl.create :mailbox, admin: true
-      @domain_id = @mailbox.domain_id
-      sign_in @mailbox
+      create_and_sign_in_mailbox
     end
 
     context 'on GET to index' do