From dfcf567f26d3538b40f67f399b6dc180eb4b8e45 Mon Sep 17 00:00:00 2001
From: Rohan Mitchell <rohan@rohanmitchell.com>
Date: Thu, 21 Nov 2013 14:35:37 +1100
Subject: [PATCH] Sort payment methods by name

---
 .../payment_methods_controller_decorator.rb   |  8 +++---
 app/models/spree/payment_method_decorator.rb  |  6 +----
 spec/features/admin/payment_method_spec.rb    | 27 +++++++++++++++++--
 spec/models/spree/payment_method_spec.rb      | 11 ++++----
 4 files changed, 35 insertions(+), 17 deletions(-)

diff --git a/app/controllers/spree/admin/payment_methods_controller_decorator.rb b/app/controllers/spree/admin/payment_methods_controller_decorator.rb
index 151a32e235..11f707d9e6 100644
--- a/app/controllers/spree/admin/payment_methods_controller_decorator.rb
+++ b/app/controllers/spree/admin/payment_methods_controller_decorator.rb
@@ -6,14 +6,14 @@ Spree::Admin::PaymentMethodsController.class_eval do
     collection = if model_class.respond_to?(:accessible_by) &&
                      !current_ability.has_block?(params[:action], model_class)
 
-                   model_class.accessible_by(current_ability, action).
-                     managed_by(spree_current_user).by_distributor # This line added
+                   model_class.accessible_by(current_ability, action)
 
                  else
-                   model_class.scoped.
-                     managed_by(spree_current_user).by_distributor # This line added
+                   model_class.scoped
                  end
 
+    collection = collection.managed_by(spree_current_user).by_name # This line added
+
     # This block added
     if params.key? :enterprise_id
       distributor = Enterprise.find params[:enterprise_id]
diff --git a/app/models/spree/payment_method_decorator.rb b/app/models/spree/payment_method_decorator.rb
index a74a023ca8..8e696989a4 100644
--- a/app/models/spree/payment_method_decorator.rb
+++ b/app/models/spree/payment_method_decorator.rb
@@ -19,11 +19,7 @@ Spree::PaymentMethod.class_eval do
     where('enterprises.id = ?', distributor)
   }
 
-  scope :by_distributor, lambda {
-    joins(:distributors).
-    order('enterprises.name, spree_payment_methods.name').
-    select('enterprises.*, spree_payment_methods.*')
-  }
+  scope :by_name, order('spree_payment_methods.name ASC')
 
   def has_distributor?(distributor)
     self.distributors.include?(distributor)
diff --git a/spec/features/admin/payment_method_spec.rb b/spec/features/admin/payment_method_spec.rb
index 948ea6f058..be91f15490 100644
--- a/spec/features/admin/payment_method_spec.rb
+++ b/spec/features/admin/payment_method_spec.rb
@@ -35,8 +35,10 @@ feature %q{
     let(:enterprise_user) { create_enterprise_user }
     let(:distributor1) { create(:distributor_enterprise, name: 'First Distributor') }
     let(:distributor2) { create(:distributor_enterprise, name: 'Second Distributor') }
+    let(:distributor3) { create(:distributor_enterprise, name: 'Third Distributor') }
     let(:pm1) { create(:payment_method, name: 'One', distributors: [distributor1]) }
-    let(:pm2) { create(:payment_method, name: 'Two', distributors: [distributor2]) }
+    let(:pm2) { create(:payment_method, name: 'Two', distributors: [distributor1, distributor2]) }
+    let(:pm3) { create(:payment_method, name: 'Three', distributors: [distributor3]) }
 
     before(:each) do
       enterprise_user.enterprise_roles.build(enterprise: distributor1).save
@@ -60,6 +62,27 @@ feature %q{
       payment_method.distributors.should == [distributor1]
     end
 
+    it "shows me only payment methods I have access to" do
+      pm1
+      pm2
+      pm3
+
+      visit spree.admin_payment_methods_path
+
+      page.should     have_content pm1.name
+      page.should     have_content pm2.name
+      page.should_not have_content pm3.name
+    end
+
+    it "does not show duplicates of payment methods" do
+      pm1
+      pm2
+
+      visit spree.admin_payment_methods_path
+      page.all('td', text: 'Two').count.should == 1
+    end
+
+
     it "shows me only payment methods for the enterprise I select" do
       pm1
       pm2
@@ -67,7 +90,7 @@ feature %q{
       click_link 'Enterprises'
       within(".enterprise-#{distributor1.id}") { click_link 'Payment Methods' }
       page.should     have_content pm1.name
-      page.should_not have_content pm2.name
+      page.should     have_content pm2.name
 
       click_link 'Enterprises'
       within(".enterprise-#{distributor2.id}") { click_link 'Payment Methods' }
diff --git a/spec/models/spree/payment_method_spec.rb b/spec/models/spree/payment_method_spec.rb
index 173f5c807c..ef70afd426 100644
--- a/spec/models/spree/payment_method_spec.rb
+++ b/spec/models/spree/payment_method_spec.rb
@@ -2,13 +2,12 @@ require 'spec_helper'
 
 module Spree
   describe PaymentMethod do
-    it "finds payment methods for a particular distributor" do
-      d1 = create(:distributor_enterprise)
-      d2 = create(:distributor_enterprise)
-      pm1 = create(:payment_method, distributors: [d1])
-      pm2 = create(:payment_method, distributors: [d2])
+    it "orders payment methods by name" do
+      pm1 = create(:payment_method, name: 'ZZ')
+      pm2 = create(:payment_method, name: 'AA')
+      pm3 = create(:payment_method, name: 'BB')
 
-      PaymentMethod.for_distributor(d1).should == [pm1]
+      PaymentMethod.by_name.should == [pm2, pm3, pm1]
     end
   end
 end