Organizations: minor code cleanup

app/controllers/organizations_controller.rb

@@ -1,20 +1,12 @@
 class OrganizationsController < ApplicationController
-  before_filter :load_resource
-
-  def load_resource
-    if params[:id]
-      @organization = Organization.find(params[:id])
-    else
-      @organizations = Organization.all
-    end
-  end
+  before_filter :load_resource, only: [:show, :edit, :update, :destroy]
 
   def new
     @organization = Organization.new
   end
 
   def index
-    @organizations = @organizations.matching(params[:q]) if params[:q].present?
+    @organizations = Organization.all
   end
 
   def show
@@ -27,7 +19,8 @@ def show
   end
 
   def create
-    @organization = @organizations.build organization_params
+    @organization = Organization.new(organization_params)
+
     if @organization.save
       redirect_to @organization, status: :created
     else
@@ -57,6 +50,10 @@ def set_current
 
   private
 
+  def load_resource
+    @organization = Organization.find(params[:id])
+  end
+
   def organization_params
     params[:organization].permit(*%w[name theme email phone web
                                      public_opening_times description address

app/models/organization.rb

@@ -2,11 +2,7 @@ class Organization < ActiveRecord::Base
   has_many :members, dependent: :destroy
   has_many :users, -> { order "members.created_at DESC" }, through: :members
   has_many :all_accounts, class_name: "Account", inverse_of: :organization
-  has_many :all_movements, class_name: "Movement", through: :all_accounts, source: :movements do
-    def with_transfer
-      joins(transfer: :movements)
-    end
-  end
+  has_many :all_movements, class_name: "Movement", through: :all_accounts, source: :movements
   has_many :all_transfers, class_name: "Transfer", through: :all_movements, source: :transfer
   has_one :account, as: :accountable
   has_many :member_accounts, through: :members, source: :account
@@ -15,11 +11,7 @@ def with_transfer
   has_many :inquiries
   has_many :documents, as: :documentable
 
-  scope :matching, ->(str) {
-    where(Organization.arel_table[:name].matches("%#{str}%"))
-  }
-
-  validates :name, uniqueness: true
+  validates :name, presence: true, uniqueness: true
 
   before_validation :ensure_url
   after_create :create_account

app/views/organizations/_form.html.erb

@@ -1,3 +1,4 @@
+<%= show_error_messages!(@organization) %>
 <%= simple_form_for @organization do |f| %>
   <%= f.input :name %>
   <%= f.input :email %>

app/views/organizations/edit.html.erb

@@ -1,10 +1,3 @@
-<% if @organization.errors.present? %>
-  <% @organization.errors.full_messages.each do |msg| %>
-    <div class="alert alert-danger" role="alert">
-      <%= msg %>
-    </div>
-  <% end %>
-<% end %>
 <h1>
   <%= link_to @organization, organization_path(@organization) %>
   <small><%= t 'global.edit' %></small>

app/views/organizations/index.html.erb

@@ -2,7 +2,7 @@
   <%= Organization.model_name.human(count: :many) %>
 </h1>
 
-<ul class="nav nav-pills">
+<ul class="nav navbar-nav pull-right">
   <% if superadmin? %>
     <li>
       <%= link_to new_organization_path do %>

db/migrate/20180514193153_change_organizations_name_constraints.rb

@@ -0,0 +1,11 @@
+class ChangeOrganizationsNameConstraints < ActiveRecord::Migration
+  def up
+    change_column_null :organizations, :name, false
+    add_index :organizations, :name, unique: true
+  end
+
+  def down
+    change_column_null :organizations, :name, true
+    remove_index :organizations, :name
+  end
+end

db/schema.rb

@@ -11,7 +11,7 @@
 #
 # It's strongly recommended that you check this file into your version control system.
 
-ActiveRecord::Schema.define(version: 20180501093846) do
+ActiveRecord::Schema.define(version: 20180514193153) do
 
   # These are extensions that must be enabled in order to support this database
   enable_extension "plpgsql"
@@ -115,7 +115,7 @@
   add_index "movements", ["transfer_id"], name: "index_movements_on_transfer_id", using: :btree
 
   create_table "organizations", force: :cascade do |t|
-    t.string   "name"
+    t.string   "name",                 limit: 255, null: false
     t.datetime "created_at"
     t.datetime "updated_at"
     t.integer  "reg_number_seq"
@@ -131,6 +131,8 @@
     t.string   "domain"
   end
 
+  add_index "organizations", ["name"], name: "index_organizations_on_name", unique: true, using: :btree
+
   create_table "posts", force: :cascade do |t|
     t.string   "title"
     t.string   "type"

spec/models/organization_spec.rb

@@ -17,35 +17,48 @@
     end
   end
 
-  it "1:  without http & https" do
-    organization.web = "www.casa.com"
-    expect(organization).to be_valid
-    expect(organization.web).to eq "http://www.casa.com"
-  end
-  it "2: with http" do
-    organization.web = "http://www.casa.com"
-    expect(organization).to be_valid
-    expect(organization.web).to eq "http://www.casa.com"
-  end
-  it "3: with https" do
-    organization.web = "https://www.casa.com"
-    expect(organization).to be_valid
-    expect(organization.web).to eq "https://www.casa.com"
-  end
-  it "4: blank" do
-    organization.web = ""
-    expect(organization).to be_valid
-    expect(organization.web).to eq ""
-  end
-  it "5: nil" do
-    organization.web = ""
-    expect(organization).to be_valid
-    expect(organization.web).to eq ""
+  describe 'ensure_url validation' do
+    it "without http & https" do
+      organization.web = "www.casa.com"
+      expect(organization).to be_valid
+      expect(organization.web).to eq "http://www.casa.com"
+    end
+
+    it "with http" do
+      organization.web = "http://www.casa.com"
+      expect(organization).to be_valid
+      expect(organization.web).to eq "http://www.casa.com"
+    end
+
+    it "with https" do
+      organization.web = "https://www.casa.com"
+      expect(organization).to be_valid
+      expect(organization.web).to eq "https://www.casa.com"
+    end
+
+    it "with blank value" do
+      organization.web = ""
+      expect(organization).to be_valid
+      expect(organization.web).to eq ""
+    end
+
+    it "with nil value" do
+      organization.web = nil
+      expect(organization).to be_valid
+      expect(organization.web).to eq nil
+    end
+
+    it "with an invalid" do
+      organization.web = "la casa"
+      expect(organization).not_to be_valid
+      expect(organization.web).to eq "la casa"
+      expect(organization.errors.size).to eq 1
+    end
   end
-  it "6: no url" do
-    organization.web = "la casa"
-    expect(organization).not_to be_valid
-    expect(organization.web).to eq "la casa"
-    expect(organization.errors.size).to eq 1
+
+  it 'name is mandatory' do
+    organization.name = nil
+    organization.save
+    expect(organization.errors[:name]).to include(I18n.t('errors.messages.blank'))
   end
 end