From b9561ecf203f4600859ad21683ed089287c6c017 Mon Sep 17 00:00:00 2001 From: Will Marshall Date: Thu, 19 Jun 2014 11:41:24 +1000 Subject: [PATCH] Minor refactoring from code review with Rohan --- .../javascripts/darkswarm/services/hubs.js.coffee | 2 +- .../javascripts/darkswarm/services/map.js.coffee | 4 ++-- .../javascripts/darkswarm/services/map_modal.js.coffee | 9 +++++---- .../javascripts/darkswarm/services/producers.js.coffee | 2 +- app/controllers/base_controller.rb | 4 ---- app/controllers/home_controller.rb | 1 - app/controllers/map_controller.rb | 1 - app/controllers/producers_controller.rb | 1 - app/views/json/_enterprises.rabl | 3 +++ app/views/json/_enterprises_for_map.rabl | 2 -- app/views/json/partials/_enterprise.rabl | 10 +--------- app/views/json/partials/_hub.rabl | 4 ++-- spec/controllers/base_controller_spec.rb | 5 ----- spec/controllers/home_controller_spec.rb | 6 ------ spec/controllers/map_controller_spec.rb | 5 ----- spec/controllers/producers_controller_spec.rb | 5 ----- .../unit/darkswarm/services/hubs_spec.js.coffee | 6 +++--- .../unit/darkswarm/services/producers_spec.js.coffee | 2 +- 18 files changed, 19 insertions(+), 53 deletions(-) delete mode 100644 app/views/json/_enterprises_for_map.rabl diff --git a/app/assets/javascripts/darkswarm/services/hubs.js.coffee b/app/assets/javascripts/darkswarm/services/hubs.js.coffee index 70df62e888..5d55fa7de9 100644 --- a/app/assets/javascripts/darkswarm/services/hubs.js.coffee +++ b/app/assets/javascripts/darkswarm/services/hubs.js.coffee @@ -2,7 +2,7 @@ Darkswarm.factory 'Hubs', ($filter, Enterprises) -> new class Hubs constructor: -> @hubs = @order Enterprises.enterprises.filter (hub)-> - hub.enterprise_type == "hub" + hub.is_distributor order: (hubs)-> $filter('orderBy')(hubs, ['-active', '+orders_close_at']) diff --git a/app/assets/javascripts/darkswarm/services/map.js.coffee b/app/assets/javascripts/darkswarm/services/map.js.coffee index fd76eeff91..ed4bc8dae4 100644 --- a/app/assets/javascripts/darkswarm/services/map.js.coffee +++ b/app/assets/javascripts/darkswarm/services/map.js.coffee @@ -7,8 +7,8 @@ Darkswarm.factory "OfnMap", (Enterprises, MapModal)-> # Adding methods to each enterprise extend: (enterprise)-> new class MapMarker - # We're whitelisting attributes because Gmaps tries to crawl - # our data, and our data is recursive + # We're whitelisting attributes because GMaps tries to crawl + # our data, and our data is recursive, so it breaks latitude: enterprise.latitude longitude: enterprise.longitude icon: enterprise.icon diff --git a/app/assets/javascripts/darkswarm/services/map_modal.js.coffee b/app/assets/javascripts/darkswarm/services/map_modal.js.coffee index 2f1bd832b4..cd82edbe98 100644 --- a/app/assets/javascripts/darkswarm/services/map_modal.js.coffee +++ b/app/assets/javascripts/darkswarm/services/map_modal.js.coffee @@ -2,9 +2,10 @@ Darkswarm.factory "MapModal", ($modal, $rootScope)-> new class MapModal open: (enterprise)-> scope = $rootScope.$new(true) # Spawn an isolate to contain the enterprise - if enterprise.enterprise_type == "producer" - scope.producer = enterprise - $modal.open(templateUrl: "map_modal_producer.html", scope: scope) - else + + if enterprise.is_distributor scope.hub = enterprise $modal.open(templateUrl: "map_modal_hub.html", scope: scope) + else + scope.producer = enterprise + $modal.open(templateUrl: "map_modal_producer.html", scope: scope) diff --git a/app/assets/javascripts/darkswarm/services/producers.js.coffee b/app/assets/javascripts/darkswarm/services/producers.js.coffee index 6483ef35eb..ac8354c101 100644 --- a/app/assets/javascripts/darkswarm/services/producers.js.coffee +++ b/app/assets/javascripts/darkswarm/services/producers.js.coffee @@ -2,5 +2,5 @@ Darkswarm.factory 'Producers', (Enterprises) -> new class Producers constructor: -> @producers = Enterprises.enterprises.filter (enterprise)-> - enterprise.enterprise_type == "producer" + enterprise.is_primary_producer diff --git a/app/controllers/base_controller.rb b/app/controllers/base_controller.rb index 1a4c0aac8d..2ac35fa3ad 100644 --- a/app/controllers/base_controller.rb +++ b/app/controllers/base_controller.rb @@ -15,8 +15,4 @@ class BaseController < ApplicationController def load_active_distributors @active_distributors ||= Enterprise.distributors_with_active_order_cycles end - - def load_visible_enterprises - @enterprises = Enterprise.visible - end end diff --git a/app/controllers/home_controller.rb b/app/controllers/home_controller.rb index 97b386157d..76e179ed22 100644 --- a/app/controllers/home_controller.rb +++ b/app/controllers/home_controller.rb @@ -1,7 +1,6 @@ class HomeController < BaseController layout 'darkswarm' before_filter :load_active_distributors - before_filter :load_visible_enterprises def index end diff --git a/app/controllers/map_controller.rb b/app/controllers/map_controller.rb index d63d7f86a9..a980ba8f40 100644 --- a/app/controllers/map_controller.rb +++ b/app/controllers/map_controller.rb @@ -1,7 +1,6 @@ class MapController < BaseController layout 'darkswarm' before_filter :load_active_distributors - before_filter :load_visible_enterprises def index end diff --git a/app/controllers/producers_controller.rb b/app/controllers/producers_controller.rb index d7d665de90..b101a95b7f 100644 --- a/app/controllers/producers_controller.rb +++ b/app/controllers/producers_controller.rb @@ -1,7 +1,6 @@ class ProducersController < BaseController layout 'darkswarm' before_filter :load_active_distributors - before_filter :load_visible_enterprises def index end diff --git a/app/views/json/_enterprises.rabl b/app/views/json/_enterprises.rabl index 9b4988b83f..0f01743bfc 100644 --- a/app/views/json/_enterprises.rabl +++ b/app/views/json/_enterprises.rabl @@ -1,3 +1,6 @@ +# TODO: This should be moved into the controller +# RABL is tricky to pass variables into: so we do this as a workaround for now +# I noticed some vague comments on Rabl github about this, but haven't looked into collection Enterprise.visible extends 'json/partials/enterprise' extends 'json/partials/producer' diff --git a/app/views/json/_enterprises_for_map.rabl b/app/views/json/_enterprises_for_map.rabl deleted file mode 100644 index 9ce100ab65..0000000000 --- a/app/views/json/_enterprises_for_map.rabl +++ /dev/null @@ -1,2 +0,0 @@ -collection @enterprises -extends "json/partials/enterprise" diff --git a/app/views/json/partials/_enterprise.rabl b/app/views/json/partials/_enterprise.rabl index 86cd9f08aa..20c7ff2332 100644 --- a/app/views/json/partials/_enterprise.rabl +++ b/app/views/json/partials/_enterprise.rabl @@ -1,12 +1,4 @@ -attributes :name, :id, :description, :latitude, :longitude, :long_description, :website, :instagram, :linkedin, :twitter, :facebook - -node :enterprise_type do |enterprise| - if enterprise.is_primary_producer? - "producer" - elsif enterprise.is_distributor? - "hub" - end -end +attributes :name, :id, :description, :latitude, :longitude, :long_description, :website, :instagram, :linkedin, :twitter, :facebook, :is_primary_producer, :is_distributor node :email do |enterprise| enterprise.email.to_s.reverse diff --git a/app/views/json/partials/_hub.rabl b/app/views/json/partials/_hub.rabl index c1f7879268..1c11b23005 100644 --- a/app/views/json/partials/_hub.rabl +++ b/app/views/json/partials/_hub.rabl @@ -8,10 +8,10 @@ node :path do |enterprise| shop_enterprise_path(enterprise) end node :pickup do |hub| - not hub.shipping_methods.where(:require_ship_address => false).empty? + hub.shipping_methods.where(:require_ship_address => false).present? end node :delivery do |hub| - not hub.shipping_methods.where(:require_ship_address => true).empty? + hub.shipping_methods.where(:require_ship_address => true).present? end node :active do |hub| @active_distributors.include?(hub) diff --git a/spec/controllers/base_controller_spec.rb b/spec/controllers/base_controller_spec.rb index 85afcbc7ff..e431f14e3f 100644 --- a/spec/controllers/base_controller_spec.rb +++ b/spec/controllers/base_controller_spec.rb @@ -23,9 +23,4 @@ describe BaseController do Enterprise.should_receive(:distributors_with_active_order_cycles) controller.load_active_distributors end - - it "loads visible enterprises" do - Enterprise.should_receive(:visible) - controller.load_visible_enterprises - end end diff --git a/spec/controllers/home_controller_spec.rb b/spec/controllers/home_controller_spec.rb index 54b6611438..babb1c84e6 100644 --- a/spec/controllers/home_controller_spec.rb +++ b/spec/controllers/home_controller_spec.rb @@ -7,7 +7,6 @@ describe HomeController do before do Enterprise.stub(:distributors_with_active_order_cycles).and_return [distributor] - Enterprise.stub(:visible).and_return [distributor] end it "sets active distributors" do @@ -15,11 +14,6 @@ describe HomeController do assigns[:active_distributors].should == [distributor] end - it "loads visible enterprises" do - get :index - assigns[:enterprises].should == [distributor] - end - it "does not show invisible hubs" do get :index response.body.should_not have_content invisible_distributor.name diff --git a/spec/controllers/map_controller_spec.rb b/spec/controllers/map_controller_spec.rb index 455420380d..f3d56853f9 100644 --- a/spec/controllers/map_controller_spec.rb +++ b/spec/controllers/map_controller_spec.rb @@ -1,11 +1,6 @@ require 'spec_helper' describe MapController do - it "loads all visible enterprises" do - Enterprise.should_receive(:visible) - get :index - end - it "loads active distributors" do Enterprise.should_receive(:distributors_with_active_order_cycles) get :index diff --git a/spec/controllers/producers_controller_spec.rb b/spec/controllers/producers_controller_spec.rb index 7b399df209..d5a90a77d3 100644 --- a/spec/controllers/producers_controller_spec.rb +++ b/spec/controllers/producers_controller_spec.rb @@ -12,9 +12,4 @@ describe ProducersController do get :index assigns[:active_distributors].should == [distributor] end - - it "loads visible enterprises" do - get :index - assigns[:enterprises].should == [distributor] - end end diff --git a/spec/javascripts/unit/darkswarm/services/hubs_spec.js.coffee b/spec/javascripts/unit/darkswarm/services/hubs_spec.js.coffee index 9d4513af35..41828242a8 100644 --- a/spec/javascripts/unit/darkswarm/services/hubs_spec.js.coffee +++ b/spec/javascripts/unit/darkswarm/services/hubs_spec.js.coffee @@ -6,19 +6,19 @@ describe "Hubs service", -> id: 2 active: false orders_close_at: new Date() - enterprise_type: "hub" + is_distributor: true } { id: 3 active: false orders_close_at: new Date() - enterprise_type: "hub" + is_distributor: true } { id: 1 active: true orders_close_at: new Date() - enterprise_type: "hub" + is_distributor: true } ] diff --git a/spec/javascripts/unit/darkswarm/services/producers_spec.js.coffee b/spec/javascripts/unit/darkswarm/services/producers_spec.js.coffee index 403bf186ba..282e4725de 100644 --- a/spec/javascripts/unit/darkswarm/services/producers_spec.js.coffee +++ b/spec/javascripts/unit/darkswarm/services/producers_spec.js.coffee @@ -2,7 +2,7 @@ describe "Producers service", -> Producers = null Enterprises = null enterprises = [ - {enterprise_type: "producer"} + {is_primary_producer: true} ] beforeEach ->