From 4fa09639cb7337d2bc16f6ae263fc06c41a44625 Mon Sep 17 00:00:00 2001 From: Rob Harrington Date: Wed, 6 Jun 2018 16:27:23 +1000 Subject: [PATCH 01/71] Rewrite user stat query for improved performance of homepage --- app/controllers/home_controller.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/controllers/home_controller.rb b/app/controllers/home_controller.rb index 55b6b827e1..94dcd7d49c 100644 --- a/app/controllers/home_controller.rb +++ b/app/controllers/home_controller.rb @@ -7,7 +7,7 @@ class HomeController < BaseController if ContentConfig.home_show_stats @num_distributors = Enterprise.is_distributor.activated.visible.count @num_producers = Enterprise.is_primary_producer.activated.visible.count - @num_users = Spree::User.joins(:orders).merge(Spree::Order.complete).count('DISTINCT spree_users.*') + @num_users = Spree::Order.complete.count('DISTINCT user_id') @num_orders = Spree::Order.complete.count end end From f2e1caabff4e30fbb08c77684f9b54f0184be78e Mon Sep 17 00:00:00 2001 From: Frank West Date: Fri, 15 Jun 2018 08:27:26 -0700 Subject: [PATCH 02/71] Fix NoMethodError in order cycles index When a user's session has timed out and they try to load new data on the order cycles page by changing filters, the application throws a `NoMethodError` because we are prepending the load data method before checking the user's session. We can fix this by removing the prepend on this action. --- app/controllers/admin/order_cycles_controller.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/controllers/admin/order_cycles_controller.rb b/app/controllers/admin/order_cycles_controller.rb index 445e268085..bb2e2da2a7 100644 --- a/app/controllers/admin/order_cycles_controller.rb +++ b/app/controllers/admin/order_cycles_controller.rb @@ -6,7 +6,7 @@ module Admin class OrderCyclesController < ResourceController include OrderCyclesHelper - prepend_before_filter :load_data_for_index, :only => :index + before_filter :load_data_for_index, only: :index before_filter :require_coordinator, only: :new before_filter :remove_protected_attrs, only: [:update] before_filter :check_editable_schedule_ids, only: [:create, :update] From e73b37820166901e1bd9ebc2641696a5b6ef61a9 Mon Sep 17 00:00:00 2001 From: Matt-Yorkley Date: Fri, 15 Jun 2018 21:50:44 +0100 Subject: [PATCH 03/71] Adjust embedded response headers --- app/controllers/application_controller.rb | 6 ++++-- spec/requests/embedded_shopfronts_headers_spec.rb | 4 ++-- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index d22e8a6ba4..e3ea8b375f 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -60,7 +60,7 @@ class ApplicationController < ActionController::Base return if embedding_without_https? response.headers.delete 'X-Frame-Options' - response.headers['Content-Security-Policy'] = "frame-ancestors #{URI(request.referer).host.downcase}" + response.headers['Content-Security-Policy'] = "frame-ancestors 'self' #{URI(request.referer).host.downcase}" check_embedded_request set_embedded_layout @@ -73,8 +73,10 @@ class ApplicationController < ActionController::Base end def embeddable? - whitelist = Spree::Config[:embedded_shopfronts_whitelist] domain = embedded_shopfront_referer + return true if domain == request.host + + whitelist = Spree::Config[:embedded_shopfronts_whitelist] Spree::Config[:enable_embedded_shopfronts] && whitelist.present? && domain.present? && whitelist.include?(domain) end diff --git a/spec/requests/embedded_shopfronts_headers_spec.rb b/spec/requests/embedded_shopfronts_headers_spec.rb index 9d2c1c523e..6d6ea4b9f6 100644 --- a/spec/requests/embedded_shopfronts_headers_spec.rb +++ b/spec/requests/embedded_shopfronts_headers_spec.rb @@ -52,7 +52,7 @@ describe "setting response headers for embedded shopfronts", type: :request do expect(response.status).to be 200 expect(response.headers['X-Frame-Options']).to be_nil - expect(response.headers['Content-Security-Policy']).to eq "frame-ancestors external-site.com" + expect(response.headers['Content-Security-Policy']).to eq "frame-ancestors 'self' external-site.com" get spree.admin_path @@ -73,7 +73,7 @@ describe "setting response headers for embedded shopfronts", type: :request do expect(response.status).to be 200 expect(response.headers['X-Frame-Options']).to be_nil - expect(response.headers['Content-Security-Policy']).to eq "frame-ancestors www.external-site.com" + expect(response.headers['Content-Security-Policy']).to eq "frame-ancestors 'self' www.external-site.com" end end end From ff0e0d9f3d6461caf8f749942f1e6de607779619 Mon Sep 17 00:00:00 2001 From: Matt-Yorkley Date: Sat, 16 Jun 2018 02:49:22 +0100 Subject: [PATCH 04/71] Move logic from ApplicationController to service and improve clarity --- app/controllers/application_controller.rb | 47 +--------- app/services/embedded_page_service.rb | 90 +++++++++++++++++++ .../embedded_shopfronts_headers_spec.rb | 4 +- 3 files changed, 95 insertions(+), 46 deletions(-) create mode 100644 app/services/embedded_page_service.rb diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index e3ea8b375f..08a552cb6b 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -56,50 +56,9 @@ class ApplicationController < ActionController::Base end def enable_embedded_shopfront - return unless embeddable? - return if embedding_without_https? - - response.headers.delete 'X-Frame-Options' - response.headers['Content-Security-Policy'] = "frame-ancestors 'self' #{URI(request.referer).host.downcase}" - - check_embedded_request - set_embedded_layout - end - - def embedded_shopfront_referer - return if request.referer.blank? - domain = URI(request.referer).host.downcase - domain.start_with?('www.') ? domain[4..-1] : domain - end - - def embeddable? - domain = embedded_shopfront_referer - return true if domain == request.host - - whitelist = Spree::Config[:embedded_shopfronts_whitelist] - Spree::Config[:enable_embedded_shopfronts] && whitelist.present? && domain.present? && whitelist.include?(domain) - end - - def embedding_without_https? - request.referer && URI(request.referer).scheme != 'https' && !Rails.env.test? && !Rails.env.development? - end - - def check_embedded_request - return unless params[:embedded_shopfront] - - # Show embedded shopfront CSS - session[:embedded_shopfront] = true - - # Get shopfront slug and set redirect path - if params[:controller] == 'enterprises' && params[:action] == 'shop' && params[:id] - slug = params[:id] - session[:shopfront_redirect] = '/' + slug + '/shop?embedded_shopfront=true' - end - end - - def set_embedded_layout - return unless session[:embedded_shopfront] - @shopfront_layout = 'embedded' + embed_service = EmbeddedPageService.new(params, session, request, response) + embed_service.embed! + @shopfront_layout = 'embedded' if embed_service.use_embedded_layout end def action diff --git a/app/services/embedded_page_service.rb b/app/services/embedded_page_service.rb new file mode 100644 index 0000000000..b4ab2372cc --- /dev/null +++ b/app/services/embedded_page_service.rb @@ -0,0 +1,90 @@ +# Processes requests for pages embedded in iframes + +class EmbeddedPageService + attr_reader :use_embedded_layout + + def initialize(params, session, request, response) + @params = params + @session = session + @request = request + @response = response + + @embedding_domain = @session[:embedding_domain] + @use_embedded_layout = false + end + + def embed! + return unless embeddable? + return if embedding_without_https? + + process_embedded_request + set_response_headers + set_embedded_layout + end + + private + + def embeddable? + return true if current_referer == @request.host + + domain = current_referer_without_www + whitelist = Spree::Config[:embedded_shopfronts_whitelist] + + embedding_enabled? && whitelist.present? && domain.present? && whitelist.include?(domain) + end + + def embedding_without_https? + @request.referer && URI(@request.referer).scheme != 'https' && !Rails.env.test? && !Rails.env.development? + end + + def process_embedded_request + return unless @params[:embedded_shopfront] + + set_embedding_domain + + @session[:embedded_shopfront] = true + set_logout_redirect + end + + def set_response_headers + @response.headers.delete 'X-Frame-Options' + @response.headers['Content-Security-Policy'] = "frame-ancestors 'self' #{@embedding_domain}" + end + + def set_embedding_domain + return unless @params[:embedded_shopfront] + return if current_referer == @request.host + + @embedding_domain = current_referer + @session[:embedding_domain] = current_referer + end + + def set_logout_redirect + return unless enterprise_slug + @session[:shopfront_redirect] = '/' + enterprise_slug + '/shop?embedded_shopfront=true' + end + + def enterprise_slug + return false unless @params[:controller] == 'enterprises' && @params[:action] == 'shop' && @params[:id] + @params[:id] + end + + def current_referer + return if @request.referer.blank? + URI(@request.referer).host.downcase + end + + def current_referer_without_www + return unless current_referer + current_referer.start_with?('www.') ? current_referer[4..-1] : current_referer + end + + def set_embedded_layout + return unless @session[:embedded_shopfront] + @use_embedded_layout = true + end + + def embedding_enabled? + Spree::Config[:enable_embedded_shopfronts] + end +end diff --git a/spec/requests/embedded_shopfronts_headers_spec.rb b/spec/requests/embedded_shopfronts_headers_spec.rb index 6d6ea4b9f6..6428b009be 100644 --- a/spec/requests/embedded_shopfronts_headers_spec.rb +++ b/spec/requests/embedded_shopfronts_headers_spec.rb @@ -48,7 +48,7 @@ describe "setting response headers for embedded shopfronts", type: :request do end it "allows iframes on certain pages when enabled in configuration" do - get shops_path + get enterprise_shop_path(enterprise) + '?embedded_shopfront=true' expect(response.status).to be 200 expect(response.headers['X-Frame-Options']).to be_nil @@ -69,7 +69,7 @@ describe "setting response headers for embedded shopfronts", type: :request do end it "matches the URL structure in the header" do - get shops_path + get enterprise_shop_path(enterprise) + '?embedded_shopfront=true' expect(response.status).to be 200 expect(response.headers['X-Frame-Options']).to be_nil From 6e8187145995cdd88725ad74c57dea9585c91018 Mon Sep 17 00:00:00 2001 From: Matt-Yorkley Date: Wed, 20 Jun 2018 15:14:56 +0100 Subject: [PATCH 05/71] Specs for new EmbeddedPageService --- spec/services/embedded_page_service_spec.rb | 59 +++++++++++++++++++++ 1 file changed, 59 insertions(+) create mode 100644 spec/services/embedded_page_service_spec.rb diff --git a/spec/services/embedded_page_service_spec.rb b/spec/services/embedded_page_service_spec.rb new file mode 100644 index 0000000000..f1ea8a971e --- /dev/null +++ b/spec/services/embedded_page_service_spec.rb @@ -0,0 +1,59 @@ +require 'spec_helper' + +describe EmbeddedPageService do + let(:enterprise_slug) { 'test-enterprise' } + let(:params) { { controller: 'enterprises', action: 'shop', id: enterprise_slug, embedded_shopfront: true } } + let(:session) { {} } + let(:request) { ActionController::TestRequest.new('HTTP_HOST' => 'ofn-instance.com', 'HTTP_REFERER' => 'https://embedding-enterprise.com') } + let(:response) { ActionController::TestResponse.new(200, 'X-Frame-Options' => 'DENY', 'Content-Security-Policy' => "frame-ancestors 'none'") } + let(:service) { EmbeddedPageService.new(params, session, request, response) } + + before do + Spree::Config.set( + enable_embedded_shopfronts: true, + embedded_shopfronts_whitelist: 'embedding-enterprise.com example.com' + ) + end + + describe "#embed!" do + context "when the request's referer is in the whitelist" do + before { service.embed! } + + it "sets the response headers to enables embedding requests from the embedding site" do + expect(response.headers).to_not include 'X-Frame-Options' => 'DENY' + expect(response.headers).to include 'Content-Security-Policy' => "frame-ancestors 'self' embedding-enterprise.com" + end + + it "sets session variables" do + expect(session[:embedded_shopfront]).to eq true + expect(session[:embedding_domain]).to eq 'embedding-enterprise.com' + expect(session[:shopfront_redirect]).to eq '/' + enterprise_slug + '/shop?embedded_shopfront=true' + end + end + + context "when embedding is enabled for a different site in the current session" do + before do + session[:embedding_domain] = 'another-enterprise.com' + session[:shopfront_redirect] = '/another-enterprise/shop?embedded_shopfront=true' + service.embed! + end + + it "resets the session variables for the new request" do + expect(session[:embedded_shopfront]).to eq true + expect(session[:embedding_domain]).to eq 'embedding-enterprise.com' + expect(session[:shopfront_redirect]).to eq '/' + enterprise_slug + '/shop?embedded_shopfront=true' + end + end + + context "when the request's referer is not in the whitelist" do + before do + Spree::Config.set(embedded_shopfronts_whitelist: 'example.com') + service.embed! + end + + it "does not enable embedding" do + expect(response.headers['X-Frame-Options']).to eq 'DENY' + end + end + end +end From aaba6da1629a4f0782a10341c6c85e3aa13dc5b3 Mon Sep 17 00:00:00 2001 From: Matt-Yorkley Date: Sat, 16 Jun 2018 14:30:17 +0100 Subject: [PATCH 06/71] Add available_on notes to PI guide --- app/views/admin/product_import/guide/_columns.html.haml | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/app/views/admin/product_import/guide/_columns.html.haml b/app/views/admin/product_import/guide/_columns.html.haml index d5aa454670..c79a881162 100644 --- a/app/views/admin/product_import/guide/_columns.html.haml +++ b/app/views/admin/product_import/guide/_columns.html.haml @@ -91,3 +91,10 @@ %td (Various, see notes) %td Sets the product shipping category %td See below for a list of available categories + %tr + %td + %strong available_on + %td No + %td 2018-05-21 + %td Sets the date from which the product will be available + %td Date format is: YYYY-MM-DD From 172fa168ead0a01be6e623045b46f401c3cf4377 Mon Sep 17 00:00:00 2001 From: Matt-Yorkley Date: Thu, 21 Jun 2018 14:18:32 +0100 Subject: [PATCH 07/71] Change layout attribute to method with question mark --- app/controllers/application_controller.rb | 2 +- app/services/embedded_page_service.rb | 6 ++++-- spec/services/embedded_page_service_spec.rb | 6 +++++- 3 files changed, 10 insertions(+), 4 deletions(-) diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 08a552cb6b..65d1fb1f33 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -58,7 +58,7 @@ class ApplicationController < ActionController::Base def enable_embedded_shopfront embed_service = EmbeddedPageService.new(params, session, request, response) embed_service.embed! - @shopfront_layout = 'embedded' if embed_service.use_embedded_layout + @shopfront_layout = 'embedded' if embed_service.use_embedded_layout? end def action diff --git a/app/services/embedded_page_service.rb b/app/services/embedded_page_service.rb index b4ab2372cc..8d6e27df26 100644 --- a/app/services/embedded_page_service.rb +++ b/app/services/embedded_page_service.rb @@ -1,8 +1,6 @@ # Processes requests for pages embedded in iframes class EmbeddedPageService - attr_reader :use_embedded_layout - def initialize(params, session, request, response) @params = params @session = session @@ -22,6 +20,10 @@ class EmbeddedPageService set_embedded_layout end + def use_embedded_layout? + @use_embedded_layout + end + private def embeddable? diff --git a/spec/services/embedded_page_service_spec.rb b/spec/services/embedded_page_service_spec.rb index f1ea8a971e..eb44b014ab 100644 --- a/spec/services/embedded_page_service_spec.rb +++ b/spec/services/embedded_page_service_spec.rb @@ -15,7 +15,7 @@ describe EmbeddedPageService do ) end - describe "#embed!" do + describe "processing embedded page requests" do context "when the request's referer is in the whitelist" do before { service.embed! } @@ -29,6 +29,10 @@ describe EmbeddedPageService do expect(session[:embedding_domain]).to eq 'embedding-enterprise.com' expect(session[:shopfront_redirect]).to eq '/' + enterprise_slug + '/shop?embedded_shopfront=true' end + + it "publicly reports that embedded layout should be used" do + expect(service.use_embedded_layout?).to be true + end end context "when embedding is enabled for a different site in the current session" do From e25574790b32de7ec9d47c3c3abe068444bf6e10 Mon Sep 17 00:00:00 2001 From: Rob Harrington Date: Fri, 6 Apr 2018 14:55:21 +1000 Subject: [PATCH 08/71] Split out float: right css from .help-btn.tiny selector --- app/assets/stylesheets/darkswarm/ui.css.scss | 3 +++ app/views/shared/components/_show_profiles.html.haml | 2 +- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/app/assets/stylesheets/darkswarm/ui.css.scss b/app/assets/stylesheets/darkswarm/ui.css.scss index 1d1b9e166a..529fb6e48b 100644 --- a/app/assets/stylesheets/darkswarm/ui.css.scss +++ b/app/assets/stylesheets/darkswarm/ui.css.scss @@ -87,6 +87,9 @@ button.success, .button.success { &.tiny { padding: 0rem; margin: 0; + } + + &.right { float: right; } diff --git a/app/views/shared/components/_show_profiles.html.haml b/app/views/shared/components/_show_profiles.html.haml index 84a55ae405..5a1b5ec1db 100644 --- a/app/views/shared/components/_show_profiles.html.haml +++ b/app/views/shared/components/_show_profiles.html.haml @@ -1,6 +1,6 @@ .small-12.medium-6.columns.text-right .profile-checkbox - %button.button.secondary.tiny.help-btn.ng-scope{:popover => t(:components_profiles_popover, sitename: Spree::Config[:site_name]), "popover-placement" => "left"}>< + %button.button.secondary.tiny.right.help-btn.ng-scope{:popover => t(:components_profiles_popover, sitename: Spree::Config[:site_name]), "popover-placement" => "left"}>< %i.ofn-i_013-help %label %input{"ng-model" => "show_profiles", type: "checkbox", name: "profile"} From 6e76fd816474a85add770d10509c08e639634309 Mon Sep 17 00:00:00 2001 From: Rob Harrington Date: Fri, 20 Apr 2018 11:27:33 +1000 Subject: [PATCH 09/71] Add Api::CustomersController with update action --- app/controllers/api/customers_controller.rb | 16 ++++++ app/models/spree/ability_decorator.rb | 4 ++ config/routes.rb | 2 + .../api/customers_controller_spec.rb | 49 +++++++++++++++++++ 4 files changed, 71 insertions(+) create mode 100644 app/controllers/api/customers_controller.rb create mode 100644 spec/controllers/api/customers_controller_spec.rb diff --git a/app/controllers/api/customers_controller.rb b/app/controllers/api/customers_controller.rb new file mode 100644 index 0000000000..9df7534259 --- /dev/null +++ b/app/controllers/api/customers_controller.rb @@ -0,0 +1,16 @@ +module Api + class CustomersController < Spree::Api::BaseController + respond_to :json + + def update + @customer = Customer.find(params[:id]) + authorize! :update, @customer + + if @customer.update_attributes(params[:customer]) + render text: @customer.id, :status => 200 + else + invalid_resource!(@customer) + end + end + end +end diff --git a/app/models/spree/ability_decorator.rb b/app/models/spree/ability_decorator.rb index 385b9be032..daf8161142 100644 --- a/app/models/spree/ability_decorator.rb +++ b/app/models/spree/ability_decorator.rb @@ -64,6 +64,10 @@ class AbilityDecorator can [:update, :destroy], Spree::CreditCard do |credit_card| credit_card.user == user end + + can [:update], Customer do |customer| + customer.user == user + end end # New users can create an enterprise, and gain other permissions from doing this. diff --git a/config/routes.rb b/config/routes.rb index 53356e2f52..f0f6bd4488 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -217,6 +217,8 @@ Openfoodnetwork::Application.routes.draw do get :job_queue end + resources :customers, only: [:update] + post '/product_images/:product_id', to: 'product_images#update_product_image' end diff --git a/spec/controllers/api/customers_controller_spec.rb b/spec/controllers/api/customers_controller_spec.rb new file mode 100644 index 0000000000..764f767549 --- /dev/null +++ b/spec/controllers/api/customers_controller_spec.rb @@ -0,0 +1,49 @@ +require 'spec_helper' + +module Api + describe CustomersController, type: :controller do + include AuthenticationWorkflow + render_views + + let(:user) { create(:user) } + let(:customer) { create(:customer, user: user) } + let(:params) { { format: :json, id: customer.id, customer: { code: '123' } } } + + describe "#update" do + context "as a user who is not associated with the customer" do + before do + allow(controller).to receive(:spree_current_user) { create(:user) } + end + + it "returns unauthorized" do + spree_post :update, params + assert_unauthorized! + end + end + + context "as the user associated with the customer" do + before do + allow(controller).to receive(:spree_current_user) { user } + end + + context "when the update request is successful" do + it "returns the id of the updated customer" do + spree_post :update, params + expect(response.status).to eq 200 + expect(response.body).to eq customer.id.to_s + end + end + + context "when the update request fails" do + before { params[:customer][:email] = '' } + + it "returns a 422, with an error message" do + spree_post :update, params + expect(response.status).to be 422 + expect(JSON.parse(response.body)['error']).to be + end + end + end + end + end +end From 29922d4be992854318c8bfe62434d8275e3575de Mon Sep 17 00:00:00 2001 From: Rob Harrington Date: Wed, 11 Apr 2018 09:54:43 +1000 Subject: [PATCH 10/71] Add allow_charges field to Customer model --- .../20180406045821_add_charges_allowed_to_customers.rb | 5 +++++ db/schema.rb | 9 +++++---- 2 files changed, 10 insertions(+), 4 deletions(-) create mode 100644 db/migrate/20180406045821_add_charges_allowed_to_customers.rb diff --git a/db/migrate/20180406045821_add_charges_allowed_to_customers.rb b/db/migrate/20180406045821_add_charges_allowed_to_customers.rb new file mode 100644 index 0000000000..4503dc87b6 --- /dev/null +++ b/db/migrate/20180406045821_add_charges_allowed_to_customers.rb @@ -0,0 +1,5 @@ +class AddChargesAllowedToCustomers < ActiveRecord::Migration + def change + add_column :customers, :allow_charges, :boolean, default: false, null: false + end +end diff --git a/db/schema.rb b/db/schema.rb index 787c37c227..caad9f27a5 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -79,15 +79,16 @@ ActiveRecord::Schema.define(:version => 20180418025217) do add_index "coordinator_fees", ["order_cycle_id"], :name => "index_coordinator_fees_on_order_cycle_id" create_table "customers", :force => true do |t| - t.string "email", :null => false - t.integer "enterprise_id", :null => false + t.string "email", :null => false + t.integer "enterprise_id", :null => false t.string "code" t.integer "user_id" - t.datetime "created_at", :null => false - t.datetime "updated_at", :null => false + t.datetime "created_at", :null => false + t.datetime "updated_at", :null => false t.integer "bill_address_id" t.integer "ship_address_id" t.string "name" + t.boolean "allow_charges", :default => false, :null => false end add_index "customers", ["bill_address_id"], :name => "index_customers_on_bill_address_id" From ffa8a8c7d685e04e39b445d1abc840ae8628123f Mon Sep 17 00:00:00 2001 From: Rob Harrington Date: Fri, 27 Apr 2018 16:04:58 +1000 Subject: [PATCH 11/71] Create Api::BaseController to allow use of ActiveModelSerializers Also add index action to Api::CustomersController --- app/controllers/api/base_controller.rb | 13 +++++++++ app/controllers/api/customers_controller.rb | 9 ++++--- app/controllers/api/statuses_controller.rb | 2 +- app/serializers/api/customer_serializer.rb | 5 ++++ config/routes.rb | 2 +- .../api/customers_controller_spec.rb | 27 ++++++++++++++++--- spec/support/api_helper.rb | 15 +++++++++++ 7 files changed, 64 insertions(+), 9 deletions(-) create mode 100644 app/controllers/api/base_controller.rb create mode 100644 app/serializers/api/customer_serializer.rb create mode 100644 spec/support/api_helper.rb diff --git a/app/controllers/api/base_controller.rb b/app/controllers/api/base_controller.rb new file mode 100644 index 0000000000..f25c47417d --- /dev/null +++ b/app/controllers/api/base_controller.rb @@ -0,0 +1,13 @@ +# Base controller for OFN's API +# Includes the minimum machinery required by ActiveModelSerializers +module Api + class BaseController < Spree::Api::BaseController + # Need to include these because Spree::Api::BaseContoller inherits + # from ActionController::Metal rather than ActionController::Base + # and they are required by ActiveModelSerializers + include ActionController::Serialization + include ActionController::UrlFor + include Rails.application.routes.url_helpers + use_renderers :json + end +end diff --git a/app/controllers/api/customers_controller.rb b/app/controllers/api/customers_controller.rb index 9df7534259..cbbe4ce35f 100644 --- a/app/controllers/api/customers_controller.rb +++ b/app/controllers/api/customers_controller.rb @@ -1,13 +1,16 @@ module Api - class CustomersController < Spree::Api::BaseController - respond_to :json + class CustomersController < BaseController + def index + @customers = current_api_user.customers + render json: @customers, each_serializer: CustomerSerializer + end def update @customer = Customer.find(params[:id]) authorize! :update, @customer if @customer.update_attributes(params[:customer]) - render text: @customer.id, :status => 200 + render json: @customer, serializer: CustomerSerializer, status: 200 else invalid_resource!(@customer) end diff --git a/app/controllers/api/statuses_controller.rb b/app/controllers/api/statuses_controller.rb index c8844b868b..49a6f991ff 100644 --- a/app/controllers/api/statuses_controller.rb +++ b/app/controllers/api/statuses_controller.rb @@ -1,5 +1,5 @@ module Api - class StatusesController < BaseController + class StatusesController < ::BaseController respond_to :json def job_queue diff --git a/app/serializers/api/customer_serializer.rb b/app/serializers/api/customer_serializer.rb new file mode 100644 index 0000000000..44914e0a49 --- /dev/null +++ b/app/serializers/api/customer_serializer.rb @@ -0,0 +1,5 @@ +module Api + class CustomerSerializer < ActiveModel::Serializer + attributes :id, :enterprise_id, :name, :code, :email, :allow_charges + end +end diff --git a/config/routes.rb b/config/routes.rb index f0f6bd4488..ba4da8ae2e 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -217,7 +217,7 @@ Openfoodnetwork::Application.routes.draw do get :job_queue end - resources :customers, only: [:update] + resources :customers, only: [:index, :update] post '/product_images/:product_id', to: 'product_images#update_product_image' end diff --git a/spec/controllers/api/customers_controller_spec.rb b/spec/controllers/api/customers_controller_spec.rb index 764f767549..f6c8f4e0a5 100644 --- a/spec/controllers/api/customers_controller_spec.rb +++ b/spec/controllers/api/customers_controller_spec.rb @@ -3,13 +3,32 @@ require 'spec_helper' module Api describe CustomersController, type: :controller do include AuthenticationWorkflow + include OpenFoodNetwork::ApiHelper render_views let(:user) { create(:user) } - let(:customer) { create(:customer, user: user) } - let(:params) { { format: :json, id: customer.id, customer: { code: '123' } } } + + describe "index" do + let!(:customer1) { create(:customer) } + let!(:customer2) { create(:customer) } + + before do + user.customers << customer1 + allow(controller).to receive(:spree_current_user) { user } + end + + it "lists customers associated with the current user" do + spree_get :index + expect(response.status).to eq 200 + expect(json_response.length).to eq 1 + expect(json_response.first[:id]).to eq customer1.id + end + end describe "#update" do + let(:customer) { create(:customer, user: user) } + let(:params) { { format: :json, id: customer.id, customer: { code: '123' } } } + context "as a user who is not associated with the customer" do before do allow(controller).to receive(:spree_current_user) { create(:user) } @@ -30,7 +49,7 @@ module Api it "returns the id of the updated customer" do spree_post :update, params expect(response.status).to eq 200 - expect(response.body).to eq customer.id.to_s + expect(json_response[:id]).to eq customer.id end end @@ -40,7 +59,7 @@ module Api it "returns a 422, with an error message" do spree_post :update, params expect(response.status).to be 422 - expect(JSON.parse(response.body)['error']).to be + expect(json_response[:error]).to be end end end diff --git a/spec/support/api_helper.rb b/spec/support/api_helper.rb new file mode 100644 index 0000000000..08918fb761 --- /dev/null +++ b/spec/support/api_helper.rb @@ -0,0 +1,15 @@ +module OpenFoodNetwork + module ApiHelper + def json_response + json_response = JSON.parse(response.body) + case json_response + when Hash + json_response.with_indifferent_access + when Array + json_response.map(&:with_indifferent_access) + else + json_response + end + end + end +end From 6457a17fde359f00ddae254e67a0dc4a9d2799d0 Mon Sep 17 00:00:00 2001 From: Rob Harrington Date: Fri, 27 Apr 2018 17:42:34 +1000 Subject: [PATCH 12/71] Add basic view allowing customers to authorise shop use of their credit cards --- .../authorised_shops_controller.js.coffee | 3 ++ .../darkswarm/services/customer.js.coffee | 20 ++++++++++ .../darkswarm/services/customers.js.coffee | 13 +++++++ .../darkswarm/services/shops.js.coffee | 13 +++++++ .../stylesheets/darkswarm/account.css.scss | 6 +++ .../spree/users/_authorised_shops.html.haml | 13 +++++++ app/views/spree/users/_cards.html.haml | 13 +++++-- config/locales/en.yml | 3 ++ spec/features/consumer/account/cards_spec.rb | 11 +++++- .../services/customer_spec.js.coffee | 39 +++++++++++++++++++ .../services/customers_spec.js.coffee | 24 ++++++++++++ .../darkswarm/services/shops_spec.js.coffee | 27 +++++++++++++ 12 files changed, 181 insertions(+), 4 deletions(-) create mode 100644 app/assets/javascripts/darkswarm/controllers/authorised_shops_controller.js.coffee create mode 100644 app/assets/javascripts/darkswarm/services/customer.js.coffee create mode 100644 app/assets/javascripts/darkswarm/services/customers.js.coffee create mode 100644 app/assets/javascripts/darkswarm/services/shops.js.coffee create mode 100644 app/views/spree/users/_authorised_shops.html.haml create mode 100644 spec/javascripts/unit/darkswarm/services/customer_spec.js.coffee create mode 100644 spec/javascripts/unit/darkswarm/services/customers_spec.js.coffee create mode 100644 spec/javascripts/unit/darkswarm/services/shops_spec.js.coffee diff --git a/app/assets/javascripts/darkswarm/controllers/authorised_shops_controller.js.coffee b/app/assets/javascripts/darkswarm/controllers/authorised_shops_controller.js.coffee new file mode 100644 index 0000000000..f100a0a7c3 --- /dev/null +++ b/app/assets/javascripts/darkswarm/controllers/authorised_shops_controller.js.coffee @@ -0,0 +1,3 @@ +angular.module("Darkswarm").controller "AuthorisedShopsCtrl", ($scope, Customers, Shops) -> + $scope.customers = Customers.index() + $scope.shopsByID = Shops.byID diff --git a/app/assets/javascripts/darkswarm/services/customer.js.coffee b/app/assets/javascripts/darkswarm/services/customer.js.coffee new file mode 100644 index 0000000000..ac27945c54 --- /dev/null +++ b/app/assets/javascripts/darkswarm/services/customer.js.coffee @@ -0,0 +1,20 @@ +angular.module("Darkswarm").factory 'Customer', ($resource, RailsFlashLoader) -> + Customer = $resource('/api/customers/:id/:action.json', {}, { + 'index': + method: 'GET' + isArray: true + 'update': + method: 'PUT' + params: + id: '@id' + transformRequest: (data, headersGetter) -> + angular.toJson(customer: data) + }) + + Customer.prototype.update = -> + @$update().then (response) => + RailsFlashLoader.loadFlash({success: t('js.changes_saved')}) + , (response) => + RailsFlashLoader.loadFlash({error: response.data.error}) + + Customer diff --git a/app/assets/javascripts/darkswarm/services/customers.js.coffee b/app/assets/javascripts/darkswarm/services/customers.js.coffee new file mode 100644 index 0000000000..cf5c56563a --- /dev/null +++ b/app/assets/javascripts/darkswarm/services/customers.js.coffee @@ -0,0 +1,13 @@ +angular.module("Darkswarm").factory 'Customers', (Customer) -> + new class Customers + all: [] + byID: {} + + index: (params={}) -> + Customer.index params, (data) => @load(data) + @all + + load: (customers) -> + for customer in customers + @all.push customer + @byID[customer.id] = customer diff --git a/app/assets/javascripts/darkswarm/services/shops.js.coffee b/app/assets/javascripts/darkswarm/services/shops.js.coffee new file mode 100644 index 0000000000..0af4152508 --- /dev/null +++ b/app/assets/javascripts/darkswarm/services/shops.js.coffee @@ -0,0 +1,13 @@ +angular.module("Darkswarm").factory 'Shops', ($injector) -> + new class Shops + all: [] + byID: {} + + constructor: -> + if $injector.has('shops') + @load($injector.get('shops')) + + load: (shops) -> + for shop in shops + @all.push shop + @byID[shop.id] = shop diff --git a/app/assets/stylesheets/darkswarm/account.css.scss b/app/assets/stylesheets/darkswarm/account.css.scss index c41cd2ef98..44d93b4cd1 100644 --- a/app/assets/stylesheets/darkswarm/account.css.scss +++ b/app/assets/stylesheets/darkswarm/account.css.scss @@ -28,6 +28,12 @@ margin-bottom: 0px; } } + + .authorised_shops{ + table { + width: 100%; + } + } } .orders { diff --git a/app/views/spree/users/_authorised_shops.html.haml b/app/views/spree/users/_authorised_shops.html.haml new file mode 100644 index 0000000000..692c953f12 --- /dev/null +++ b/app/views/spree/users/_authorised_shops.html.haml @@ -0,0 +1,13 @@ +%table + %tr + %th= t(:shop_title) + %th= t(:allow_charges?) + %tr.customer{ id: "customer{{ customer.id }}", ng: { repeat: "customer in customers" } } + %td.shop{ ng: { bind: 'shopsByID[customer.enterprise_id].name' } } + %td.allow_charges + %input{ type: 'checkbox', + name: 'allow_charges', + ng: { model: 'customer.allow_charges', + change: 'customer.update()', + "true-value" => "true", + "false-value" => "false" } } diff --git a/app/views/spree/users/_cards.html.haml b/app/views/spree/users/_cards.html.haml index 80069c282f..8929aac791 100644 --- a/app/views/spree/users/_cards.html.haml +++ b/app/views/spree/users/_cards.html.haml @@ -10,6 +10,13 @@ %button.button.primary{ ng: { click: 'showForm()', hide: 'CreditCard.visible' } } = t(:add_a_card) - .small-12.medium-6.columns.new_card{ ng: { show: 'CreditCard.visible', class: '{visible: CreditCard.visible}' } } - %h3= t(:add_a_new_card) - = render 'new_card_form' + .small-12.medium-6.columns + .new_card{ ng: { show: 'CreditCard.visible', class: '{visible: CreditCard.visible}' } } + %h3= t(:add_a_new_card) + = render 'new_card_form' + .authorised_shops{ ng: { controller: 'AuthorisedShopsCtrl', hide: 'CreditCard.visible' } } + %h3 + = t('.authorised_shops') + %button.button.secondary.tiny.help-btn.ng-scope{ :popover => t('.authorised_shops_popover'), "popover-placement" => 'right' } + %i.ofn-i_013-help + = render 'authorised_shops' diff --git a/config/locales/en.yml b/config/locales/en.yml index 35754e0bc2..597dd60c36 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -2740,5 +2740,8 @@ See the %{link} to find out more about %{sitename}'s features and to start using saved_cards: default?: Default? delete?: Delete? + cards: + authorised_shops: Authorised Shops + authorised_shops_popover: This is a list of shops which are permitted to charge your default credit card for OFN services (eg. subscriptions). Your card details will be kept secure and will not be shared with shop owners. localized_number: invalid_format: has an invalid format. Please enter a number. diff --git a/spec/features/consumer/account/cards_spec.rb b/spec/features/consumer/account/cards_spec.rb index 2f78511135..25c6410ade 100644 --- a/spec/features/consumer/account/cards_spec.rb +++ b/spec/features/consumer/account/cards_spec.rb @@ -4,6 +4,7 @@ feature "Credit Cards", js: true do include AuthenticationWorkflow describe "as a logged in user" do let(:user) { create(:user) } + let!(:customer) { create(:customer, user: user) } let!(:default_card) { create(:credit_card, user_id: user.id, gateway_customer_profile_id: 'cus_AZNMJ', is_default: true) } let!(:non_default_card) { create(:credit_card, user_id: user.id, gateway_customer_profile_id: 'cus_FDTG') } @@ -49,10 +50,10 @@ feature "Credit Cards", js: true do expect(page).to have_content I18n.t('js.default_card_updated') + expect(default_card.reload.is_default).to be false within(".card#card#{default_card.id}") do expect(find_field('default_card')).to_not be_checked end - expect(default_card.reload.is_default).to be false expect(non_default_card.reload.is_default).to be true # Shows the interface for adding a card @@ -67,6 +68,14 @@ feature "Credit Cards", js: true do expect(page).to have_content I18n.t(:card_has_been_removed, number: "x-#{default_card.last_digits}") expect(page).to_not have_selector ".card#card#{default_card.id}" + + # Allows authorisation of card use by shops + within "tr#customer#{customer.id}" do + expect(find_field('allow_charges')).to_not be_checked + find_field('allow_charges').click + end + expect(page).to have_content I18n.t('js.changes_saved') + expect(customer.reload.allow_charges).to be true end end end diff --git a/spec/javascripts/unit/darkswarm/services/customer_spec.js.coffee b/spec/javascripts/unit/darkswarm/services/customer_spec.js.coffee new file mode 100644 index 0000000000..33a1d72cdc --- /dev/null +++ b/spec/javascripts/unit/darkswarm/services/customer_spec.js.coffee @@ -0,0 +1,39 @@ +describe 'Customer', -> + describe "update", -> + $httpBackend = null + customer = null + response = { id: 3, code: '1234' } + RailsFlashLoaderMock = jasmine.createSpyObj('RailsFlashLoader', ['loadFlash']) + + beforeEach -> + module 'Darkswarm' + module ($provide) -> + $provide.value 'RailsFlashLoader', RailsFlashLoaderMock + null + + inject (_$httpBackend_, Customer)-> + customer = new Customer(id: 3) + $httpBackend = _$httpBackend_ + + it "nests the params inside 'customer'", -> + $httpBackend + .expectPUT('/api/customers/3.json', { customer: { id: 3 } }) + .respond 200, response + customer.update() + $httpBackend.flush() + + describe "when the request succeeds", -> + it "shows a success flash", -> + $httpBackend.expectPUT('/api/customers/3.json').respond 200, response + customer.update() + $httpBackend.flush() + expect(RailsFlashLoaderMock.loadFlash) + .toHaveBeenCalledWith({success: jasmine.any(String)}) + + describe "when the request fails", -> + it "shows a error flash", -> + $httpBackend.expectPUT('/api/customers/3.json').respond 400, { error: 'Some error' } + customer.update() + $httpBackend.flush() + expect(RailsFlashLoaderMock.loadFlash) + .toHaveBeenCalledWith({error: 'Some error'}) diff --git a/spec/javascripts/unit/darkswarm/services/customers_spec.js.coffee b/spec/javascripts/unit/darkswarm/services/customers_spec.js.coffee new file mode 100644 index 0000000000..9680b89341 --- /dev/null +++ b/spec/javascripts/unit/darkswarm/services/customers_spec.js.coffee @@ -0,0 +1,24 @@ +describe 'Customers', -> + describe "index", -> + $httpBackend = null + Customers = null + customerList = ['somecustomer'] + + beforeEach -> + module 'Darkswarm' + module ($provide) -> + $provide.value 'RailsFlashLoader', null + null + + inject (_$httpBackend_, _Customers_)-> + Customers = _Customers_ + $httpBackend = _$httpBackend_ + + it "asks for customers and returns @all, promises to populate via @load", -> + spyOn(Customers,'load').and.callThrough() + $httpBackend.expectGET('/api/customers.json').respond 200, customerList + result = Customers.index() + $httpBackend.flush() + expect(Customers.load).toHaveBeenCalled() + expect(result).toEqual customerList + expect(Customers.all).toEqual customerList diff --git a/spec/javascripts/unit/darkswarm/services/shops_spec.js.coffee b/spec/javascripts/unit/darkswarm/services/shops_spec.js.coffee new file mode 100644 index 0000000000..ddc9e14af5 --- /dev/null +++ b/spec/javascripts/unit/darkswarm/services/shops_spec.js.coffee @@ -0,0 +1,27 @@ +describe 'Shops', -> + describe "initialisation", -> + Shops = null + shops = ['some shop'] + + beforeEach -> + module 'Darkswarm' + + describe "when the injector does not have a value for 'shops'", -> + beforeEach -> + inject (_Shops_) -> + Shops = _Shops_ + + it "does nothing, leaves @all empty", -> + expect(Shops.all).toEqual [] + + describe "when the injector has a value for 'shops'", -> + beforeEach -> + module ($provide) -> + $provide.value 'shops', shops + null + + inject (_Shops_) -> + Shops = _Shops_ + + it "loads injected shops array into @all", -> + expect(Shops.all).toEqual shops From 32622c77bceae8cca405e6224ad0edffba9d88df Mon Sep 17 00:00:00 2001 From: Rob Harrington Date: Fri, 22 Jun 2018 13:57:21 +1000 Subject: [PATCH 13/71] Add basic help modal directive Useful for showing help text that is too long for a tool tip --- .../darkswarm/directives/help_modal.js.coffee | 10 ++++++++++ app/assets/javascripts/templates/help-modal.html.haml | 9 +++++++++ app/assets/stylesheets/darkswarm/help-modal.css.scss | 9 +++++++++ 3 files changed, 28 insertions(+) create mode 100644 app/assets/javascripts/darkswarm/directives/help_modal.js.coffee create mode 100644 app/assets/javascripts/templates/help-modal.html.haml create mode 100644 app/assets/stylesheets/darkswarm/help-modal.css.scss diff --git a/app/assets/javascripts/darkswarm/directives/help_modal.js.coffee b/app/assets/javascripts/darkswarm/directives/help_modal.js.coffee new file mode 100644 index 0000000000..6aef4f481b --- /dev/null +++ b/app/assets/javascripts/darkswarm/directives/help_modal.js.coffee @@ -0,0 +1,10 @@ +Darkswarm.directive "helpModal", ($modal, $compile, $templateCache)-> + restrict: 'A' + scope: + helpText: "@helpModal" + + link: (scope, elem, attrs, ctrl)-> + compiled = $compile($templateCache.get('help-modal.html'))(scope) + + elem.on "click", => + $modal.open(controller: ctrl, template: compiled, scope: scope, windowClass: 'help-modal small') diff --git a/app/assets/javascripts/templates/help-modal.html.haml b/app/assets/javascripts/templates/help-modal.html.haml new file mode 100644 index 0000000000..f87abd8fd4 --- /dev/null +++ b/app/assets/javascripts/templates/help-modal.html.haml @@ -0,0 +1,9 @@ +.row.help-icon + .small-12.text-center + %i.ofn-i_013-help +.row.help-text + .small-12.columns.text-center + {{ helpText }} +.row.text-center + %button.primary.small{ ng: { click: '$close()' } } + = t(:ok) diff --git a/app/assets/stylesheets/darkswarm/help-modal.css.scss b/app/assets/stylesheets/darkswarm/help-modal.css.scss new file mode 100644 index 0000000000..c26b17edc2 --- /dev/null +++ b/app/assets/stylesheets/darkswarm/help-modal.css.scss @@ -0,0 +1,9 @@ +.help-modal { + .help-text { + font-size: 1rem; + margin: 20px 0px; + } + .help-icon { + font-size: 4rem; + } +} From 7db7084008106dd3aa80c4c786d0495107b992d2 Mon Sep 17 00:00:00 2001 From: Rob Harrington Date: Fri, 22 Jun 2018 14:05:05 +1000 Subject: [PATCH 14/71] Use help-modal to display help text for authorised shops Also updated the text slightly to make it more clear when the purpose of authorised shops are --- app/views/spree/users/_cards.html.haml | 2 +- config/locales/en.yml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/app/views/spree/users/_cards.html.haml b/app/views/spree/users/_cards.html.haml index 8929aac791..180dc7d478 100644 --- a/app/views/spree/users/_cards.html.haml +++ b/app/views/spree/users/_cards.html.haml @@ -17,6 +17,6 @@ .authorised_shops{ ng: { controller: 'AuthorisedShopsCtrl', hide: 'CreditCard.visible' } } %h3 = t('.authorised_shops') - %button.button.secondary.tiny.help-btn.ng-scope{ :popover => t('.authorised_shops_popover'), "popover-placement" => 'right' } + %button.button.secondary.tiny.help-btn.ng-scope{ "help-modal" => t('.authorised_shops_popover') } %i.ofn-i_013-help = render 'authorised_shops' diff --git a/config/locales/en.yml b/config/locales/en.yml index 597dd60c36..aaec61c99a 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -2742,6 +2742,6 @@ See the %{link} to find out more about %{sitename}'s features and to start using delete?: Delete? cards: authorised_shops: Authorised Shops - authorised_shops_popover: This is a list of shops which are permitted to charge your default credit card for OFN services (eg. subscriptions). Your card details will be kept secure and will not be shared with shop owners. + authorised_shops_popover: This is the list of shops which are permitted to charge your default credit card for any subscriptions (ie. repeating orders) you may have. Your card details will be kept secure and will not be shared with shop owners. You will always be notified when you are charged. localized_number: invalid_format: has an invalid format. Please enter a number. From d1d9c5a092d91b84e2f64dc7e6c1b5a237993f17 Mon Sep 17 00:00:00 2001 From: Rob Harrington Date: Fri, 22 Jun 2018 14:12:41 +1000 Subject: [PATCH 15/71] Add help button to saved cards list on account page Describes the purpose of the default card --- app/views/spree/users/_cards.html.haml | 6 +++++- config/locales/en.yml | 1 + 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/app/views/spree/users/_cards.html.haml b/app/views/spree/users/_cards.html.haml index 180dc7d478..6f7d13a06a 100644 --- a/app/views/spree/users/_cards.html.haml +++ b/app/views/spree/users/_cards.html.haml @@ -2,7 +2,11 @@ .credit_cards{"ng-controller" => "CreditCardsCtrl"} .row .small-12.medium-6.columns - %h3= t(:saved_cards) + %h3 + = t(:saved_cards) + %button.button.secondary.tiny.help-btn.ng-scope{ "help-modal" => t('.saved_cards_popover') } + %i.ofn-i_013-help + .saved_cards{ ng: { show: 'savedCreditCards.length > 0' } } = render 'saved_cards' .no_cards{ ng: { hide: 'savedCreditCards.length > 0' } } diff --git a/config/locales/en.yml b/config/locales/en.yml index aaec61c99a..8a34f8b9ff 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -2743,5 +2743,6 @@ See the %{link} to find out more about %{sitename}'s features and to start using cards: authorised_shops: Authorised Shops authorised_shops_popover: This is the list of shops which are permitted to charge your default credit card for any subscriptions (ie. repeating orders) you may have. Your card details will be kept secure and will not be shared with shop owners. You will always be notified when you are charged. + saved_cards_popover: This is the list of cards you have opted to save for later use. Your 'default' will be selected automatically when you checkout an order, and can be charged by any shops you have allowed to do so (see right). localized_number: invalid_format: has an invalid format. Please enter a number. From 5e6291bce36de8315a8fb6ee60e3e9dadf796944 Mon Sep 17 00:00:00 2001 From: Rob Harrington Date: Fri, 22 Jun 2018 15:31:32 +1000 Subject: [PATCH 16/71] Don't request customers if list is already populated --- app/assets/javascripts/darkswarm/services/customers.js.coffee | 1 + 1 file changed, 1 insertion(+) diff --git a/app/assets/javascripts/darkswarm/services/customers.js.coffee b/app/assets/javascripts/darkswarm/services/customers.js.coffee index cf5c56563a..fe2c862c37 100644 --- a/app/assets/javascripts/darkswarm/services/customers.js.coffee +++ b/app/assets/javascripts/darkswarm/services/customers.js.coffee @@ -4,6 +4,7 @@ angular.module("Darkswarm").factory 'Customers', (Customer) -> byID: {} index: (params={}) -> + return @all if @all.length Customer.index params, (data) => @load(data) @all From 0d9b03b0660ad32dcdd3dda217be483e56468bd1 Mon Sep 17 00:00:00 2001 From: luisramos0 Date: Fri, 22 Jun 2018 11:42:50 +0100 Subject: [PATCH 17/71] Hide postcode - not necessary as passed from billing address and wrecks mobile UX --- .../javascripts/darkswarm/directives/stripe_elements.js.coffee | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/assets/javascripts/darkswarm/directives/stripe_elements.js.coffee b/app/assets/javascripts/darkswarm/directives/stripe_elements.js.coffee index d325b6f962..d88e0868a6 100644 --- a/app/assets/javascripts/darkswarm/directives/stripe_elements.js.coffee +++ b/app/assets/javascripts/darkswarm/directives/stripe_elements.js.coffee @@ -10,7 +10,7 @@ Darkswarm.directive "stripeElements", ($injector, StripeElements) -> stripe = $injector.get('stripeObject') card = stripe.elements().create 'card', - hidePostalCode: false + hidePostalCode: true style: base: fontFamily: "Roboto, Arial, sans-serif" From ddb9ae1140a9df5d4e4f2a204595c81ff9d77060 Mon Sep 17 00:00:00 2001 From: Rob Harrington Date: Fri, 22 Jun 2018 15:34:17 +1000 Subject: [PATCH 18/71] Load all shops that a user is associated with as a customer Regardless of the presence of an order --- app/controllers/api/customers_controller.rb | 2 +- app/helpers/injection_helper.rb | 3 ++- app/models/customer.rb | 5 +++++ spec/controllers/api/customers_controller_spec.rb | 12 ++++++++++++ 4 files changed, 20 insertions(+), 2 deletions(-) diff --git a/app/controllers/api/customers_controller.rb b/app/controllers/api/customers_controller.rb index cbbe4ce35f..e983b372c9 100644 --- a/app/controllers/api/customers_controller.rb +++ b/app/controllers/api/customers_controller.rb @@ -1,7 +1,7 @@ module Api class CustomersController < BaseController def index - @customers = current_api_user.customers + @customers = current_api_user.customers.of_regular_shops render json: @customers, each_serializer: CustomerSerializer end diff --git a/app/helpers/injection_helper.rb b/app/helpers/injection_helper.rb index ea5589abe6..f4e857607e 100644 --- a/app/helpers/injection_helper.rb +++ b/app/helpers/injection_helper.rb @@ -69,7 +69,8 @@ module InjectionHelper end def inject_shops - shops = Enterprise.where(id: @orders.pluck(:distributor_id).uniq) + customers = spree_current_user.customers.of_regular_shops + shops = Enterprise.where(id: @orders.pluck(:distributor_id).uniq | customers.pluck(:enterprise_id)) inject_json_ams "shops", shops.all, Api::ShopForOrdersSerializer end diff --git a/app/models/customer.rb b/app/models/customer.rb index dfddd90c80..0ec6f52262 100644 --- a/app/models/customer.rb +++ b/app/models/customer.rb @@ -23,6 +23,11 @@ class Customer < ActiveRecord::Base scope :of, ->(enterprise) { where(enterprise_id: enterprise) } + scope :of_regular_shops, lambda { + next scoped unless Spree::Config.accounts_distributor_id + where('enterprise_id <> ?', Spree::Config.accounts_distributor_id) + } + before_create :associate_user private diff --git a/spec/controllers/api/customers_controller_spec.rb b/spec/controllers/api/customers_controller_spec.rb index f6c8f4e0a5..360037cc90 100644 --- a/spec/controllers/api/customers_controller_spec.rb +++ b/spec/controllers/api/customers_controller_spec.rb @@ -23,6 +23,18 @@ module Api expect(json_response.length).to eq 1 expect(json_response.first[:id]).to eq customer1.id end + + context "when the accounts distributor id has been set" do + before do + Spree::Config.set(accounts_distributor_id: customer1.enterprise.id) + end + + it "ignores the customer for that enterprise (if it exists)" do + spree_get :index + expect(response.status).to eq 200 + expect(json_response.length).to eq 0 + end + end end describe "#update" do From 792701297b2be66bdfc74dde3ca59f0dd8f8a493 Mon Sep 17 00:00:00 2001 From: Maikel Linke Date: Wed, 27 Jun 2018 11:12:54 +1000 Subject: [PATCH 19/71] Remove unused scope Enterprise.active_distributors Working on the Spree upgrade, we found that this scope is using the soon obsolete column `spree_products.count_on_hand`. Trying to measure the impact of changing this scope, I couldn't find any use of it. There is a variable called `active_distributors` used when serialising enterprises, but that variable is initialised with `Enterprise.distributors_with_active_order_cycles.ready_for_checkout`, not using the `active_distributors` scope. See also: https://github.com/openfoodfoundation/openfoodnetwork/issues/2013 --- app/models/enterprise.rb | 6 ----- spec/models/enterprise_spec.rb | 42 ---------------------------------- 2 files changed, 48 deletions(-) diff --git a/app/models/enterprise.rb b/app/models/enterprise.rb index 97855812ef..2a7a69df0a 100644 --- a/app/models/enterprise.rb +++ b/app/models/enterprise.rb @@ -129,12 +129,6 @@ class Enterprise < ActiveRecord::Base joins('LEFT OUTER JOIN exchange_variants ON (exchange_variants.exchange_id = exchanges.id)'). joins('LEFT OUTER JOIN spree_variants ON (spree_variants.id = exchange_variants.variant_id)') - scope :active_distributors, lambda { - with_distributed_products_outer.with_order_cycles_as_distributor_outer. - where('(product_distributions.product_id IS NOT NULL AND spree_products.deleted_at IS NULL AND spree_products.available_on <= ? AND spree_products.count_on_hand > 0) OR (order_cycles.id IS NOT NULL AND order_cycles.orders_open_at <= ? AND order_cycles.orders_close_at >= ?)', Time.zone.now, Time.zone.now, Time.zone.now). - select('DISTINCT enterprises.*') - } - scope :distributors_with_active_order_cycles, lambda { with_order_cycles_as_distributor_outer. merge(OrderCycle.active). diff --git a/spec/models/enterprise_spec.rb b/spec/models/enterprise_spec.rb index 8fc1b57a0b..54ec420b36 100644 --- a/spec/models/enterprise_spec.rb +++ b/spec/models/enterprise_spec.rb @@ -321,48 +321,6 @@ describe Enterprise do end end - describe "active_distributors" do - it "finds active distributors by product distributions" do - d = create(:distributor_enterprise) - create(:product, :distributors => [d]) - Enterprise.active_distributors.should == [d] - end - - it "doesn't show distributors of deleted products" do - d = create(:distributor_enterprise) - create(:product, :distributors => [d], :deleted_at => Time.zone.now) - Enterprise.active_distributors.should be_empty - end - - it "doesn't show distributors of unavailable products" do - d = create(:distributor_enterprise) - create(:product, :distributors => [d], :available_on => 1.week.from_now) - Enterprise.active_distributors.should be_empty - end - - it "doesn't show distributors of out of stock products" do - d = create(:distributor_enterprise) - create(:product, :distributors => [d], :on_hand => 0) - Enterprise.active_distributors.should be_empty - end - - it "finds active distributors by order cycles" do - s = create(:supplier_enterprise) - d = create(:distributor_enterprise) - p = create(:product) - create(:simple_order_cycle, suppliers: [s], distributors: [d], variants: [p.master]) - Enterprise.active_distributors.should == [d] - end - - it "doesn't show distributors from inactive order cycles" do - s = create(:supplier_enterprise) - d = create(:distributor_enterprise) - p = create(:product) - create(:simple_order_cycle, suppliers: [s], distributors: [d], variants: [p.master], orders_open_at: 1.week.from_now, orders_close_at: 2.weeks.from_now) - Enterprise.active_distributors.should be_empty - end - end - describe "supplying_variant_in" do it "finds producers by supply of master variant" do s = create(:supplier_enterprise) From 306bfa1944c6c94821f6aef2477c8a41cb162a9c Mon Sep 17 00:00:00 2001 From: Maikel Linke Date: Wed, 27 Jun 2018 11:25:44 +1000 Subject: [PATCH 20/71] Remove unused method enterprise method `Enterprise.has_supplied_products_on_hand?` is not used anywhere. --- app/models/enterprise.rb | 4 ---- spec/models/enterprise_spec.rb | 20 -------------------- 2 files changed, 24 deletions(-) diff --git a/app/models/enterprise.rb b/app/models/enterprise.rb index 2a7a69df0a..ea1724f20a 100644 --- a/app/models/enterprise.rb +++ b/app/models/enterprise.rb @@ -193,10 +193,6 @@ class Enterprise < ActiveRecord::Base end end - def has_supplied_products_on_hand? - self.supplied_products.where('count_on_hand > 0').present? - end - def to_param permalink end diff --git a/spec/models/enterprise_spec.rb b/spec/models/enterprise_spec.rb index 54ec420b36..73cd3e1c5b 100644 --- a/spec/models/enterprise_spec.rb +++ b/spec/models/enterprise_spec.rb @@ -489,26 +489,6 @@ describe Enterprise do end end - describe "has_supplied_products_on_hand?" do - before :each do - @supplier = create(:supplier_enterprise) - end - - it "returns false when no products" do - @supplier.should_not have_supplied_products_on_hand - end - - it "returns false when the product is out of stock" do - create(:product, :supplier => @supplier, :on_hand => 0) - @supplier.should_not have_supplied_products_on_hand - end - - it "returns true when the product is in stock" do - create(:product, :supplier => @supplier, :on_hand => 1) - @supplier.should have_supplied_products_on_hand - end - end - describe "finding variants distributed by the enterprise" do it "finds master and other variants" do d = create(:distributor_enterprise) From b7de80dd7f5a0d85bc8d6033af6c19d083e4e901 Mon Sep 17 00:00:00 2001 From: Maikel Linke Date: Wed, 27 Jun 2018 11:41:55 +1000 Subject: [PATCH 21/71] Remove unused count_on_hand setting from spec This spec doesn't need to set the product's `count_on_hand`. The product comes with a count of 3, but what matters is the count of variants. --- spec/controllers/cart_controller_spec.rb | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/spec/controllers/cart_controller_spec.rb b/spec/controllers/cart_controller_spec.rb index f7f48c3a06..5d939b6254 100644 --- a/spec/controllers/cart_controller_spec.rb +++ b/spec/controllers/cart_controller_spec.rb @@ -6,11 +6,7 @@ module OpenFoodNetwork render_views let(:user) { FactoryBot.create(:user) } - let(:product1) do - p1 = FactoryBot.create(:product) - p1.update_column(:count_on_hand, 10) - p1 - end + let(:product1) { FactoryBot.create(:product) } let(:cart) { Cart.create(user: user) } let(:distributor) { FactoryBot.create(:distributor_enterprise) } From c496d0f14d77a769aa37e70c2b4963b891a7d3d4 Mon Sep 17 00:00:00 2001 From: Rob Harrington Date: Thu, 10 May 2018 17:49:13 +1000 Subject: [PATCH 22/71] Allow credit owed on payments made via stripe to be refunded via the admin section --- app/assets/stylesheets/admin/icons.css.scss | 1 + app/models/spree/gateway/stripe_connect.rb | 5 ++ .../spree/admin/payments_controller_spec.rb | 54 +++++++++++++++++++ 3 files changed, 60 insertions(+) diff --git a/app/assets/stylesheets/admin/icons.css.scss b/app/assets/stylesheets/admin/icons.css.scss index e7737ce8e9..9bd900d083 100644 --- a/app/assets/stylesheets/admin/icons.css.scss +++ b/app/assets/stylesheets/admin/icons.css.scss @@ -1,3 +1,4 @@ @import 'plugins/font-awesome'; .icon-refund:before { @extend .icon-ok:before } +.icon-credit:before { @extend .icon-ok:before } diff --git a/app/models/spree/gateway/stripe_connect.rb b/app/models/spree/gateway/stripe_connect.rb index 6efd2f79ef..e9e70671b2 100644 --- a/app/models/spree/gateway/stripe_connect.rb +++ b/app/models/spree/gateway/stripe_connect.rb @@ -43,6 +43,11 @@ module Spree provider.void(response_code, gateway_options) end + def credit(money, _creditcard, response_code, gateway_options) + gateway_options[:stripe_account] = stripe_account_id + provider.refund(money, response_code, gateway_options) + end + def create_profile(payment) return unless payment.source.gateway_customer_profile_id.nil? diff --git a/spec/controllers/spree/admin/payments_controller_spec.rb b/spec/controllers/spree/admin/payments_controller_spec.rb index 50edfc468a..7bf9e6e522 100644 --- a/spec/controllers/spree/admin/payments_controller_spec.rb +++ b/spec/controllers/spree/admin/payments_controller_spec.rb @@ -66,5 +66,59 @@ describe Spree::Admin::PaymentsController, type: :controller do end end end + + context "requesting a partial credit on a payment" do + let(:params) { { id: payment.id, order_id: order.number, e: :credit } } + + # Required for the respond override in the controller decorator to work + before { @request.env['HTTP_REFERER'] = spree.admin_order_payments_url(payment) } + + context "that was processed by stripe" do + let!(:payment_method) { create(:stripe_payment_method, distributors: [shop], preferred_enterprise_id: shop.id) } + let!(:payment) { create(:payment, order: order, state: 'completed', payment_method: payment_method, response_code: 'ch_1a2b3c', amount: order.total + 5) } + + + before do + allow(Stripe).to receive(:api_key) { "sk_test_12345" } + end + + context "where the request succeeds" do + before do + stub_request(:post, "https://sk_test_12345:@api.stripe.com/v1/charges/ch_1a2b3c/refunds"). + to_return(:status => 200, :body => JSON.generate(id: 're_123', object: 'refund', status: 'succeeded') ) + end + + it "partially refunds the payment" do + order.reload + expect(order.payment_total).to eq order.total + 5 + expect(order.outstanding_balance).to eq(-5) + spree_put :fire, params + expect(payment.reload.state).to eq 'completed' + order.reload + expect(order.payment_total).to eq order.total + expect(order.outstanding_balance).to eq 0 + end + end + + context "where the request fails" do + before do + stub_request(:post, "https://sk_test_12345:@api.stripe.com/v1/charges/ch_1a2b3c/refunds"). + to_return(:status => 200, :body => JSON.generate(error: { message: "Bup-bow!"}) ) + end + + it "does not void the payment" do + order.reload + expect(order.payment_total).to eq order.total + 5 + expect(order.outstanding_balance).to eq(-5) + spree_put :fire, params + expect(payment.reload.state).to eq 'completed' + order.reload + expect(order.payment_total).to eq order.total + 5 + expect(order.outstanding_balance).to eq -5 + expect(flash[:error]).to eq "Bup-bow!" + end + end + end + end end end From 2da623436258b66e6cd0b6375fb5a1d181413653 Mon Sep 17 00:00:00 2001 From: Rob Harrington Date: Thu, 7 Jun 2018 15:50:28 +1000 Subject: [PATCH 23/71] Add spec for StripeGateway#refund --- .../spree/gateway/stripe_connect_spec.rb | 31 +++++++++++++++---- 1 file changed, 25 insertions(+), 6 deletions(-) diff --git a/spec/models/spree/gateway/stripe_connect_spec.rb b/spec/models/spree/gateway/stripe_connect_spec.rb index 4e39a7812e..eb9bacdd2c 100644 --- a/spec/models/spree/gateway/stripe_connect_spec.rb +++ b/spec/models/spree/gateway/stripe_connect_spec.rb @@ -2,10 +2,11 @@ require 'spec_helper' describe Spree::Gateway::StripeConnect, type: :model do let(:provider) do - double('provider').tap do |p| - p.stub(:purchase) - p.stub(:authorize) - p.stub(:capture) + instance_double(ActiveMerchant::Billing::StripeGateway).tap do |p| + allow(p).to receive(:purchase) + allow(p).to receive(:authorize) + allow(p).to receive(:capture) + allow(p).to receive(:refund) end end @@ -14,8 +15,8 @@ describe Spree::Gateway::StripeConnect, type: :model do before do allow(Stripe).to receive(:api_key) { "sk_test_123456" } allow(subject).to receive(:stripe_account_id) { stripe_account_id } - subject.stub(:options_for_purchase_or_auth).and_return(['money', 'cc', 'opts']) - subject.stub(:provider).and_return provider + allow(subject).to receive(:options_for_purchase_or_auth).and_return(['money', 'cc', 'opts']) + allow(subject).to receive(:provider).and_return provider end describe "#token_from_card_profile_ids" do @@ -70,4 +71,22 @@ describe Spree::Gateway::StripeConnect, type: :model do expect(subject.send(:tokenize_instance_customer_card, customer_id, card_id)).to eq token_mock[:id] end end + + describe "#credit" do + let(:gateway_options) { { some: 'option' } } + let(:money) { double(:money) } + let(:response_code) { double(:response_code) } + + before do + subject.credit(money, double(:creditcard), response_code, gateway_options) + end + + it "delegates to ActiveMerchant::Billing::StripeGateway#refund" do + expect(provider).to have_received(:refund) + end + + it "adds the stripe_account to the gateway options hash" do + expect(provider).to have_received(:refund).with(money, response_code, hash_including(stripe_account: stripe_account_id)) + end + end end From 82e3016a2682c3885afbfd708270bb0c8e49c8ec Mon Sep 17 00:00:00 2001 From: Rob Harrington Date: Thu, 7 Jun 2018 15:56:38 +1000 Subject: [PATCH 24/71] Add comment to StripeGateway wrapper methods indicating that they are named by Spree --- app/models/spree/gateway/stripe_connect.rb | 3 +++ 1 file changed, 3 insertions(+) diff --git a/app/models/spree/gateway/stripe_connect.rb b/app/models/spree/gateway/stripe_connect.rb index e9e70671b2..2f192eb655 100644 --- a/app/models/spree/gateway/stripe_connect.rb +++ b/app/models/spree/gateway/stripe_connect.rb @@ -31,6 +31,7 @@ module Spree StripeAccount.find_by_enterprise_id(preferred_enterprise_id).andand.stripe_user_id end + # NOTE: the name of this method is determined by Spree::Payment::Processing def purchase(money, creditcard, gateway_options) provider.purchase(*options_for_purchase_or_auth(money, creditcard, gateway_options)) rescue Stripe::StripeError => e @@ -38,11 +39,13 @@ module Spree failed_activemerchant_billing_response(e.message) end + # NOTE: the name of this method is determined by Spree::Payment::Processing def void(response_code, _creditcard, gateway_options) gateway_options[:stripe_account] = stripe_account_id provider.void(response_code, gateway_options) end + # NOTE: the name of this method is determined by Spree::Payment::Processing def credit(money, _creditcard, response_code, gateway_options) gateway_options[:stripe_account] = stripe_account_id provider.refund(money, response_code, gateway_options) From 09534b41e9a806daf91c5633d0b03e009e297b30 Mon Sep 17 00:00:00 2001 From: Frank West Date: Fri, 15 Jun 2018 11:07:39 -0700 Subject: [PATCH 25/71] Remove taxon when primary taxon is changed We are adding taxons to the product as you change the primary taxon. However we never remove the previous primary taxon so it forces the user to update the taxons manually. This can be a big problem if you are bulk updating products. We now remove the taxon that matches the previously set primary taxon. --- app/models/spree/product_decorator.rb | 6 ++++++ spec/models/spree/product_spec.rb | 16 ++++++++++++++++ 2 files changed, 22 insertions(+) diff --git a/app/models/spree/product_decorator.rb b/app/models/spree/product_decorator.rb index b9d45c4039..18220aaafd 100644 --- a/app/models/spree/product_decorator.rb +++ b/app/models/spree/product_decorator.rb @@ -38,6 +38,7 @@ Spree::Product.class_eval do before_validation :sanitize_permalink before_save :add_primary_taxon_to_taxons after_touch :touch_distributors + after_save :remove_previous_primary_taxon_from_taxons after_save :ensure_standard_variant after_save :update_units after_save :refresh_products_cache @@ -245,6 +246,11 @@ Spree::Product.class_eval do taxons << primary_taxon unless taxons.include? primary_taxon end + def remove_previous_primary_taxon_from_taxons + return unless primary_taxon_id_changed? && primary_taxon_id_was + taxons.destroy(primary_taxon_id_was) + end + def self.all_variant_unit_option_types Spree::OptionType.where('name LIKE ?', 'unit_%%') end diff --git a/spec/models/spree/product_spec.rb b/spec/models/spree/product_spec.rb index bf443bf2b9..f90de704e2 100644 --- a/spec/models/spree/product_spec.rb +++ b/spec/models/spree/product_spec.rb @@ -193,6 +193,22 @@ module Spree expect { product.delete }.to change { distributor.reload.updated_at } end end + + it "adds the primary taxon to the product's taxon list" do + taxon = create(:taxon) + product = create(:product, primary_taxon: taxon) + + expect(product.taxons).to include(taxon) + end + + it "removes the previous primary taxon from the taxon list" do + original_taxon = create(:taxon) + product = create(:product, primary_taxon: original_taxon) + product.primary_taxon = create(:taxon) + product.save! + + expect(product.taxons).not_to include(original_taxon) + end end describe "scopes" do From fc2844a3d5e1fcf97e389c2ef4d76382f8e2b556 Mon Sep 17 00:00:00 2001 From: Rob Harrington Date: Fri, 4 May 2018 11:38:35 +1000 Subject: [PATCH 26/71] Add default_card method to user model --- app/models/spree/user_decorator.rb | 4 ++++ spec/models/spree/user_spec.rb | 28 ++++++++++++++++++++++++++++ 2 files changed, 32 insertions(+) diff --git a/app/models/spree/user_decorator.rb b/app/models/spree/user_decorator.rb index 3877428c86..8ae76a11b4 100644 --- a/app/models/spree/user_decorator.rb +++ b/app/models/spree/user_decorator.rb @@ -73,6 +73,10 @@ Spree.user_class.class_eval do owned_enterprises(:reload).size < enterprise_limit end + def default_card + credit_cards.where(is_default: true).first + end + private def limit_owned_enterprises diff --git a/spec/models/spree/user_spec.rb b/spec/models/spree/user_spec.rb index 39a1ce530e..3cd4da93af 100644 --- a/spec/models/spree/user_spec.rb +++ b/spec/models/spree/user_spec.rb @@ -126,4 +126,32 @@ describe Spree.user_class do end end end + + describe "default_card" do + let(:user) { create(:user) } + + context "when the user has no credit cards" do + it "returns nil" do + expect(user.default_card).to be nil + end + end + + context "when the user has one credit card" do + let!(:card) { create(:credit_card, user: user) } + + it "should be assigned as the default and be returned" do + expect(card.reload.is_default).to be true + expect(user.default_card.id).to be card.id + end + end + + context "when the user has more than one card" do + let!(:non_default_card) { create(:credit_card, user: user) } + let!(:default_card) { create(:credit_card, user: user, is_default: true) } + + it "returns the card which is specified as the default" do + expect(user.default_card.id).to be default_card.id + end + end + end end From 45895e9924f8344658047c3decb71dd47d3020c9 Mon Sep 17 00:00:00 2001 From: Rob Harrington Date: Fri, 4 May 2018 11:39:26 +1000 Subject: [PATCH 27/71] Use the user's default card to pay for subcriptions --- lib/open_food_network/subscription_payment_updater.rb | 2 +- .../open_food_network/subscription_payment_updater_spec.rb | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/open_food_network/subscription_payment_updater.rb b/lib/open_food_network/subscription_payment_updater.rb index 2a166def68..eb33aaf2f2 100644 --- a/lib/open_food_network/subscription_payment_updater.rb +++ b/lib/open_food_network/subscription_payment_updater.rb @@ -47,7 +47,7 @@ module OpenFoodNetwork end def saved_credit_card - order.subscription.credit_card + order.user.default_card end def errors_present? diff --git a/spec/lib/open_food_network/subscription_payment_updater_spec.rb b/spec/lib/open_food_network/subscription_payment_updater_spec.rb index 5fb76a0c86..8d590f59b7 100644 --- a/spec/lib/open_food_network/subscription_payment_updater_spec.rb +++ b/spec/lib/open_food_network/subscription_payment_updater_spec.rb @@ -151,7 +151,7 @@ module OpenFoodNetwork let!(:payment) { create(:payment, source: nil) } before { allow(updater).to receive(:payment) { payment } } - context "when no credit card is specified by the subscription" do + context "when no saved credit card is found" do before { allow(updater).to receive(:saved_credit_card) { nil } } it "returns false and down not update the payment source" do @@ -161,7 +161,7 @@ module OpenFoodNetwork end end - context "when a credit card is specified by the subscription" do + context "when a saved credit card is found" do let(:credit_card) { create(:credit_card) } before { allow(updater).to receive(:saved_credit_card) { credit_card } } From cf8ca1f8c10e3a0dbb81daf71e7cba31a8b7843b Mon Sep 17 00:00:00 2001 From: Rob Harrington Date: Fri, 4 May 2018 12:06:03 +1000 Subject: [PATCH 28/71] Add show action to Admin::CustomersController --- app/controllers/admin/customers_controller.rb | 4 +++ app/models/spree/ability_decorator.rb | 2 +- config/routes.rb | 2 +- .../admin/customers_controller_spec.rb | 33 +++++++++++++++++++ 4 files changed, 39 insertions(+), 2 deletions(-) diff --git a/app/controllers/admin/customers_controller.rb b/app/controllers/admin/customers_controller.rb index 6ff6278b10..d57340b653 100644 --- a/app/controllers/admin/customers_controller.rb +++ b/app/controllers/admin/customers_controller.rb @@ -23,6 +23,10 @@ module Admin end end + def show + render_as_json @customer + end + def create @customer = Customer.new(params[:customer]) if user_can_create_customer? diff --git a/app/models/spree/ability_decorator.rb b/app/models/spree/ability_decorator.rb index daf8161142..f5d05c9a3d 100644 --- a/app/models/spree/ability_decorator.rb +++ b/app/models/spree/ability_decorator.rb @@ -257,7 +257,7 @@ class AbilityDecorator can [:admin, :index, :customers, :group_buys, :bulk_coop, :sales_tax, :payments, :orders_and_distributors, :orders_and_fulfillment, :products_and_inventory, :order_cycle_management, :xero_invoices], :report can [:create], Customer - can [:admin, :index, :update, :destroy, :addresses, :cards], Customer, enterprise_id: Enterprise.managed_by(user).pluck(:id) + can [:admin, :index, :update, :destroy, :addresses, :cards, :show], Customer, enterprise_id: Enterprise.managed_by(user).pluck(:id) can [:admin, :new, :index], Subscription can [:create, :edit, :update, :cancel, :pause, :unpause], Subscription do |subscription| user.enterprises.include?(subscription.shop) diff --git a/config/routes.rb b/config/routes.rb index ba4da8ae2e..0ab7fe1a3c 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -147,7 +147,7 @@ Openfoodnetwork::Application.routes.draw do resources :inventory_items, only: [:create, :update] - resources :customers, only: [:index, :create, :update, :destroy] do + resources :customers, only: [:index, :create, :update, :destroy, :show] do get :addresses, on: :member get :cards, on: :member end diff --git a/spec/controllers/admin/customers_controller_spec.rb b/spec/controllers/admin/customers_controller_spec.rb index 41a639c4b3..bd9ff1f0f7 100644 --- a/spec/controllers/admin/customers_controller_spec.rb +++ b/spec/controllers/admin/customers_controller_spec.rb @@ -235,4 +235,37 @@ describe Admin::CustomersController, type: :controller do end end end + + describe "show" do + let(:enterprise) { create(:distributor_enterprise) } + let(:another_enterprise) { create(:distributor_enterprise) } + + context "json" do + let!(:customer) { create(:customer, enterprise: enterprise) } + + context "where I manage the customer's enterprise" do + render_views + + before do + controller.stub spree_current_user: enterprise.owner + end + + it "renders the customer as json" do + spree_get :show, format: :json, id: customer.id + expect(JSON.parse(response.body)["id"]).to eq customer.id + end + end + + context "where I don't manage the customer's enterprise" do + before do + controller.stub spree_current_user: another_enterprise.owner + end + + it "prevents me from updating the customer" do + spree_get :show, format: :json, id: customer.id + expect(response).to redirect_to spree.unauthorized_path + end + end + end + end end From 21c24eb69b9237facbff1053b2f05ce718a8eb02 Mon Sep 17 00:00:00 2001 From: Rob Harrington Date: Wed, 9 May 2018 11:35:13 +1000 Subject: [PATCH 29/71] Validate presence and auth of default card for customer --- app/services/subscription_validator.rb | 9 ++++---- spec/services/subscription_validator_spec.rb | 23 ++++++++++---------- 2 files changed, 16 insertions(+), 16 deletions(-) diff --git a/app/services/subscription_validator.rb b/app/services/subscription_validator.rb index d7dcbd39f3..14cae211a6 100644 --- a/app/services/subscription_validator.rb +++ b/app/services/subscription_validator.rb @@ -76,10 +76,11 @@ class SubscriptionValidator end def credit_card_ok? - return unless payment_method.andand.type == "Spree::Gateway::StripeConnect" - return errors.add(:credit_card, :blank) unless credit_card_id - return if customer.andand.user.andand.credit_card_ids.andand.include? credit_card_id - errors.add(:credit_card, :not_available) + return unless customer && payment_method + return unless payment_method.type == "Spree::Gateway::StripeConnect" + return errors.add(:credit_card, :charges_not_allowed) unless customer.allow_charges + return if customer.user.andand.default_card.present? + errors.add(:credit_card, :no_default_card) end def subscription_line_items_present? diff --git a/spec/services/subscription_validator_spec.rb b/spec/services/subscription_validator_spec.rb index 59db5415b8..43ab894aa6 100644 --- a/spec/services/subscription_validator_spec.rb +++ b/spec/services/subscription_validator_spec.rb @@ -332,10 +332,10 @@ describe SubscriptionValidator do context "when using the StripeConnect payment gateway" do let(:payment_method) { instance_double(Spree::PaymentMethod, type: "Spree::Gateway::StripeConnect") } - before { expect(subscription).to receive(:credit_card_id).at_least(:once) { credit_card_id } } + before { expect(subscription).to receive(:customer).at_least(:once) { customer } } - context "when a credit card is not present" do - let(:credit_card_id) { nil } + context "when the customer does not allow charges" do + let(:customer) { instance_double(Customer, allow_charges: false) } it "adds an error and returns false" do expect(validator.valid?).to be false @@ -343,12 +343,11 @@ describe SubscriptionValidator do end end - context "when a credit card is present" do - let(:credit_card_id) { 12 } - before { expect(subscription).to receive(:customer).at_least(:once) { customer } } + context "when the customer allows charges" do + let(:customer) { instance_double(Customer, allow_charges: true) } context "and the customer is not associated with a user" do - let(:customer) { instance_double(Customer, user: nil) } + before { allow(customer).to receive(:user) { nil } } it "adds an error and returns false" do expect(validator.valid?).to be false @@ -357,10 +356,10 @@ describe SubscriptionValidator do end context "and the customer is associated with a user" do - let(:customer) { instance_double(Customer, user: user) } + before { expect(customer).to receive(:user).once { user } } - context "and the user has no credit cards which match that specified" do - let(:user) { instance_double(Spree::User, credit_card_ids: [1, 2, 3, 4]) } + context "and the user has no default card set" do + let(:user) { instance_double(Spree::User, default_card: nil) } it "adds an error and returns false" do expect(validator.valid?).to be false @@ -368,8 +367,8 @@ describe SubscriptionValidator do end end - context "and the user has a credit card which matches that specified" do - let(:user) { instance_double(Spree::User, credit_card_ids: [1, 2, 3, 12]) } + context "and the user has a default card set" do + let(:user) { instance_double(Spree::User, default_card: 'some card') } it "returns true" do expect(validator.valid?).to be true From a03dd1e10c16f8df313b4c3649c2d1ff892b6d19 Mon Sep 17 00:00:00 2001 From: Rob Harrington Date: Wed, 9 May 2018 11:37:23 +1000 Subject: [PATCH 30/71] Serialize default card auth and presence for Customers --- app/serializers/api/admin/customer_serializer.rb | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/app/serializers/api/admin/customer_serializer.rb b/app/serializers/api/admin/customer_serializer.rb index 327ac97189..ea666f308f 100644 --- a/app/serializers/api/admin/customer_serializer.rb +++ b/app/serializers/api/admin/customer_serializer.rb @@ -1,5 +1,6 @@ class Api::Admin::CustomerSerializer < ActiveModel::Serializer attributes :id, :email, :enterprise_id, :user_id, :code, :tags, :tag_list, :name + attributes :allow_charges, :default_card_present? has_one :ship_address, serializer: Api::AddressSerializer has_one :bill_address, serializer: Api::AddressSerializer @@ -18,4 +19,9 @@ class Api::Admin::CustomerSerializer < ActiveModel::Serializer tag_rule_map || { text: tag, rules: nil } end end + + def default_card_present? + return unless object.user + object.user.default_card.present? + end end From c71a5ec0df7a766b7de80c046c778c75408d50ee Mon Sep 17 00:00:00 2001 From: Rob Harrington Date: Wed, 9 May 2018 11:39:56 +1000 Subject: [PATCH 31/71] Update subscription form to use new card validations for Stripe payment method --- .../controllers/details_controller.js.coffee | 52 ++++++++++--------- .../services/credit_card_resource.js.coffee | 5 -- .../services/customer_resource.js.coffee | 2 + .../admin/subscriptions/_details.html.haml | 11 ++-- config/locales/en.yml | 8 +-- spec/features/admin/subscriptions_spec.rb | 24 ++++----- 6 files changed, 50 insertions(+), 52 deletions(-) delete mode 100644 app/assets/javascripts/admin/subscriptions/services/credit_card_resource.js.coffee create mode 100644 app/assets/javascripts/admin/subscriptions/services/customer_resource.js.coffee diff --git a/app/assets/javascripts/admin/subscriptions/controllers/details_controller.js.coffee b/app/assets/javascripts/admin/subscriptions/controllers/details_controller.js.coffee index b3afc1c5d6..b4ace027dc 100644 --- a/app/assets/javascripts/admin/subscriptions/controllers/details_controller.js.coffee +++ b/app/assets/javascripts/admin/subscriptions/controllers/details_controller.js.coffee @@ -1,38 +1,42 @@ -angular.module("admin.subscriptions").controller "DetailsController", ($scope, $http, CreditCardResource, StatusMessage) -> +angular.module("admin.subscriptions").controller "DetailsController", ($scope, $http, CustomerResource, StatusMessage) -> $scope.cardRequired = false $scope.registerNextCallback 'details', -> $scope.subscription_form.$submitted = true - if $scope.subscription_details_form.$valid - $scope.subscription_form.$setPristine() - StatusMessage.clear() - $scope.setView('address') - else - StatusMessage.display 'failure', t('admin.subscriptions.details.invalid_error') + return unless $scope.validate() + $scope.subscription_form.$setPristine() + StatusMessage.clear() + $scope.setView('address') $scope.$watch "subscription.customer_id", (newValue, oldValue) -> return if !newValue? - $scope.loadAddresses(newValue) unless $scope.subscription.id? - $scope.loadCreditCards(newValue) + $scope.loadCustomer(newValue) unless $scope.subscription.id? $scope.$watch "subscription.payment_method_id", (newValue, oldValue) -> return if !newValue? paymentMethod = ($scope.paymentMethods.filter (pm) -> pm.id == newValue)[0] return unless paymentMethod? - if paymentMethod.type == "Spree::Gateway::StripeConnect" - $scope.cardRequired = true - else - $scope.cardRequired = false - $scope.subscription.credit_card_id = null + $scope.cardRequired = (paymentMethod.type == "Spree::Gateway::StripeConnect") + $scope.loadCustomer() if $scope.cardRequired && !$scope.customer - $scope.loadAddresses = (customer_id) -> - $http.get("/admin/customers/#{customer_id}/addresses") - .success (response) => - delete response.bill_address.id - delete response.ship_address.id - angular.extend($scope.subscription.bill_address, response.bill_address) - angular.extend($scope.subscription.ship_address, response.ship_address) - $scope.shipAddressFromBilling() unless response.ship_address.address1? + $scope.loadCustomer = -> + params = { id: $scope.subscription.customer_id } + $scope.customer = CustomerResource.get params, (response) -> + for address in ['bill_address','ship_address'] + return unless response[address] + delete response[address].id + return if $scope.subscription[address].address1? + angular.extend($scope.subscription[address], response[address]) + $scope.shipAddressFromBilling() unless response.ship_address?.address1? - $scope.loadCreditCards = (customer_id) -> - $scope.creditCards = CreditCardResource.index(customer_id: customer_id) + $scope.validate = -> + return true if $scope.subscription_details_form.$valid && $scope.creditCardOk() + StatusMessage.display 'failure', t('admin.subscriptions.details.invalid_error') + false + + $scope.creditCardOk = -> + return true unless $scope.cardRequired + return false unless $scope.customer + return false unless $scope.customer.allow_charges + return false unless $scope.customer.default_card_present + true diff --git a/app/assets/javascripts/admin/subscriptions/services/credit_card_resource.js.coffee b/app/assets/javascripts/admin/subscriptions/services/credit_card_resource.js.coffee deleted file mode 100644 index 0bb31cf3b0..0000000000 --- a/app/assets/javascripts/admin/subscriptions/services/credit_card_resource.js.coffee +++ /dev/null @@ -1,5 +0,0 @@ -angular.module("admin.subscriptions").factory 'CreditCardResource', ($resource) -> - resource = $resource '/admin/customers/:customer_id/cards.json', {}, - 'index': - method: 'GET' - isArray: true diff --git a/app/assets/javascripts/admin/subscriptions/services/customer_resource.js.coffee b/app/assets/javascripts/admin/subscriptions/services/customer_resource.js.coffee new file mode 100644 index 0000000000..56c999278e --- /dev/null +++ b/app/assets/javascripts/admin/subscriptions/services/customer_resource.js.coffee @@ -0,0 +1,2 @@ +angular.module("admin.subscriptions").factory 'CustomerResource', ($resource) -> + $resource '/admin/customers/:id.json' diff --git a/app/views/admin/subscriptions/_details.html.haml b/app/views/admin/subscriptions/_details.html.haml index 6fa57f41f4..7dedbf26f4 100644 --- a/app/views/admin/subscriptions/_details.html.haml +++ b/app/views/admin/subscriptions/_details.html.haml @@ -14,19 +14,16 @@ .error{ ng: { repeat: 'error in errors.schedule', show: 'subscription_details_form.schedule_id.$pristine'} } {{ error }} .row - .columns.alpha.field{ ng: { class: '{four: cardRequired, seven: !cardRequired}' } } + .seven.columns.alpha.field %label{ for: 'payment_method_id'} = t('admin.payment_method') %span.with-tip.icon-question-sign{ data: { powertip: "#{t('.allowed_payment_method_types_tip')}" } } %input.ofn-select2.fullwidth#payment_method_id{ name: 'payment_method_id', type: 'number', data: 'paymentMethods', required: true, placeholder: t('admin.choose'), ng: { model: 'subscription.payment_method_id' } } .error{ ng: { show: 'subscription_form.$submitted && subscription_details_form.payment_method_id.$error.required' } }= t(:error_required) .error{ ng: { repeat: 'error in errors.payment_method', show: 'subscription_details_form.payment_method_id.$pristine' } } {{ error }} - .three.columns.field{ ng: { show: 'cardRequired' } } - %label{ for: 'credit_card_id'}= t('.credit_card') - %input.ofn-select2.fullwidth#credit_card_id{ name: 'credit_card_id', type: 'number', data: 'creditCards', text: 'formatted', placeholder: t('admin.choose'), ng: { model: 'subscription.credit_card_id', required: "cardRequired" } } - .error{ ng: { show: 'creditCards.$promise && creditCards.$resolved && creditCards.length == 0' } }= t('.no_cards_available') - .error{ ng: { show: 'subscription_form.$submitted && subscription_details_form.credit_card_id.$error.required' } }= t(:error_required) - .error{ ng: { repeat: 'error in errors.credit_card', show: 'subscription_details_form.credit_card_id.$pristine' } } {{ error }} + .error{ ng: { show: 'cardRequired && customer.$promise && customer.$resolved && !customer.allow_charges' } }= t('.charges_not_allowed') + .error{ ng: { show: 'cardRequired && customer.$promise && customer.$resolved && customer.allow_charges && !customer.default_card_present' } }= t('.no_default_card') + .error{ ng: { repeat: 'error in errors.credit_card', show: 'subscription_details_form.payment_method_id.$pristine' } } {{ error }} .two.columns   .seven.columns.omega.field %label{ for: 'shipping_method_id'}= t('admin.shipping_method') diff --git a/config/locales/en.yml b/config/locales/en.yml index 8a34f8b9ff..87a6799143 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -100,8 +100,8 @@ en: shipping_method: not_available_to_shop: "is not available to %{shop}" credit_card: - not_available: "is not available" - blank: "is required" + charges_not_allowed: "charges are not allowed by this customer" + no_default_card: "not available for this customer" devise: confirmations: send_instructions: "You will receive an email with instructions about how to confirm your account in a few minutes." @@ -1025,7 +1025,9 @@ en: invalid_error: Oops! Please fill in all of the required fields... allowed_payment_method_types_tip: Only Cash and Stripe payment methods may be used at the moment credit_card: Credit Card - no_cards_available: No cards available + charges_not_allowed: Charges are not allowed by this customer + no_default_card: Customer has no cards available to charge + card_ok: Customer has a card available to charge loading_flash: loading: LOADING SUBSCRIPTIONS review: diff --git a/spec/features/admin/subscriptions_spec.rb b/spec/features/admin/subscriptions_spec.rb index ad115cbb5a..556164386a 100644 --- a/spec/features/admin/subscriptions_spec.rb +++ b/spec/features/admin/subscriptions_spec.rb @@ -124,9 +124,7 @@ feature 'Subscriptions' do let(:address) { create(:address) } let!(:customer_user) { create(:user) } let!(:credit_card1) { create(:credit_card, user: customer_user, cc_type: 'visa', last_digits: 1111, month: 10, year: 2030) } - let!(:credit_card2) { create(:credit_card, user: customer_user, cc_type: 'master', last_digits: 9999, month: 2, year: 2044) } - let!(:credit_card3) { create(:credit_card, cc_type: 'visa', last_digits: 5555, month: 6, year: 2066) } - let!(:customer) { create(:customer, enterprise: shop, bill_address: address, user: customer_user) } + let!(:customer) { create(:customer, enterprise: shop, bill_address: address, user: customer_user, allow_charges: true) } let!(:product1) { create(:product, supplier: shop) } let!(:product2) { create(:product, supplier: shop) } let!(:variant1) { create(:variant, product: product1, unit_value: '100', price: 12.00, option_values: []) } @@ -149,21 +147,14 @@ feature 'Subscriptions' do select2_select payment_method.name, from: 'payment_method_id' select2_select shipping_method.name, from: 'shipping_method_id' - # Credit card - card1_option = "Visa x-1111 #{I18n.t(:card_expiry_abbreviation)}:10/2030" - card2_option = "Master x-9999 #{I18n.t(:card_expiry_abbreviation)}:02/2044" - card3_option = "Visa x-5555 #{I18n.t(:card_expiry_abbreviation)}:06/2066" - expect(page).to have_select2 'credit_card_id', with_options: [card1_option, card2_option], without_options: [card3_option] - - # No date or credit card filled out, so error returned + # No date, so error returned click_button('Next') - expect(page).to have_content 'can\'t be blank', count: 2 + expect(page).to have_content 'can\'t be blank', count: 1 expect(page).to have_content 'Oops! Please fill in all of the required fields...' find_field('begins_at').click within(".ui-datepicker-calendar") do find('.ui-datepicker-today').click end - select2_select card2_option, from: 'credit_card_id' click_button('Next') expect(page).to have_content 'BILLING ADDRESS' @@ -263,7 +254,6 @@ feature 'Subscriptions' do expect(subscription.shipping_method).to eq shipping_method expect(subscription.bill_address.firstname).to eq 'Freda' expect(subscription.ship_address.firstname).to eq 'Freda' - expect(subscription.credit_card_id).to eq credit_card2.id # Standing Line Items are created expect(subscription.subscription_line_items.count).to eq 1 @@ -287,6 +277,7 @@ feature 'Subscriptions' do let!(:variant3_oc) { create(:simple_order_cycle, coordinator: shop, orders_open_at: 2.days.from_now, orders_close_at: 7.days.from_now) } let!(:variant3_ex) { variant3_oc.exchanges.create(sender: shop, receiver: shop, variants: [variant3]) } let!(:payment_method) { create(:payment_method, distributors: [shop]) } + let!(:stripe_payment_method) { create(:stripe_payment_method, name: 'Credit Card', distributors: [shop], preferred_enterprise_id: shop.id) } let!(:shipping_method) { create(:shipping_method, distributors: [shop]) } let!(:subscription) { create(:subscription, @@ -306,6 +297,13 @@ feature 'Subscriptions' do click_button 'edit-details' expect(page).to have_selector '#s2id_customer_id.select2-container-disabled' expect(page).to have_selector '#s2id_schedule_id.select2-container-disabled' + + # Can't use a Stripe payment method because customer does not allow it + select2_select stripe_payment_method.name, from: 'payment_method_id' + expect(page).to have_content I18n.t('admin.subscriptions.details.charges_not_allowed') + click_button 'Save Changes' + expect(page).to have_content 'Credit card charges are not allowed by this customer' + select2_select payment_method.name, from: 'payment_method_id' click_button 'Review' # Existing products should be visible From e0d46aa10549b0ae18c8a0745e374291b9fcaf07 Mon Sep 17 00:00:00 2001 From: Rob Harrington Date: Thu, 10 May 2018 13:08:03 +1000 Subject: [PATCH 32/71] Add new serializer to allow search for customer addresses --- .../controllers/details_controller.js.coffee | 1 + app/controllers/admin/customers_controller.rb | 6 +++++- .../admin/subscription_customer_serializer.rb | 20 +++++++++++++++++++ .../subscription_customer_serializer_spec.rb | 18 +++++++++++++++++ 4 files changed, 44 insertions(+), 1 deletion(-) create mode 100644 app/serializers/api/admin/subscription_customer_serializer.rb create mode 100644 spec/serializers/admin/subscription_customer_serializer_spec.rb diff --git a/app/assets/javascripts/admin/subscriptions/controllers/details_controller.js.coffee b/app/assets/javascripts/admin/subscriptions/controllers/details_controller.js.coffee index b4ace027dc..55d2a46b42 100644 --- a/app/assets/javascripts/admin/subscriptions/controllers/details_controller.js.coffee +++ b/app/assets/javascripts/admin/subscriptions/controllers/details_controller.js.coffee @@ -21,6 +21,7 @@ angular.module("admin.subscriptions").controller "DetailsController", ($scope, $ $scope.loadCustomer = -> params = { id: $scope.subscription.customer_id } + params.ams_prefix = 'subscription' unless $scope.subscription.id $scope.customer = CustomerResource.get params, (response) -> for address in ['bill_address','ship_address'] return unless response[address] diff --git a/app/controllers/admin/customers_controller.rb b/app/controllers/admin/customers_controller.rb index d57340b653..0e7eec1e90 100644 --- a/app/controllers/admin/customers_controller.rb +++ b/app/controllers/admin/customers_controller.rb @@ -24,7 +24,7 @@ module Admin end def show - render_as_json @customer + render_as_json @customer, ams_prefix: params[:ams_prefix] end def create @@ -91,5 +91,9 @@ module Admin spree_current_user.admin? || spree_current_user.enterprises.include?(@customer.enterprise) end + + def ams_prefix_whitelist + [:subscription] + end end end diff --git a/app/serializers/api/admin/subscription_customer_serializer.rb b/app/serializers/api/admin/subscription_customer_serializer.rb new file mode 100644 index 0000000000..7533fbc93a --- /dev/null +++ b/app/serializers/api/admin/subscription_customer_serializer.rb @@ -0,0 +1,20 @@ +module Api + module Admin + # Used by admin subscription form + # Searches for a ship and bill addresses for the customer + # where they are not already explicitly set + class SubscriptionCustomerSerializer < CustomerSerializer + def bill_address + finder.bill_address + end + + def ship_address + finder.ship_address + end + + def finder + @finder ||= OpenFoodNetwork::AddressFinder.new(object, object.email) + end + end + end +end diff --git a/spec/serializers/admin/subscription_customer_serializer_spec.rb b/spec/serializers/admin/subscription_customer_serializer_spec.rb new file mode 100644 index 0000000000..af5d769804 --- /dev/null +++ b/spec/serializers/admin/subscription_customer_serializer_spec.rb @@ -0,0 +1,18 @@ +describe Api::Admin::SubscriptionCustomerSerializer do + let(:address) { build(:address) } + let(:customer) { build(:customer) } + let(:serializer) { Api::Admin::SubscriptionCustomerSerializer.new(customer) } + let(:finder_mock) { instance_double(OpenFoodNetwork::AddressFinder, bill_address: address, ship_address: address) } + + before do + allow(serializer).to receive(:finder) { finder_mock } + end + + it "serializes a customer " do + result = JSON.parse(serializer.to_json) + expect(result['email']).to eq customer.email + expect(result['ship_address']['id']).to be nil + expect(result['ship_address']['address1']).to eq address.address1 + expect(result['ship_address']['firstname']).to eq address.firstname + end +end From 21c3f7d21cacc7886e4e9a8f464c4a545e8a2469 Mon Sep 17 00:00:00 2001 From: Rob Harrington Date: Thu, 10 May 2018 18:35:41 +1000 Subject: [PATCH 33/71] Remove unrequired #cards and #addresses actions from Admin::CustomerController --- app/controllers/admin/customers_controller.rb | 16 --- app/models/spree/ability_decorator.rb | 2 +- config/routes.rb | 5 +- .../admin/customers_controller_spec.rb | 97 ------------------- 4 files changed, 2 insertions(+), 118 deletions(-) diff --git a/app/controllers/admin/customers_controller.rb b/app/controllers/admin/customers_controller.rb index 0e7eec1e90..e5368077d7 100644 --- a/app/controllers/admin/customers_controller.rb +++ b/app/controllers/admin/customers_controller.rb @@ -59,22 +59,6 @@ module Admin end end - # GET /admin/customers/:id/addresses - # Used by subscriptions form to load details for selected customer - def addresses - finder = OpenFoodNetwork::AddressFinder.new(@customer, @customer.email) - bill_address = Api::AddressSerializer.new(finder.bill_address).serializable_hash - ship_address = Api::AddressSerializer.new(finder.ship_address).serializable_hash - render json: { bill_address: bill_address, ship_address: ship_address } - end - - # GET /admin/customers/:id/cards - # Used by subscriptions form to load details for selected customer - def cards - cards = Spree::CreditCard.where(user_id: @customer.user_id) - render json: ActiveModel::ArraySerializer.new(cards, each_serializer: Api::CreditCardSerializer) - end - private def collection diff --git a/app/models/spree/ability_decorator.rb b/app/models/spree/ability_decorator.rb index f5d05c9a3d..53198251b9 100644 --- a/app/models/spree/ability_decorator.rb +++ b/app/models/spree/ability_decorator.rb @@ -257,7 +257,7 @@ class AbilityDecorator can [:admin, :index, :customers, :group_buys, :bulk_coop, :sales_tax, :payments, :orders_and_distributors, :orders_and_fulfillment, :products_and_inventory, :order_cycle_management, :xero_invoices], :report can [:create], Customer - can [:admin, :index, :update, :destroy, :addresses, :cards, :show], Customer, enterprise_id: Enterprise.managed_by(user).pluck(:id) + can [:admin, :index, :update, :destroy, :show], Customer, enterprise_id: Enterprise.managed_by(user).pluck(:id) can [:admin, :new, :index], Subscription can [:create, :edit, :update, :cancel, :pause, :unpause], Subscription do |subscription| user.enterprises.include?(subscription.shop) diff --git a/config/routes.rb b/config/routes.rb index 0ab7fe1a3c..913b09da67 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -147,10 +147,7 @@ Openfoodnetwork::Application.routes.draw do resources :inventory_items, only: [:create, :update] - resources :customers, only: [:index, :create, :update, :destroy, :show] do - get :addresses, on: :member - get :cards, on: :member - end + resources :customers, only: [:index, :create, :update, :destroy, :show] resources :tag_rules, only: [], format: :json do get :map_by_tag, on: :collection diff --git a/spec/controllers/admin/customers_controller_spec.rb b/spec/controllers/admin/customers_controller_spec.rb index bd9ff1f0f7..63aad6ed60 100644 --- a/spec/controllers/admin/customers_controller_spec.rb +++ b/spec/controllers/admin/customers_controller_spec.rb @@ -139,103 +139,6 @@ describe Admin::CustomersController, type: :controller do end end - describe "#addresses" do - let!(:enterprise) { create(:enterprise) } - let(:bill_address) { create(:address, firstname: "Dominic", address1: "123 Lala Street" ) } - let(:ship_address) { create(:address, firstname: "Dom", address1: "123 Sesame Street") } - let(:managed_customer) { create(:customer, enterprise: enterprise, bill_address: bill_address, ship_address: ship_address) } - let(:unmanaged_customer) { create(:customer) } - let(:params) { { format: :json } } - - before { login_as_enterprise_user [enterprise] } - - context "when I manage the customer" do - before { params.merge!(id: managed_customer.id) } - - it "returns with serialized addresses for the customer" do - spree_get :addresses, params - json_response = JSON.parse(response.body) - expect(json_response.keys).to include "bill_address", "ship_address" - expect(json_response["bill_address"]["firstname"]).to eq "Dominic" - expect(json_response["bill_address"]["address1"]).to eq "123 Lala Street" - expect(json_response["ship_address"]["firstname"]).to eq "Dom" - expect(json_response["ship_address"]["address1"]).to eq "123 Sesame Street" - end - end - - context "when I don't manage the customer" do - before { params.merge!(customer_id: unmanaged_customer.id) } - - it "redirects to unauthorised" do - spree_get :addresses, params - expect(response).to redirect_to spree.unauthorized_path - end - end - - context "when no customer with a matching id exists" do - before { params.merge!(customer_id: 1) } - - it "redirects to unauthorised" do - spree_get :addresses, params - expect(response).to redirect_to spree.unauthorized_path - end - end - end - - describe "#cards" do - let(:user) { create(:user) } - let!(:enterprise) { create(:enterprise) } - let!(:credit_card1) { create(:credit_card, user: user) } - let!(:credit_card2) { create(:credit_card) } - let(:managed_customer) { create(:customer, enterprise: enterprise) } - let(:unmanaged_customer) { create(:customer) } - let(:params) { { format: :json } } - - before { login_as_enterprise_user [enterprise] } - - context "when I manage the customer" do - before { params.merge!(id: managed_customer.id) } - - context "when the customer is not associated with a user" do - it "returns with an empty array" do - spree_get :cards, params - json_response = JSON.parse(response.body) - expect(json_response).to eq [] - end - end - - context "when the customer is associated with a user" do - before { managed_customer.update_attributes(user_id: user.id) } - - it "returns with serialized cards for the customer" do - spree_get :cards, params - json_response = JSON.parse(response.body) - expect(json_response).to be_an Array - expect(json_response.length).to be 1 - expect(json_response.first["id"]).to eq credit_card1.id - end - end - end - - context "when I don't manage the customer" do - before { params.merge!(customer_id: unmanaged_customer.id) } - - it "redirects to unauthorised" do - spree_get :cards, params - expect(response).to redirect_to spree.unauthorized_path - end - end - - context "when no customer with a matching id exists" do - before { params.merge!(customer_id: 1) } - - it "redirects to unauthorised" do - spree_get :cards, params - expect(response).to redirect_to spree.unauthorized_path - end - end - end - describe "show" do let(:enterprise) { create(:distributor_enterprise) } let(:another_enterprise) { create(:distributor_enterprise) } From 4863f2b5b42e7c25bb8c1fbaf2fe3c1703d5eb33 Mon Sep 17 00:00:00 2001 From: Rob Harrington Date: Thu, 10 May 2018 18:45:07 +1000 Subject: [PATCH 34/71] Remove unrequired credit_card_id field from subscriptions table --- app/models/subscription.rb | 1 - ...0083800_remove_credit_card_from_subscriptions.rb | 13 +++++++++++++ db/schema.rb | 5 +---- spec/models/subscription_spec.rb | 1 - 4 files changed, 14 insertions(+), 6 deletions(-) create mode 100644 db/migrate/20180510083800_remove_credit_card_from_subscriptions.rb diff --git a/app/models/subscription.rb b/app/models/subscription.rb index c5a3d8bd55..165b063ae0 100644 --- a/app/models/subscription.rb +++ b/app/models/subscription.rb @@ -8,7 +8,6 @@ class Subscription < ActiveRecord::Base belongs_to :payment_method, class_name: 'Spree::PaymentMethod' belongs_to :bill_address, foreign_key: :bill_address_id, class_name: Spree::Address belongs_to :ship_address, foreign_key: :ship_address_id, class_name: Spree::Address - belongs_to :credit_card, foreign_key: :credit_card_id, class_name: 'Spree::CreditCard' has_many :subscription_line_items, inverse_of: :subscription has_many :order_cycles, through: :schedule has_many :proxy_orders diff --git a/db/migrate/20180510083800_remove_credit_card_from_subscriptions.rb b/db/migrate/20180510083800_remove_credit_card_from_subscriptions.rb new file mode 100644 index 0000000000..0821fbef61 --- /dev/null +++ b/db/migrate/20180510083800_remove_credit_card_from_subscriptions.rb @@ -0,0 +1,13 @@ +class RemoveCreditCardFromSubscriptions < ActiveRecord::Migration + def up + remove_foreign_key :subscriptions, name: :subscriptions_credit_card_id_fk + remove_index :subscriptions, :credit_card_id + remove_column :subscriptions, :credit_card_id + end + + def down + add_column :subscriptions, :credit_card_id, :integer + add_index :subscriptions, :credit_card_id + add_foreign_key :subscriptions, :spree_credit_cards, name: :subscriptions_credit_card_id_fk, column: :credit_card_id + end +end diff --git a/db/schema.rb b/db/schema.rb index caad9f27a5..d3121a1f40 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -11,7 +11,7 @@ # # It's strongly recommended to check this file into your version control system. -ActiveRecord::Schema.define(:version => 20180418025217) do +ActiveRecord::Schema.define(:version => 20180510083800) do create_table "account_invoices", :force => true do |t| t.integer "user_id", :null => false @@ -1118,13 +1118,11 @@ ActiveRecord::Schema.define(:version => 20180418025217) do t.integer "ship_address_id", :null => false t.datetime "canceled_at" t.datetime "paused_at" - t.integer "credit_card_id" t.decimal "shipping_fee_estimate", :precision => 8, :scale => 2 t.decimal "payment_fee_estimate", :precision => 8, :scale => 2 end add_index "subscriptions", ["bill_address_id"], :name => "index_subscriptions_on_bill_address_id" - add_index "subscriptions", ["credit_card_id"], :name => "index_subscriptions_on_credit_card_id" add_index "subscriptions", ["customer_id"], :name => "index_subscriptions_on_customer_id" add_index "subscriptions", ["payment_method_id"], :name => "index_subscriptions_on_payment_method_id" add_index "subscriptions", ["schedule_id"], :name => "index_subscriptions_on_schedule_id" @@ -1363,7 +1361,6 @@ ActiveRecord::Schema.define(:version => 20180418025217) do add_foreign_key "subscriptions", "schedules", name: "subscriptions_schedule_id_fk" add_foreign_key "subscriptions", "spree_addresses", name: "subscriptions_bill_address_id_fk", column: "bill_address_id" add_foreign_key "subscriptions", "spree_addresses", name: "subscriptions_ship_address_id_fk", column: "ship_address_id" - add_foreign_key "subscriptions", "spree_credit_cards", name: "subscriptions_credit_card_id_fk", column: "credit_card_id" add_foreign_key "subscriptions", "spree_payment_methods", name: "subscriptions_payment_method_id_fk", column: "payment_method_id" add_foreign_key "subscriptions", "spree_shipping_methods", name: "subscriptions_shipping_method_id_fk", column: "shipping_method_id" diff --git a/spec/models/subscription_spec.rb b/spec/models/subscription_spec.rb index f0cb9f39e6..c8ae2b5f24 100644 --- a/spec/models/subscription_spec.rb +++ b/spec/models/subscription_spec.rb @@ -9,7 +9,6 @@ describe Subscription, type: :model do it { expect(subject).to belong_to(:payment_method) } it { expect(subject).to belong_to(:ship_address) } it { expect(subject).to belong_to(:bill_address) } - it { expect(subject).to belong_to(:credit_card) } it { expect(subject).to have_many(:subscription_line_items) } it { expect(subject).to have_many(:order_cycles) } it { expect(subject).to have_many(:proxy_orders) } From edde929ced344e02928928220a6b13b86e570c0f Mon Sep 17 00:00:00 2001 From: Rob Harrington Date: Fri, 11 May 2018 14:42:01 +1000 Subject: [PATCH 35/71] Replace uses of #stub with #allow in Admin::CustomersController --- .../admin/customers_controller_spec.rb | 20 +++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/spec/controllers/admin/customers_controller_spec.rb b/spec/controllers/admin/customers_controller_spec.rb index 63aad6ed60..b1503d89f7 100644 --- a/spec/controllers/admin/customers_controller_spec.rb +++ b/spec/controllers/admin/customers_controller_spec.rb @@ -9,7 +9,7 @@ describe Admin::CustomersController, type: :controller do context "html" do before do - controller.stub spree_current_user: enterprise.owner + allow(controller).to receive(:spree_current_user) { enterprise.owner } end it "returns an empty @collection" do @@ -23,7 +23,7 @@ describe Admin::CustomersController, type: :controller do context "where I manage the enterprise" do before do - controller.stub spree_current_user: enterprise.owner + allow(controller).to receive(:spree_current_user) { enterprise.owner } end context "and enterprise_id is given in params" do @@ -50,7 +50,7 @@ describe Admin::CustomersController, type: :controller do context "and I do not manage the enterprise" do before do - controller.stub spree_current_user: another_enterprise.owner + allow(controller).to receive(:spree_current_user) { another_enterprise.owner } end it "returns an empty collection" do @@ -72,7 +72,7 @@ describe Admin::CustomersController, type: :controller do render_views before do - controller.stub spree_current_user: enterprise.owner + allow(controller).to receive(:spree_current_user) { enterprise.owner } end it "allows me to update the customer" do @@ -85,7 +85,7 @@ describe Admin::CustomersController, type: :controller do context "where I don't manage the customer's enterprise" do before do - controller.stub spree_current_user: another_enterprise.owner + allow(controller).to receive(:spree_current_user) { another_enterprise.owner } end it "prevents me from updating the customer" do @@ -109,7 +109,7 @@ describe Admin::CustomersController, type: :controller do context "json" do context "where I manage the customer's enterprise" do before do - controller.stub spree_current_user: enterprise.owner + allow(controller).to receive(:spree_current_user) { enterprise.owner } end it "allows me to create the customer" do @@ -119,7 +119,7 @@ describe Admin::CustomersController, type: :controller do context "where I don't manage the customer's enterprise" do before do - controller.stub spree_current_user: another_enterprise.owner + allow(controller).to receive(:spree_current_user) { another_enterprise.owner } end it "prevents me from creating the customer" do @@ -129,7 +129,7 @@ describe Admin::CustomersController, type: :controller do context "where I am the admin user" do before do - controller.stub spree_current_user: create(:admin_user) + allow(controller).to receive(:spree_current_user) { create(:admin_user) } end it "allows admins to create the customer" do @@ -150,7 +150,7 @@ describe Admin::CustomersController, type: :controller do render_views before do - controller.stub spree_current_user: enterprise.owner + allow(controller).to receive(:spree_current_user) { enterprise.owner } end it "renders the customer as json" do @@ -161,7 +161,7 @@ describe Admin::CustomersController, type: :controller do context "where I don't manage the customer's enterprise" do before do - controller.stub spree_current_user: another_enterprise.owner + allow(controller).to receive(:spree_current_user) { another_enterprise.owner } end it "prevents me from updating the customer" do From a902af42a33766423f9a451a9b87e61ab4215dc7 Mon Sep 17 00:00:00 2001 From: Rob Harrington Date: Mon, 25 Jun 2018 15:30:20 +1000 Subject: [PATCH 36/71] Update attribute that errors are added to from credit_card to payment_method --- app/services/subscription_validator.rb | 5 ++--- config/locales/en.yml | 6 +++--- spec/services/subscription_validator_spec.rb | 9 ++++----- 3 files changed, 9 insertions(+), 11 deletions(-) diff --git a/app/services/subscription_validator.rb b/app/services/subscription_validator.rb index 14cae211a6..33fc2baf77 100644 --- a/app/services/subscription_validator.rb +++ b/app/services/subscription_validator.rb @@ -23,7 +23,6 @@ class SubscriptionValidator delegate :shop, :customer, :schedule, :shipping_method, :payment_method, to: :subscription delegate :bill_address, :ship_address, :begins_at, :ends_at, to: :subscription - delegate :credit_card, :credit_card_id, to: :subscription delegate :subscription_line_items, to: :subscription def initialize(subscription) @@ -78,9 +77,9 @@ class SubscriptionValidator def credit_card_ok? return unless customer && payment_method return unless payment_method.type == "Spree::Gateway::StripeConnect" - return errors.add(:credit_card, :charges_not_allowed) unless customer.allow_charges + return errors.add(:payment_method, :charges_not_allowed) unless customer.allow_charges return if customer.user.andand.default_card.present? - errors.add(:credit_card, :no_default_card) + errors.add(:payment_method, :no_default_card) end def subscription_line_items_present? diff --git a/config/locales/en.yml b/config/locales/en.yml index 87a6799143..76da2ba08e 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -97,11 +97,11 @@ en: payment_method: not_available_to_shop: "is not available to %{shop}" invalid_type: "must be a Cash or Stripe method" + charges_not_allowed: "^Credit card charges are not allowed by this customer" + no_default_card: "^No default card available for this customer" shipping_method: not_available_to_shop: "is not available to %{shop}" - credit_card: - charges_not_allowed: "charges are not allowed by this customer" - no_default_card: "not available for this customer" + devise: confirmations: send_instructions: "You will receive an email with instructions about how to confirm your account in a few minutes." diff --git a/spec/services/subscription_validator_spec.rb b/spec/services/subscription_validator_spec.rb index 43ab894aa6..6670d14fa4 100644 --- a/spec/services/subscription_validator_spec.rb +++ b/spec/services/subscription_validator_spec.rb @@ -30,7 +30,6 @@ describe SubscriptionValidator do ship_address: true, begins_at: true, ends_at: true, - credit_card: true } end @@ -339,7 +338,7 @@ describe SubscriptionValidator do it "adds an error and returns false" do expect(validator.valid?).to be false - expect(validator.errors[:credit_card]).to_not be_empty + expect(validator.errors[:payment_method]).to_not be_empty end end @@ -351,7 +350,7 @@ describe SubscriptionValidator do it "adds an error and returns false" do expect(validator.valid?).to be false - expect(validator.errors[:credit_card]).to_not be_empty + expect(validator.errors[:payment_method]).to_not be_empty end end @@ -363,7 +362,7 @@ describe SubscriptionValidator do it "adds an error and returns false" do expect(validator.valid?).to be false - expect(validator.errors[:credit_card]).to_not be_empty + expect(validator.errors[:payment_method]).to_not be_empty end end @@ -372,7 +371,7 @@ describe SubscriptionValidator do it "returns true" do expect(validator.valid?).to be true - expect(validator.errors[:credit_card]).to be_empty + expect(validator.errors[:payment_method]).to be_empty end end end From 4fd333bbaf8d929ea38fa431fa249b5dd2440a92 Mon Sep 17 00:00:00 2001 From: Rob Harrington Date: Mon, 25 Jun 2018 15:31:59 +1000 Subject: [PATCH 37/71] Remove obsolete reference to credit_card from subscription serializer --- app/serializers/api/admin/subscription_serializer.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/serializers/api/admin/subscription_serializer.rb b/app/serializers/api/admin/subscription_serializer.rb index cbba4cc483..91b30e4e8c 100644 --- a/app/serializers/api/admin/subscription_serializer.rb +++ b/app/serializers/api/admin/subscription_serializer.rb @@ -2,7 +2,7 @@ module Api module Admin class SubscriptionSerializer < ActiveModel::Serializer attributes :id, :shop_id, :customer_id, :schedule_id, :payment_method_id, :shipping_method_id, :begins_at, :ends_at - attributes :customer_email, :schedule_name, :edit_path, :canceled_at, :paused_at, :state, :credit_card_id + attributes :customer_email, :schedule_name, :edit_path, :canceled_at, :paused_at, :state attributes :shipping_fee_estimate, :payment_fee_estimate has_many :subscription_line_items, serializer: Api::Admin::SubscriptionLineItemSerializer From 0afa9fae8ea464a6580017c04901a0c8fc4b723e Mon Sep 17 00:00:00 2001 From: Rob Harrington Date: Thu, 28 Jun 2018 17:56:58 +1000 Subject: [PATCH 38/71] Check authorisation before attempting to charge credit cards on order cycle close --- config/locales/en.yml | 2 +- .../subscription_payment_updater.rb | 6 +- .../subscription_payment_updater_spec.rb | 63 +++++++++++++++---- 3 files changed, 57 insertions(+), 14 deletions(-) diff --git a/config/locales/en.yml b/config/locales/en.yml index 76da2ba08e..4b30116d7e 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -75,7 +75,7 @@ en: email: taken: "There's already an account for this email. Please login or reset your password." spree/order: - no_card: There are no valid credit cards available + no_card: There are no authorised credit cards available to charge order_cycle: attributes: orders_close_at: diff --git a/lib/open_food_network/subscription_payment_updater.rb b/lib/open_food_network/subscription_payment_updater.rb index eb33aaf2f2..bdf437c963 100644 --- a/lib/open_food_network/subscription_payment_updater.rb +++ b/lib/open_food_network/subscription_payment_updater.rb @@ -42,10 +42,14 @@ module OpenFoodNetwork end def ensure_credit_card - return false if saved_credit_card.blank? + return false if saved_credit_card.blank? || !allow_charges? payment.update_attributes(source: saved_credit_card) end + def allow_charges? + order.customer.allow_charges? + end + def saved_credit_card order.user.default_card end diff --git a/spec/lib/open_food_network/subscription_payment_updater_spec.rb b/spec/lib/open_food_network/subscription_payment_updater_spec.rb index 8d590f59b7..4e238476d5 100644 --- a/spec/lib/open_food_network/subscription_payment_updater_spec.rb +++ b/spec/lib/open_food_network/subscription_payment_updater_spec.rb @@ -96,8 +96,10 @@ module OpenFoodNetwork context "and the payment source is not a credit card" do before { expect(updater).to receive(:card_set?) { false } } - context "and no credit card is available on the subscription" do - before { expect(updater).to receive(:ensure_credit_card) { false } } + context "and no default credit card has been set by the customer" do + before do + allow(order).to receive(:user) { instance_double(Spree::User, default_card: nil) } + end it "adds an error to the order and does not update the payment" do expect(payment).to_not receive(:update_attributes) @@ -105,8 +107,23 @@ module OpenFoodNetwork end end - context "but a credit card is available on the subscription" do - before { expect(updater).to receive(:ensure_credit_card) { true } } + context "and the customer has not authorised the shop to charge to credit cards" do + before do + allow(order).to receive(:user) { instance_double(Spree::User, default_card: create(:credit_card)) } + allow(order).to receive(:customer) { instance_double(Customer, allow_charges?: false) } + end + + it "adds an error to the order and does not update the payment" do + expect(payment).to_not receive(:update_attributes) + expect{ updater.update! }.to change(order.errors[:base], :count).from(0).to(1) + end + end + + context "and an authorised default credit card is available to charge" do + before do + allow(order).to receive(:user) { instance_double(Spree::User, default_card: create(:credit_card)) } + allow(order).to receive(:customer) { instance_double(Customer, allow_charges?: true) } + end context "when the payment total doesn't match the outstanding balance on the order" do before { allow(order).to receive(:outstanding_balance) { 5 } } @@ -151,8 +168,10 @@ module OpenFoodNetwork let!(:payment) { create(:payment, source: nil) } before { allow(updater).to receive(:payment) { payment } } - context "when no saved credit card is found" do - before { allow(updater).to receive(:saved_credit_card) { nil } } + context "when no default credit card is found" do + before do + allow(order).to receive(:user) { instance_double(Spree::User, default_card: nil) } + end it "returns false and down not update the payment source" do expect do @@ -161,14 +180,34 @@ module OpenFoodNetwork end end - context "when a saved credit card is found" do + context "when a default credit card is found" do let(:credit_card) { create(:credit_card) } - before { allow(updater).to receive(:saved_credit_card) { credit_card } } + before do + allow(order).to receive(:user) { instance_double(Spree::User, default_card: credit_card) } + end - it "returns true and stores the credit card as the payment source" do - expect do - expect(updater.send(:ensure_credit_card)).to be true - end.to change(payment, :source_id).from(nil).to(credit_card.id) + context "and charge have not been authorised by the customer" do + before do + allow(order).to receive(:customer) { instance_double(Customer, allow_charges?: false) } + end + + it "returns false and does not update the payment source" do + expect do + expect(updater.send(:ensure_credit_card)).to be false + end.to_not change(payment, :source).from(nil) + end + end + + context "and charges have been authorised by the customer" do + before do + allow(order).to receive(:customer) { instance_double(Customer, allow_charges?: true) } + end + + it "returns true and stores the credit card as the payment source" do + expect do + expect(updater.send(:ensure_credit_card)).to be true + end.to change(payment, :source_id).from(nil).to(credit_card.id) + end end end end From 3dacd06b6b77e3b3d469c026cfb6e1b2372ce35a Mon Sep 17 00:00:00 2001 From: Rob Harrington Date: Thu, 28 Jun 2018 18:56:13 +1000 Subject: [PATCH 39/71] Reload order before sending emails to ensure state is up to date --- app/jobs/subscription_confirm_job.rb | 2 ++ spec/jobs/subscription_confirm_job_spec.rb | 6 ++++-- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/app/jobs/subscription_confirm_job.rb b/app/jobs/subscription_confirm_job.rb index d9c284d7f8..f0c8e8d07f 100644 --- a/app/jobs/subscription_confirm_job.rb +++ b/app/jobs/subscription_confirm_job.rb @@ -46,11 +46,13 @@ class SubscriptionConfirmJob end def send_confirm_email + @order.update! record_success(@order) SubscriptionMailer.confirmation_email(@order).deliver end def send_failed_payment_email + @order.update! record_and_log_error(:failed_payment, @order) SubscriptionMailer.failed_payment_email(@order).deliver end diff --git a/spec/jobs/subscription_confirm_job_spec.rb b/spec/jobs/subscription_confirm_job_spec.rb index f1b64b1343..7556a30478 100644 --- a/spec/jobs/subscription_confirm_job_spec.rb +++ b/spec/jobs/subscription_confirm_job_spec.rb @@ -174,7 +174,7 @@ describe SubscriptionConfirmJob do end describe "#send_confirm_email" do - let(:order) { double(:order) } + let(:order) { instance_double(Spree::Order) } let(:mail_mock) { double(:mailer_mock, deliver: true) } before do @@ -183,6 +183,7 @@ describe SubscriptionConfirmJob do end it "records a success and sends the email" do + expect(order).to receive(:update!) expect(job).to receive(:record_success).with(order).once job.send(:send_confirm_email) expect(SubscriptionMailer).to have_received(:confirmation_email).with(order) @@ -191,7 +192,7 @@ describe SubscriptionConfirmJob do end describe "#send_failed_payment_email" do - let(:order) { double(:order) } + let(:order) { instance_double(Spree::Order) } let(:mail_mock) { double(:mailer_mock, deliver: true) } before do @@ -200,6 +201,7 @@ describe SubscriptionConfirmJob do end it "records and logs an error and sends the email" do + expect(order).to receive(:update!) expect(job).to receive(:record_and_log_error).with(:failed_payment, order).once job.send(:send_failed_payment_email) expect(SubscriptionMailer).to have_received(:failed_payment_email).with(order) From 15db5e1d5002ac3880d6a4d87266ad3d99c3cdd7 Mon Sep 17 00:00:00 2001 From: Transifex-Openfoodnetwork Date: Mon, 9 Jul 2018 20:23:01 +1000 Subject: [PATCH 40/71] Updating translations for config/locales/en_GB.yml --- config/locales/en_GB.yml | 319 +++++++++++++++++++++++++++++++++++++-- 1 file changed, 305 insertions(+), 14 deletions(-) diff --git a/config/locales/en_GB.yml b/config/locales/en_GB.yml index eec1f844f6..d50003b209 100644 --- a/config/locales/en_GB.yml +++ b/config/locales/en_GB.yml @@ -1,4 +1,5 @@ en_GB: + language_name: "English" activerecord: attributes: spree/order: @@ -7,25 +8,108 @@ en_GB: completed_at: Completed At number: Number email: Customer E-Mail + spree/payment: + amount: Amount + order_cycle: + orders_close_at: Close date errors: models: spree/user: attributes: email: taken: "There's already an account for this email. Please login or reset your password." + spree/order: + no_card: There are no valid credit cards available + order_cycle: + attributes: + orders_close_at: + after_orders_open_at: must be after open date + activemodel: + errors: + models: + subscription_validator: + attributes: + subscription_line_items: + at_least_one_product: "^Please add at least one product" + not_available: "^%{name} is not available from the selected schedule" + ends_at: + after_begins_at: "must be after begins at" + customer: + does_not_belong_to_shop: "does not belong to %{shop}" + schedule: + not_coordinated_by_shop: "is not coordinated by %{shop}" + payment_method: + not_available_to_shop: "is not available to %{shop}" + invalid_type: "must be a Cash or Stripe method" + shipping_method: + not_available_to_shop: "is not available to %{shop}" + credit_card: + not_available: "is not available" + blank: "is required" devise: + confirmations: + send_instructions: "You will receive an email with instructions about how to confirm your account in a few minutes." + failed_to_send: "An error occurred whilst sending your confirmation email." + resend_confirmation_email: "Resend confirmation email." + confirmed: "Thanks for confirming your email! You can now log in." + not_confirmed: "Your email address could not be confirmed. Perhaps you have already completed this step?" + user_registrations: + spree_user: + signed_up_but_unconfirmed: "A message with a confirmation link has been sent to your email address. Please open the link to activate your account." failure: invalid: | Invalid email or password. Were you a guest last time? Perhaps you need to create an account or reset your password. + unconfirmed: "You have to confirm your account before continuing." + already_registered: "This email address is already registered. Please log in to continue, or go back and use another email address." + user_passwords: + spree_user: + updated_not_active: "Your password has been reset, but your email has not been confirmed yet." enterprise_mailer: confirmation_instructions: subject: "Please confirm the email address for %{enterprise}" welcome: subject: "%{enterprise} is now on %{sitename}" + invite_manager: + subject: "%{enterprise} has invited you to be a manager" producer_mailer: order_cycle: subject: "Order cycle report for %{producer}" + subscription_mailer: + placement_summary_email: + subject: A summary of recently placed subscription orders + greeting: "Hi %{name}," + intro: "Below is a summary of the subscription orders that have just been placed for %{shop}." + confirmation_summary_email: + subject: A summary of recently confirmed subscription orders + greeting: "Hi %{name}," + intro: "Below is a summary of the subscription orders that have just been finalised for %{shop}." + summary_overview: + total: A total of %{count} subscriptions were marked for automatic processing. + success_zero: Of these, none were processed successfully. + success_some: Of these, %{count} were processed successfully. + success_all: All were processed successfully. + issues: Details of the issues encountered are provided below. + summary_detail: + no_message_provided: No error message provided + changes: + title: Insufficient Stock (%{count} orders) + explainer: These orders were processed but insufficient stock was available for some requested items + empty: + title: No Stock (%{count} orders) + explainer: These orders were unable to be processed because no stock was available for any requested items + complete: + title: Already Processed (%{count} orders) + explainer: These orders were already marked as complete, and were therefore left untouched + processing: + title: Error Encountered (%{count} orders) + explainer: Automatic processing of these orders failed due to an error. The error has been listed where possible. + failed_payment: + title: Failed Payment (%{count} orders) + explainer: Automatic processing of payment for these orders failed due to an error. The error has been listed where possible. + other: + title: Other Failure (%{count} orders) + explainer: Automatic processing of these orders failed for an unknown reason. This should not occur, please contact us if you are seeing this. home: "OFN" title: Open Food Network welcome_to: 'Welcome to ' @@ -56,6 +140,9 @@ en_GB: say_no: "No" say_yes: "Yes" then: then + ongoing: Ongoing + bill_address: Billing Address + ship_address: Shipping Address sort_order_cycles_on_shopfront_by: "Sort Order Cycles On Shopfront By" required_fields: Required fields are denoted with an asterisk select_continue: Select and Continue @@ -106,23 +193,36 @@ en_GB: filter_results: Filter Results quantity: Quantity pick_up: Pick up + copy: Copy actions: create_and_add_another: "Create and Add Another" admin: + begins_at: Begins At + begins_on: Begins On + customer: Customer date: Date email: Email + ends_at: Ends At + ends_on: Ends On name: Name on_hand: In Stock on_demand: Unlimited on_demand?: Unlimited? order_cycle: Order Cycle + payment: Payment + payment_method: Payment Method phone: Phone price: Price producer: Producer + image: Image product: Product quantity: Quantity + schedule: Schedule + shipping: Shipping + shipping_method: Shipping Method shop: Shop sku: SKU + status_state: County tags: Tags variant: Variant weight: Weight @@ -140,6 +240,10 @@ en_GB: save: Save cancel: Cancel back: Back + show_more: Show more + show_n_more: Show %{num} more + choose: "Choose..." + please_select: Please select... columns: Columns actions: Actions viewing: "Viewing: %{current_view_name}" @@ -276,12 +380,20 @@ en_GB: available_on: Available On av_on: "Av. On" import_date: Imported + upload_an_image: Upload an image + product_search_keywords: Product Search Keywords + product_search_tip: Type words to help search your products in the shops. Use space to separate each keyword. + SEO_keywords: SEO Keywords + seo_tip: Type words to help search your products in the web. Use space to separate each keyword. Search: Search properties: property_name: Property Name inherited_property: Inherited Property variants: to_order_tip: "Items made to order do not have a set stock level, such as loaves of bread made fresh to order." + product_distributions: "Product Distributions" + group_buy_options: "Group Buy Options" + back_to_products_list: "Back to products list" product_import: title: Product Import file_not_found: File not found or could not be opened @@ -290,6 +402,8 @@ en_GB: model: no_file: "error: no file uploaded" could_not_process: "could not process file: invalid filetype" + incorrect_value: incorrect value + conditional_blank: can't be blank if unit_type is blank no_product: did not match any products in the database not_found: not found in database blank: can't be blank @@ -304,7 +418,13 @@ en_GB: product_list: Product list inventories: Inventories import: Import + upload: Upload import: + review: Review + proceed: Proceed + save: Save + results: Results + save_imported: Save imported products no_valid_entries: No valid entries found none_to_save: There are no entries that can be saved some_invalid_entries: Imported file contains some invalid entries @@ -319,10 +439,10 @@ en_GB: reset_absent?: Reset absent products? overwrite_all: Overwrite all overwrite_empty: Overwrite if empty - default_stock: Set default stock level - default_tax_cat: Set default tax category - default_shipping_cat: Set default shipping category - default_available_date: Set default available date + default_stock: Set stock level + default_tax_cat: Set tax category + default_shipping_cat: Set shipping category + default_available_date: Set available date validation_overview: Import validation overview entries_found: Entries found in imported file entries_with_errors: Items contain errors and will not be imported @@ -342,8 +462,8 @@ en_GB: inventory_updated: Inventory items updated products_reset: Products had stock level reset to zero inventory_reset: Inventory items had stock level reset to zero - all_saved: "All %{num} items saved successfully" - total_saved: "%{num} items saved successfully" + all_saved: "All items saved successfully" + some_saved: "items saved successfully" save_errors: Save errors view_products: View Products view_inventory: View Inventory @@ -398,6 +518,7 @@ en_GB: max_fulfilled_units: "Max Fulfilled Units" order_error: "Some errors must be resolved before you can update orders.\nAny fields with red borders contain errors." variants_without_unit_value: "WARNING: Some variants do not have a unit value" + select_variant: "Select a variant" enterprise: select_outgoing_oc_products_from: Select outgoing OC products from enterprises: @@ -424,6 +545,9 @@ en_GB: contact: name: Name name_placeholder: eg. Amanda Plum + email_address: Public Email Address + email_address_placeholder: eg. hello@food.co.uk + email_address_tip: "This email address will be displayed in your public profile" phone: Phone phone_placeholder: eg. 98 7654 3210 website: Website @@ -500,6 +624,10 @@ en_GB: allow_order_changes_tip: "Allow customers to change their order as long the order cycle is open." allow_order_changes_false: "Placed orders cannot be changed / cancelled" allow_order_changes_true: "Customers can change / cancel orders while order cycle is open" + enable_subscriptions: "Subscriptions" + enable_subscriptions_tip: "Enable subscriptions functionality?" + enable_subscriptions_false: "Disabled" + enable_subscriptions_true: "Enabled" shopfront_message: Shopfront Message shopfront_message_placeholder: > An optional explanation for customers detailing how your shopfront works, @@ -542,6 +670,7 @@ en_GB: resend: Resend owner: 'Owner' contact: "Contact" + contact_tip: "The manager who will receive enterprise emails for orders and notifications. Must have a confirmed email adress." owner_tip: The primary user responsible for this enterprise. notifications: Notifications notifications_tip: Notifications about orders will be send to this email address. @@ -549,6 +678,11 @@ en_GB: notifications_note: 'Note: A new email address may need to be confirmed prior to use' managers: Managers managers_tip: The other users with permission to manage this enterprise. + invite_manager: "Invite Manager" + invite_manager_tip: "Invite an unregistered user to sign up and become a manager of this enterprise." + add_unregistered_user: "Add an unregistered user" + email_confirmed: "Email confirmed" + email_not_confirmed: "Email not confirmed" actions: edit_profile: Edit Profile properties: Properties @@ -605,7 +739,10 @@ en_GB: welcome_title: Welcome to the Open Food Network! welcome_text: You have successfully created a next_step: Next step - choose_starting_point: 'Choose your starting point:' + choose_starting_point: 'Choose your package:' + invite_manager: + user_already_exists: "User already exists" + error: "Something went wrong" order_cycles: edit: advanced_settings: Advanced Settings @@ -642,11 +779,27 @@ en_GB: add_a_tag: Add a tag delivery_details: Pickup / Delivery details debug_info: Debug information + index: + involving: Involving + schedule: Schedule + schedules: Schedules + adding_a_new_schedule: Adding A New Schedule + updating_a_schedule: Updating A Schedule + new_schedule: New Schedule + create_schedule: Create Schedule + update_schedule: Update Schedule + delete_schedule: Delete Schedule + created_schedule: Created schedule + updated_schedule: Updated schedule + deleted_schedule: Deleted schedule + schedule_name_placeholder: Schedule Name + name_required_error: Please enter a name for this schedule + no_order_cycles_error: Please select at least one order cycle (drag and drop) name_and_timing_form: name: Name orders_open: Orders open at coordinator: Coordinator - order_closes: Orders close + orders_close: Orders close row: suppliers: suppliers distributors: distributors @@ -658,9 +811,22 @@ en_GB: customer_instructions_placeholder: Pick-up or delivery notes products: Products fees: Fees + destroy_errors: + schedule_present: That order cycle is linked to a schedule and cannot be deleted. Please unlink or delete the schedule first. + bulk_update: + no_data: Hm, something went wrong. No order cycle data found. + date_warning: + msg: This order cycle is linked to %{n} open subscription orders. Changing this date now will not affect any orders which have already been placed, but should be avoided if possible. Are you sure you want to proceed? + cancel: Cancel + proceed: Proceed producer_properties: index: title: Producer Properties + proxy_orders: + cancel: + could_not_cancel_the_order: Could not cancel the order + resume: + could_not_resume_the_order: Could not resume the order shared: user_guide_link: user_guide: User Guide @@ -734,10 +900,67 @@ en_GB: packing: name: Packing Reports subscriptions: + subscriptions: Subscriptions + new: New Subscription + create: Create Subscription + index: + please_select_a_shop: Please select a shop + edit_subscription: Edit Subscription + pause_subscription: Pause Subscription + cancel_subscription: Cancel Subscription + setup_explanation: + just_a_few_more_steps: 'Just a few more steps before you can begin:' + enable_subscriptions: "Enable subscriptions for at least one of your shops" + enable_subscriptions_step_1_html: 1. Go to the %{enterprises_link} page, find your shop, and click "Manage" + enable_subscriptions_step_2: 2. Under "Shop Preferences", enable the Subscriptions option + set_up_shipping_and_payment_methods_html: Set up %{shipping_link} and %{payment_link} methods + set_up_shipping_and_payment_methods_note_html: Note that only Cash and Stripe payment methods may
be used with subscriptions + ensure_at_least_one_customer_html: Ensure that at least one %{customer_link} exists + create_at_least_one_schedule: Create at least one Schedule + create_at_least_one_schedule_step_1_html: 1. Go to the on the %{order_cycles_link} page + create_at_least_one_schedule_step_2: 2. Create an order cycle if you have not already done so + create_at_least_one_schedule_step_3: 3. Click '+ New Schedule', and fill out the form + once_you_are_done_you_can_html: Once you are done, you can %{reload_this_page_link} + reload_this_page: reload this page + steps: + details: 1. Basic Details + products: 3. Add Products + review: 4. Review & Save + details: + details: Details + invalid_error: Oops! Please fill in all of the required fields... + allowed_payment_method_types_tip: Only Cash and Stripe payment methods may be used at the moment + credit_card: Credit Card + no_cards_available: No cards available + loading_flash: + loading: LOADING SUBSCRIPTIONS review: details: Details address: Address products: Products + product_already_in_order: This product has already been added to the order. Please edit the quantity directly. + orders: + number: Number + confirm_edit: Are you sure you want to edit this order? Doing so may make it more difficult to automatically sync changes to the subscription in the future. + confirm_cancel_msg: Are you sure you want to cancel this subscription? This action cannot be undone. + cancel_failure_msg: 'Sorry, cancellation failed!' + confirm_pause_msg: Are you sure you want to pause this subscription? + pause_failure_msg: 'Sorry, pausing failed!' + confirm_unpause_msg: Are you sure you want to unpause this subscription? + unpause_failure_msg: 'Sorry, unpausing failed!' + confirm_cancel_open_orders_msg: "Some orders for this subscription are currently open. The customer has already been notified that the order will be placed. Would you like to cancel these order(s) or keep them?" + resume_canceled_orders_msg: "Some orders for this subscription can be resumed right now. You can resume them from the orders dropdown." + yes_cancel_them: Cancel them + no_keep_them: Keep them + yes_i_am_sure: Yes, I'm sure + order_update_issues_msg: Some orders could not be automatically updated, this is most likely because they have been manually edited. Please review the issues listed below and make any adjustments to individual orders if required. + no_results: + no_subscriptions: No subscriptions yet... + why_dont_you_add_one: Why don't you add one? :) + no_matching_subscriptions: No matching subscriptions found + schedules: + destroy: + associated_subscriptions_error: This schedule cannot be deleted because it has associated subscriptions stripe_connect_settings: edit: title: "Stripe Connect" @@ -786,6 +1009,7 @@ en_GB: require_customer_login: "This shop is for customers only." require_login_html: "Please %{login} if you have an account already. Otherwise, %{register} to become a customer." require_customer_html: "Please %{contact} %{enterprise} to become a customer." + card_could_not_be_updated: Card could not be updated card_could_not_be_saved: card could not be saved spree_gateway_error_flash_for_checkout: "There was a problem with your payment information: %{error}" invoice_billing_address: "Billing address:" @@ -852,8 +1076,11 @@ en_GB: no_payment: no payment methods no_shipping_or_payment: no shipping or payment methods unconfirmed: unconfirmed + days: days + label_shop: "Shop" label_shops: "Shops" label_map: "Map" + label_producer: "Producer" label_producers: "Producers" label_groups: "Groups" label_about: "About" @@ -919,6 +1146,7 @@ en_GB: footer_legal_tos: "Terms and conditions" footer_legal_visit: "Find us on" footer_legal_text_html: "Open Food Network is a free and open source software platform. Our content is licensed with %{content_license} and our code with %{code_license}." + footer_skylight_dashboard_html: Performance data is available on %{dashboard}. home_shop: Shop Now brandstory_headline: "Re-imagining Local Food" brandstory_intro: "Online tools for buying, selling & distributing local food" @@ -1002,6 +1230,7 @@ en_GB: email_admin_html: "You can manage your account by logging into the %{link} or by clicking on the cog in the top right hand side of the homepage, and selecting Administration." email_community_html: "We also have an online forum for community discussion related to OFN software and the unique challenges of running a food enterprise. You are encouraged to join in. We are constantly evolving and your input into this forum will shape what happens next. %{link}" join_community: "Join the community" + email_confirmation_activate_account: "Before we can activate your new account, we need to confirm your email address." email_confirmation_greeting: "Hi, %{contact}!" email_confirmation_profile_created: "A profile for %{name} has been successfully created! To activate your Profile we need to confirm this email address." email_confirmation_click_link: "Please click the link below to confirm your email and to continue setting up your profile." @@ -1029,6 +1258,23 @@ en_GB: email_payment_not_paid: NOT PAID email_payment_summary: Payment summary email_payment_method: "Paying via:" + email_so_placement_intro_html: "You have a new order with %{distributor}" + email_so_placement_details_html: "Here are the details of your order for %{distributor}:" + email_so_placement_changes: "Unfortunately, not all products that you requested were available. The original quantities that you requested appear crossed-out below." + email_so_payment_success_intro_html: "An automatic payment has been processed for your order from %{distributor}." + email_so_placement_explainer_html: "This order was automatically created for you." + email_so_edit_true_html: "You can make changes until orders close on %{orders_close_at}." + email_so_edit_false_html: "You can view details of this order at any time." + email_so_contact_distributor_html: "If you have any questions you can contact %{distributor} via %{email}." + email_so_confirmation_intro_html: "Your order with %{distributor} is now confirmed" + email_so_confirmation_explainer_html: "This order was automatically placed for you, and it has now been finalised." + email_so_confirmation_details_html: "Here's everything you need to know about your order from %{distributor}:" + email_so_empty_intro_html: "We tried to place a new order with %{distributor}, but had some problems..." + email_so_empty_explainer_html: "Unfortunately, none of products that you ordered were available, so no order has been placed. The original quantities that you requested appear crossed-out below." + email_so_empty_details_html: "Here are the details of the unplaced order for %{distributor}:" + email_so_failed_payment_intro_html: "We tried to process a payment, but had some problems..." + email_so_failed_payment_explainer_html: "The payment for your subscription with %{distributor} failed because of a problem with your credit card. %{distributor} has been notified of this failed payment." + email_so_failed_payment_details_html: "Here are the details of the failure provided by the payment gateway:" email_shipping_delivery_details: Delivery details email_shipping_delivery_time: "Delivery on:" email_shipping_delivery_address: "Delivery address:" @@ -1038,8 +1284,16 @@ en_GB: email_special_instructions: "Your notes:" email_signup_greeting: Hello! email_signup_welcome: "Welcome to %{sitename}!" + email_signup_confirmed_email: "Thanks for confirming your email." + email_signup_shop_html: "You can now log in at %{link}." email_signup_text: "Thanks for joining the network. If you are a customer, we look forward to introducing you to many fantastic farmers, wonderful food hubs and delicious food! If you are a producer or food enterprise, we are excited to have you as a part of the network." email_signup_help_html: "We welcome all your questions and feedback; you can use the Send Feedback button on the site or email us at %{email}" + invite_email: + greeting: "Hello!" + invited_to_manage: "You have been invited to manage %{enterprise} on %{instance}." + confirm_your_email: "You should have received or will soon receive an email with a confirmation link. You won’t be able to access %{enterprise}'s profile until you have confirmed your email." + set_a_password: "You will then be prompted to set a password before you are able to administer the enterprise." + mistakenly_sent: "Not sure why you have received this email? Please contact %{owner_email} for more information." producer_mail_greeting: "Dear" producer_mail_text_before: "We now have all the consumer orders for the next food delivery." producer_mail_order_text: "Here is a summary of the orders for your products:" @@ -1202,6 +1456,7 @@ en_GB: shops_signup_help: We're ready to help. shops_signup_help_text: You need a better return. You need new buyers and logistics partners. You need your story told across wholesale, retail, and the kitchen table. shops_signup_detail: Here's the detail. + orders: Orders orders_fees: Fees... orders_edit_title: Shopping cart orders_edit_headline: Your shopping cart @@ -1287,6 +1542,7 @@ en_GB: november: "November" december: "December" email_not_found: "Email address not found" + email_unconfirmed: "You must confirm your email address before you can reset your password." email_required: "You must provide an email address" logging_in: "Hold on a moment, we're logging you in" signup_email: "Your email" @@ -1301,6 +1557,7 @@ en_GB: password_reset_sent: "An email with instructions on resetting your password has been sent!" reset_password: "Reset password" who_is_managing_enterprise: "Who is responsible for managing %{enterprise}?" + update_and_recalculate_fees: "Update And Recalculate Fees" enterprise: registration: modal: @@ -1537,6 +1794,10 @@ en_GB: calculator: "Calculator" calculator_values: "Calculator values" flat_percent_per_item: "Flat Percent (per item)" + flat_rate_per_item: "Flat Rate (per item)" + flat_rate_per_order: "Flat Rate (per order)" + flexible_rate: "Flexible Rate" + price_sack: "Price Sack" new_order_cycles: "New Order Cycles" new_order_cycle: "New Order Cycle" select_a_coordinator_for_your_order_cycle: "Select a coordinator for your order cycle" @@ -1571,12 +1832,7 @@ en_GB: spree_admin_enterprises_fees: "Enterprise Fees" spree_admin_enterprises_none_create_a_new_enterprise: "CREATE A NEW ENTERPRISE" spree_admin_enterprises_none_text: "You don't have any enterprises yet" - spree_admin_enterprises_producers_name: "Name" - spree_admin_enterprises_producers_total_products: "Total Products" - spree_admin_enterprises_producers_active_products: "Active Products" - spree_admin_enterprises_producers_order_cycles: "Products in OCs" spree_admin_enterprises_tabs_hubs: "HUBS" - spree_admin_enterprises_tabs_producers: "PRODUCERS" spree_admin_enterprises_producers_manage_products: "MANAGE PRODUCTS" spree_admin_enterprises_any_active_products_text: "You don't have any active products." spree_admin_enterprises_create_new_product: "CREATE A NEW PRODUCT" @@ -1812,6 +2068,8 @@ en_GB: products_unsaved: "Changes to %{n} products remain unsaved." is_already_manager: "is already a manager!" no_change_to_save: "No change to save" + user_invited: "%{email} has been invited to manage this enterprise" + add_manager: "Add an existing user" users: "Users" about: "About" images: "Images" @@ -1822,6 +2080,7 @@ en_GB: social: "Social" business_details: "Business Details" properties: "Properties" + shipping: "Shipping" shipping_methods: "Shipping Methods" payment_methods: "Payment Methods" payment_method_fee: "Transaction fee" @@ -1838,7 +2097,7 @@ en_GB: content_configuration_pricing_table: "(TODO: Pricing table)" content_configuration_case_studies: "(TODO: Case studies)" content_configuration_detail: "(TODO: Detail)" - enterprise_name_error: "has already been taken. If this is your enterprise and you would like to claim ownership, please contact the current manager of this profile at %{email}." + enterprise_name_error: "has already been taken. If this is your enterprise and you would like to claim ownership, or if you would like to trade with this enterprise please contact the current manager of this profile at %{email}." enterprise_owner_error: "^%{email} is not permitted to own any more enterprises (limit is %{enterprise_limit})." enterprise_role_uniqueness_error: "^That role is already present." inventory_item_visibility_error: must be true or false @@ -1872,6 +2131,15 @@ en_GB: order_cycles_email_to_producers_notice: 'Emails to be sent to producers have been queued for sending.' order_cycles_no_permission_to_coordinate_error: "None of your enterprises have permission to coordinate an order cycle" order_cycles_no_permission_to_create_error: "You don't have permission to create an order cycle coordinated by that enterprise" + back_to_orders_list: "Back to order list" + no_orders_found: "No Orders Found" + order_information: "Order Information" + date_completed: "Date Completed" + amount: "Amount" + state_names: + ready: Ready + pending: Pending + shipped: Shipped js: saving: 'Saving...' changes_saved: 'Changes saved.' @@ -1889,9 +2157,14 @@ en_GB: choose: Choose resolve_errors: Please resolve the following errors more_items: "+ %{count} More" + default_card_updated: Default Card Updated admin: + enterprise_limit_reached: "You have reached the standard limit of enterprises per account. Write to %{contact_email} if you need to increase it." modals: got_it: Got it + close: "Close" + invite: "Invite" + invite_title: "Invite an unregistered user" tag_rule_help: title: Tag Rules overview: Overview @@ -2055,6 +2328,10 @@ en_GB: customers: select_shop: 'Please select a shop first' could_not_create: Sorry! Could not create + subscriptions: + closes: closes + closed: closed + close_date_not_set: Close date not set producers: signup: start_free_profile: "Start with a free profile, and expand when you're ready!" @@ -2062,6 +2339,8 @@ en_GB: email: Email account_updated: "Account updated!" my_account: "My account" + date: "Date" + time: "Time" admin: orders: invoice: @@ -2122,6 +2401,7 @@ en_GB: inherits_properties?: Inherits Properties? available_on: Available On av_on: "Av. On" + import_date: "Import Date" products_variant: variant_has_n_overrides: "This variant has %{n} override(s)" new_variant: "New variant" @@ -2134,6 +2414,8 @@ en_GB: display_as: display_as: Display As reports: + table: + select_and_search: "Select filters and click on SEARCH to access your data." bulk_coop: bulk_coop_supplier_report: 'Bulk Co-op - Totals by Supplier' bulk_coop_allocation: 'Bulk Co-op - Allocation' @@ -2168,14 +2450,23 @@ en_GB: address: address adjustments: adjustments awaiting_return: awaiting return + canceled: cancelled cart: cart complete: complete confirm: confirm delivery: delivery + paused: paused payment: payment + pending: pending resumed: resumed returned: returned skrill: skrill + subscription_state: + active: active + pending: pending + ended: ended + paused: paused + canceled: cancelled payment_states: balance_due: balance due completed: completed From b478ef9fd049301edb872e7bfd550faedbd07866 Mon Sep 17 00:00:00 2001 From: Maikel Linke Date: Tue, 10 Jul 2018 09:59:00 +1000 Subject: [PATCH 41/71] Make admin serialiser specs runnable on their own --- app/serializers/api/admin/subscription_customer_serializer.rb | 2 ++ spec/serializers/admin/customer_serializer_spec.rb | 2 ++ spec/serializers/admin/enterprise_serializer_spec.rb | 2 ++ spec/serializers/admin/exchange_serializer_spec.rb | 1 + spec/serializers/admin/index_enterprise_serializer_spec.rb | 2 ++ spec/serializers/admin/subscription_customer_serializer_spec.rb | 2 ++ 6 files changed, 11 insertions(+) diff --git a/app/serializers/api/admin/subscription_customer_serializer.rb b/app/serializers/api/admin/subscription_customer_serializer.rb index 7533fbc93a..6f8490abec 100644 --- a/app/serializers/api/admin/subscription_customer_serializer.rb +++ b/app/serializers/api/admin/subscription_customer_serializer.rb @@ -1,3 +1,5 @@ +require 'spec_helper' + module Api module Admin # Used by admin subscription form diff --git a/spec/serializers/admin/customer_serializer_spec.rb b/spec/serializers/admin/customer_serializer_spec.rb index 697b03f41a..7d46f8aa83 100644 --- a/spec/serializers/admin/customer_serializer_spec.rb +++ b/spec/serializers/admin/customer_serializer_spec.rb @@ -1,3 +1,5 @@ +require 'spec_helper' + describe Api::Admin::CustomerSerializer do let(:customer) { create(:customer, tag_list: "one, two, three") } let!(:tag_rule) { create(:tag_rule, enterprise: customer.enterprise, preferred_customer_tags: "two") } diff --git a/spec/serializers/admin/enterprise_serializer_spec.rb b/spec/serializers/admin/enterprise_serializer_spec.rb index b8226a73e5..0898b7d4a0 100644 --- a/spec/serializers/admin/enterprise_serializer_spec.rb +++ b/spec/serializers/admin/enterprise_serializer_spec.rb @@ -1,3 +1,5 @@ +require 'spec_helper' + describe Api::Admin::EnterpriseSerializer do let(:enterprise) { create(:distributor_enterprise) } it "serializes an enterprise" do diff --git a/spec/serializers/admin/exchange_serializer_spec.rb b/spec/serializers/admin/exchange_serializer_spec.rb index 649a0b427e..31ab38fb86 100644 --- a/spec/serializers/admin/exchange_serializer_spec.rb +++ b/spec/serializers/admin/exchange_serializer_spec.rb @@ -1,3 +1,4 @@ +require 'spec_helper' require 'open_food_network/order_cycle_permissions' describe Api::Admin::ExchangeSerializer do diff --git a/spec/serializers/admin/index_enterprise_serializer_spec.rb b/spec/serializers/admin/index_enterprise_serializer_spec.rb index 3651f53f8d..a6ad4d8d02 100644 --- a/spec/serializers/admin/index_enterprise_serializer_spec.rb +++ b/spec/serializers/admin/index_enterprise_serializer_spec.rb @@ -1,3 +1,5 @@ +require 'spec_helper' + describe Api::Admin::IndexEnterpriseSerializer do include AuthenticationWorkflow diff --git a/spec/serializers/admin/subscription_customer_serializer_spec.rb b/spec/serializers/admin/subscription_customer_serializer_spec.rb index af5d769804..8ca1bc4805 100644 --- a/spec/serializers/admin/subscription_customer_serializer_spec.rb +++ b/spec/serializers/admin/subscription_customer_serializer_spec.rb @@ -1,3 +1,5 @@ +require 'spec_helper' + describe Api::Admin::SubscriptionCustomerSerializer do let(:address) { build(:address) } let(:customer) { build(:customer) } From f9a333875576d948032b457154d2ab8e113fbbab Mon Sep 17 00:00:00 2001 From: Maikel Linke Date: Tue, 10 Jul 2018 10:01:18 +1000 Subject: [PATCH 42/71] Simplify serialiser with `delegate` --- .../api/admin/subscription_customer_serializer.rb | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/app/serializers/api/admin/subscription_customer_serializer.rb b/app/serializers/api/admin/subscription_customer_serializer.rb index 6f8490abec..2cb10bf80a 100644 --- a/app/serializers/api/admin/subscription_customer_serializer.rb +++ b/app/serializers/api/admin/subscription_customer_serializer.rb @@ -6,13 +6,8 @@ module Api # Searches for a ship and bill addresses for the customer # where they are not already explicitly set class SubscriptionCustomerSerializer < CustomerSerializer - def bill_address - finder.bill_address - end - - def ship_address - finder.ship_address - end + delegate :bill_address, to: :finder + delegate :ship_address, to: :finder def finder @finder ||= OpenFoodNetwork::AddressFinder.new(object, object.email) From 459057a7dbc779f57d68f22b75fcb69d6fe3efe3 Mon Sep 17 00:00:00 2001 From: Maikel Linke Date: Tue, 10 Jul 2018 10:31:51 +1000 Subject: [PATCH 43/71] Revert accidental copy and paste --- app/serializers/api/admin/subscription_customer_serializer.rb | 2 -- 1 file changed, 2 deletions(-) diff --git a/app/serializers/api/admin/subscription_customer_serializer.rb b/app/serializers/api/admin/subscription_customer_serializer.rb index 2cb10bf80a..c018aaf8b6 100644 --- a/app/serializers/api/admin/subscription_customer_serializer.rb +++ b/app/serializers/api/admin/subscription_customer_serializer.rb @@ -1,5 +1,3 @@ -require 'spec_helper' - module Api module Admin # Used by admin subscription form From 24f77184cf07a2507b3b8dd76e747cc5daf3282f Mon Sep 17 00:00:00 2001 From: Maikel Linke Date: Wed, 11 Jul 2018 11:31:27 +1000 Subject: [PATCH 44/71] Move i18n locale doc to wiki and reference that --- config/locales/en.yml | 47 +++++++------------------------------------ 1 file changed, 7 insertions(+), 40 deletions(-) diff --git a/config/locales/en.yml b/config/locales/en.yml index 4b30116d7e..b466c99a3e 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -1,58 +1,25 @@ # English language file # --------------------- # -# This is the source language file maintained by the Australian OFN team. +# This is the source language file maintained by the global OFN team and used +# by the Australian OFN instance. +# # Visit Transifex to translate this file into other languages: # # https://www.transifex.com/open-food-foundation/open-food-network/ # -# If you translate this file in a text editor, please share your results with us by +# Read more about it at: # -# - uploading the file to Transifex or -# - opening a pull request at GitHub. -# -# -# See http://community.openfoodnetwork.org/t/localisation-ofn-in-your-language/397 +# https://github.com/openfoodfoundation/openfoodnetwork/wiki/i18n # # Changing this file # ================== # # You are welcome to fix typos, add missing translations and remove unused ones. -# Here are some guidelines to make sure that this file is becoming more beautiful -# with every change we do. +# But read our guidelines first: # -# * Change text: No problem. Fix the typo. And please enclose the text in quotes -# to avoid any accidents. +# https://github.com/openfoodfoundation/openfoodnetwork/wiki/i18n#development # -# Example 1: "When you're using double quotes, they look like \"this\"" -# Example 2: "When you’re using double quotes, they look like “this”" -# -# The second example uses unicode to make it look prettier and avoid backslashes. -# -# * Add translations: Cool, every bit of text in the application should be here. -# If you add a translation for a view or mailer, please make use of the nested -# structure. Use the "lazy" lookup. See: http://guides.rubyonrails.org/i18n.html#looking-up-translations -# -# Avoid global keys. There are a lot already. And some are okay, for example -# "enterprises" should be the same everywhere on the page. But in doubt, -# create a new translation and give it a meaningful scope. -# -# Don't worry about duplication. We may use the same word in different contexts, -# but another language could use different words. So don't try to re-use -# translations between files. -# -# Don't move big parts around or rename scopes with a lot of entries without -# a really good reason. In some cases that causes a lot of translations in -# other languages to be lost. That causes more work for translators. -# -# * Remove translations: If you are sure that they are not used anywhere, -# please remove them. Be aware that some translations are looked up with -# variables. For example app/views/admin/contents/_fieldset.html.haml looks -# up labels for preferences. Unfortunately, they don't have a scope. -# -# * Participate in the community discussions: -# - https://community.openfoodnetwork.org/t/workflow-to-structure-translations/932 - en: # Overridden here due to a bug in spree i18n (Issue #870, and issue #1800) language_name: "English" # Localised name of this language From 5508c6741f7c64e66ffd8028c89de229fe779f8c Mon Sep 17 00:00:00 2001 From: Kristina Lim Date: Wed, 11 Jul 2018 14:10:05 +0800 Subject: [PATCH 45/71] Fix broken "great-pr" link in CONTRIBUTING.md --- CONTRIBUTING.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 60bc662403..5ea968c65c 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -47,7 +47,7 @@ Push your changes to a branch on your fork: ## Submitting a Pull Request -Use the GitHub UI to submit a [new pull request][pr] against upstream/master. To increase the chances that your pull request is swiftly accepted please have a look at our guide to [[making a great pull request]]. +Use the GitHub UI to submit a [new pull request][pr] against upstream/master. To increase the chances that your pull request is swiftly accepted please have a look at our guide to [making a great pull request][great-pr]. TL;DR: * Write tests From 056df623590c132850774d171b42ce6e83af000f Mon Sep 17 00:00:00 2001 From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com> Date: Wed, 11 Jul 2018 18:02:29 +0100 Subject: [PATCH 46/71] Add changelog category to PR template --- .github/PULL_REQUEST_TEMPLATE.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/.github/PULL_REQUEST_TEMPLATE.md b/.github/PULL_REQUEST_TEMPLATE.md index 7468fd5248..a0c8f5638b 100644 --- a/.github/PULL_REQUEST_TEMPLATE.md +++ b/.github/PULL_REQUEST_TEMPLATE.md @@ -16,6 +16,11 @@ context for others to understand it] [In case this should be present in the release notes, please write them or remove this section otherwise] +[To streamline the release process, please designate your PR with ONE of the following +categories, based on the specification from keepachangelog.com (and delete the others):] + +Changelog Category: Added | Changed | Deprecated | Removed | Fixed | Security + #### How is this related to the Spree upgrade? [Any known conflicts with the Spree Upgrade? explain them or remove this section From b302a1d03d05a6affdfba25c81d0597fb61e4c3d Mon Sep 17 00:00:00 2001 From: luisramos0 Date: Thu, 14 Jun 2018 16:43:53 +0100 Subject: [PATCH 47/71] added Enterprise registration exit message and moved step TYPE translations to a new structure on en.yml (test if transifex picks up the existing translations) --- app/views/registration/steps/_about.html.haml | 1 + app/views/registration/steps/_type.html.haml | 16 +++++++------- config/locales/en.yml | 22 +++++++++++-------- 3 files changed, 22 insertions(+), 17 deletions(-) diff --git a/app/views/registration/steps/_about.html.haml b/app/views/registration/steps/_about.html.haml index d65b989ee3..529fdebea3 100644 --- a/app/views/registration/steps/_about.html.haml +++ b/app/views/registration/steps/_about.html.haml @@ -15,6 +15,7 @@ .small-12.columns .alert-box.info{ "ofn-inline-alert" => true, ng: { show: "visible" } } %h6{ "ng-bind" => "'enterprise_success' | t:{enterprise: enterprise.name}" } + %span {{'enterprise_registration_exit_message' | t}} %a.close{ ng: { click: "close()" } } × .small-12.large-8.columns diff --git a/app/views/registration/steps/_type.html.haml b/app/views/registration/steps/_type.html.haml index 9276a5731a..ad5e075d16 100644 --- a/app/views/registration/steps/_type.html.haml +++ b/app/views/registration/steps/_type.html.haml @@ -6,9 +6,9 @@ .row .small-12.columns %header - %h2{ "ng-bind" => "'enterprise.registration.modal.steps.type.headline' | t:{enterprise: enterprise.name}" } + %h2= t(".headline", enterprise: "{{enterprise.name}}") %h4 - {{'enterprise.registration.modal.steps.type.question' | t}} + = t(".question") %form{ name: 'type', novalidate: true, ng: { controller: "RegistrationFormCtrl", submit: "create(type)" } } .row#enterprise-types{ 'data-equalizer' => true, ng: { if: "::enterprise.type != 'own'" } } @@ -17,32 +17,32 @@ .small-12.medium-6.large-6.columns{ 'data-equalizer-watch' => true } %a.btnpanel#producer-panel{ href: "#", ng: { click: "enterprise.is_primary_producer = true", class: "{selected: enterprise.is_primary_producer}" } } %i.ofn-i_059-producer - %h4 {{'enterprise.registration.modal.steps.type.yes_producer' | t}} + %h4= t(".yes_producer") .small-12.medium-6.large-6.columns{ 'data-equalizer-watch' => true } %a.btnpanel#hub-panel{ href: "#", ng: { click: "enterprise.is_primary_producer = false", class: "{selected: enterprise.is_primary_producer == false}" } } %i.ofn-i_063-hub - %h4 {{'enterprise.registration.modal.steps.type.no_producer' | t}} + %h4= t(".no_producer") .row .small-12.columns %input.chunky{ id: 'enterprise_is_primary_producer', name: 'is_primary_producer', hidden: true, required: true, ng: { model: 'enterprise.is_primary_producer' } } %span.error{ ng: { show: "type.is_primary_producer.$error.required && submitted" } } - {{'enterprise.registration.modal.steps.type.producer_field_error' | t}} + = t(".producer_field_error") .row .small-12.columns .panel.callout .left %i.ofn-i_013-help   - %p {{'enterprise.registration.modal.steps.type.yes_producer_help' | t}} + %p= t(".yes_producer_help") .panel.callout .left %i.ofn-i_013-help   - %p {{'enterprise.registration.modal.steps.type.no_producer_help' | t}} + %p= t(".no_producer_help") .row.buttons .small-12.columns %input.button.secondary{ type: "button", value: "{{'back' | t}}", ng: { click: "select('contact')" } } - %input.button.primary.right{ ng: { disabled: 'isDisabled' }, type: "submit", value: "{{'create_profile' | t}}" } + %input.button.primary.right{ ng: { disabled: 'isDisabled' }, type: "submit", value: t(".create_profile") } diff --git a/config/locales/en.yml b/config/locales/en.yml index 4b30116d7e..da0ff0cba6 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -1737,6 +1737,17 @@ See the %{link} to find out more about %{sitename}'s features and to start using registration_greeting: "Greetings!" who_is_managing_enterprise: "Who is responsible for managing %{enterprise}?" update_and_recalculate_fees: "Update And Recalculate Fees" + registration: + steps: + type: + headline: "Last step to add %{enterprise}!" + question: "Are you a producer?" + yes_producer: "Yes, I'm a producer" + no_producer: "No, I'm not a producer" + producer_field_error: "Please choose one. Are you are producer?" + yes_producer_help: "Producers make yummy things to eat and/or drink. You're a producer if you grow it, raise it, brew it, bake it, ferment it, milk it or mould it." + no_producer_help: "If you’re not a producer, you’re probably someone who sells and distributes food. You might be a hub, coop, buying group, retailer, wholesaler or other." + create_profile: "Create Profile" enterprise: registration: modal: @@ -1775,14 +1786,6 @@ See the %{link} to find out more about %{sitename}'s features and to start using phone_field_placeholder: 'eg. (03) 1234 5678' type: title: 'Type' - headline: "Last step to add %{enterprise}!" - question: "Are you a producer?" - yes_producer: "Yes, I'm a producer" - no_producer: "No, I'm not a producer" - producer_field_error: "Please choose one. Are you are producer?" - yes_producer_help: "Producers make yummy things to eat and/or drink. You're a producer if you grow it, raise it, brew it, bake it, ferment it, milk it or mould it." - no_producer_help: "If you’re not a producer, you’re probably someone who sells and distributes food. You might be a hub, coop, buying group, retailer, wholesaler or other." - about: title: 'About' images: @@ -1823,6 +1826,7 @@ See the %{link} to find out more about %{sitename}'s features and to start using enterprise_about_headline: "Nice one!" enterprise_about_message: "Now let's flesh out the details about" enterprise_success: "Success! %{enterprise} added to the Open Food Network " + enterprise_registration_exit_message: "If you exit this wizard at any stage, you can continue to create your profile by going to the admin interface." enterprise_description: "Short Description" enterprise_description_placeholder: "A short sentence describing your enterprise" enterprise_long_desc: "Long Description" @@ -1875,7 +1879,7 @@ See the %{link} to find out more about %{sitename}'s features and to start using registration_type_producer_help: "Producers make yummy things to eat and/or drink. You're a producer if you grow it, raise it, brew it, bake it, ferment it, milk it or mould it." registration_type_no_producer_help: "If you’re not a producer, you’re probably someone who sells and distributes food. You might be a hub, coop, buying group, retailer, wholesaler or other." # END - create_profile: "Create Profile" + registration_images_headline: "Thanks!" registration_images_description: "Let's upload some pretty pictures so your profile looks great! :)" From 86f2d7a34460560d51f2e7efd5975e2aa2e475f9 Mon Sep 17 00:00:00 2001 From: Maikel Linke Date: Tue, 26 Jun 2018 07:31:21 +1000 Subject: [PATCH 48/71] Remove impossible route Hash fragments are not part of the http request and not sent to the server. So the routes will never see a hash fragment which means that this route is never used. --- config/routes.rb | 1 - 1 file changed, 1 deletion(-) diff --git a/config/routes.rb b/config/routes.rb index 913b09da67..7e2bc47a32 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -8,7 +8,6 @@ Openfoodnetwork::Application.routes.draw do get "/t/products/:id", to: redirect("/") get "/about_us", to: redirect(ContentConfig.footer_about_url) - get "/#/login", to: "home#index", as: :spree_login get "/login", to: redirect("/#/login") get "/discourse/login", to: "discourse_sso#login" From 97d1b5d7fe14e1b0a4ab6af241d1d311da167c74 Mon Sep 17 00:00:00 2001 From: Maikel Linke Date: Fri, 15 Jun 2018 16:43:22 +1000 Subject: [PATCH 49/71] Add spec for creating users --- spec/features/admin/users_spec.rb | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) create mode 100644 spec/features/admin/users_spec.rb diff --git a/spec/features/admin/users_spec.rb b/spec/features/admin/users_spec.rb new file mode 100644 index 0000000000..de62c84aa6 --- /dev/null +++ b/spec/features/admin/users_spec.rb @@ -0,0 +1,22 @@ +require "spec_helper" + +feature "Managing users" do + include AuthenticationWorkflow + + context "as super-admin" do + before { quick_login_as_admin } + + describe "creating a user" do + it "works" do + visit spree.new_admin_user_path + fill_in "Email", with: "user1@example.org" + fill_in "Password", with: "user1Secret" + fill_in "Confirm Password", with: "user1Secret" + expect do + click_button "Create" + end.to change { Spree::User.count }.by 1 + expect(page).to have_text "Created Successfully" + end + end + end +end From 652cc6e677e88cd7419a60871ed67d2b19d2eb13 Mon Sep 17 00:00:00 2001 From: Maikel Linke Date: Thu, 7 Jun 2018 14:08:15 +1000 Subject: [PATCH 50/71] Import admin user edit view from spree_auth_devise We would like to do some customisations and importing the whole file is simpler and less error prone than using Spree's overrides. --- app/views/spree/admin/users/edit.html.erb | 28 +++++++++++++++++++++++ 1 file changed, 28 insertions(+) create mode 100644 app/views/spree/admin/users/edit.html.erb diff --git a/app/views/spree/admin/users/edit.html.erb b/app/views/spree/admin/users/edit.html.erb new file mode 100644 index 0000000000..1dccf3bb38 --- /dev/null +++ b/app/views/spree/admin/users/edit.html.erb @@ -0,0 +1,28 @@ + <% content_for :page_title do %> + <%= Spree.t(:editing_user) %> + <% end %> + + <% content_for :page_actions do %> +
  • + <%= button_link_to Spree.t(:back_to_users_list), spree.admin_users_path, :icon => 'icon-arrow-left' %> +
    • + <% end %> + +
    + <%= Spree.t(:general_settings) %> + +
    + <%= render :partial => 'spree/shared/error_messages', :locals => { :target => @user } %> +
    + +
    + <%= form_for [:admin, @user] do |f| %> + <%= render :partial => 'form', :locals => { :f => f } %> + +
    + <%= render :partial => 'spree/admin/shared/edit_resource_links' %> +
    + <% end %> +
    + +
    \ No newline at end of file From 23dd09a8fa9403616d29605358694001afefd4da Mon Sep 17 00:00:00 2001 From: Maikel Linke Date: Thu, 7 Jun 2018 14:17:53 +1000 Subject: [PATCH 51/71] Convert Spree view from erb to haml --- app/views/spree/admin/users/edit.html.erb | 28 ---------------------- app/views/spree/admin/users/edit.html.haml | 14 +++++++++++ 2 files changed, 14 insertions(+), 28 deletions(-) delete mode 100644 app/views/spree/admin/users/edit.html.erb create mode 100644 app/views/spree/admin/users/edit.html.haml diff --git a/app/views/spree/admin/users/edit.html.erb b/app/views/spree/admin/users/edit.html.erb deleted file mode 100644 index 1dccf3bb38..0000000000 --- a/app/views/spree/admin/users/edit.html.erb +++ /dev/null @@ -1,28 +0,0 @@ - <% content_for :page_title do %> - <%= Spree.t(:editing_user) %> - <% end %> - - <% content_for :page_actions do %> -
  • - <%= button_link_to Spree.t(:back_to_users_list), spree.admin_users_path, :icon => 'icon-arrow-left' %> -
    • - <% end %> - -
    - <%= Spree.t(:general_settings) %> - -
    - <%= render :partial => 'spree/shared/error_messages', :locals => { :target => @user } %> -
    - -
    - <%= form_for [:admin, @user] do |f| %> - <%= render :partial => 'form', :locals => { :f => f } %> - -
    - <%= render :partial => 'spree/admin/shared/edit_resource_links' %> -
    - <% end %> -
    - -
    \ No newline at end of file diff --git a/app/views/spree/admin/users/edit.html.haml b/app/views/spree/admin/users/edit.html.haml new file mode 100644 index 0000000000..93ef949000 --- /dev/null +++ b/app/views/spree/admin/users/edit.html.haml @@ -0,0 +1,14 @@ +- content_for :page_title do + = Spree.t(:editing_user) +- content_for :page_actions do + %li + = button_link_to Spree.t(:back_to_users_list), spree.admin_users_path, icon: "icon-arrow-left" +%fieldset.alpha.ten.columns{"data-hook" => "admin_user_edit_general_settings"} + %legend= Spree.t(:general_settings) + %div{"data-hook" => "admin_user_edit_form_header"} + = render partial: "spree/shared/error_messages", locals: { target: @user } + %div{"data-hook" => "admin_user_edit_form"} + = form_for [:admin, @user] do |f| + = render partial: "form", locals: { f: f } + %div{"data-hook" => "admin_user_edit_form_button"} + = render partial: "spree/admin/shared/edit_resource_links" From cb1dc6e657e56cb01de4004525a4cdc8fc44ea69 Mon Sep 17 00:00:00 2001 From: Maikel Linke Date: Thu, 7 Jun 2018 14:25:46 +1000 Subject: [PATCH 52/71] Import admin user edit form from spree_auth_devise --- app/views/spree/admin/users/_form.html.erb | 37 ++++++++++++++++++++++ 1 file changed, 37 insertions(+) create mode 100644 app/views/spree/admin/users/_form.html.erb diff --git a/app/views/spree/admin/users/_form.html.erb b/app/views/spree/admin/users/_form.html.erb new file mode 100644 index 0000000000..fb48bd1742 --- /dev/null +++ b/app/views/spree/admin/users/_form.html.erb @@ -0,0 +1,37 @@ +
    +
    + <%= f.field_container :email do %> + <%= f.label :email, Spree.t(:email) %> + <%= f.email_field :email, :class => 'fullwidth' %> + <%= error_message_on :user, :email %> + <% end %> + +
    + <%= label_tag nil, Spree.t(:roles) %> +
      + <% @roles.each do |role| %> +
    • + <%= check_box_tag 'user[spree_role_ids][]', role.id, @user.spree_roles.include?(role), :id => "user_spree_role_#{role.name}" %> + <%= label_tag role.name %> +
      • + <% end %> +
    + <%= hidden_field_tag 'user[spree_role_ids][]', '' %> +
    + +
    + +
    + <%= f.field_container :password do %> + <%= f.label :password, Spree.t(:password) %> + <%= f.password_field :password, :class => 'fullwidth' %> + <%= f.error_message_on :password %> + <% end %> + + <%= f.field_container :password do %> + <%= f.label :password_confirmation, Spree.t(:confirm_password) %> + <%= f.password_field :password_confirmation, :class => 'fullwidth' %> + <%= f.error_message_on :password_confirmation %> + <% end %> +
    +
    \ No newline at end of file From 2032ed63283dfb77257206c2d812c3d72c391332 Mon Sep 17 00:00:00 2001 From: Maikel Linke Date: Thu, 7 Jun 2018 14:29:26 +1000 Subject: [PATCH 53/71] Convert admin user edit form to haml --- app/views/spree/admin/users/_form.html.erb | 37 --------------------- app/views/spree/admin/users/_form.html.haml | 23 +++++++++++++ 2 files changed, 23 insertions(+), 37 deletions(-) delete mode 100644 app/views/spree/admin/users/_form.html.erb create mode 100644 app/views/spree/admin/users/_form.html.haml diff --git a/app/views/spree/admin/users/_form.html.erb b/app/views/spree/admin/users/_form.html.erb deleted file mode 100644 index fb48bd1742..0000000000 --- a/app/views/spree/admin/users/_form.html.erb +++ /dev/null @@ -1,37 +0,0 @@ -
    -
    - <%= f.field_container :email do %> - <%= f.label :email, Spree.t(:email) %> - <%= f.email_field :email, :class => 'fullwidth' %> - <%= error_message_on :user, :email %> - <% end %> - -
    - <%= label_tag nil, Spree.t(:roles) %> -
      - <% @roles.each do |role| %> -
    • - <%= check_box_tag 'user[spree_role_ids][]', role.id, @user.spree_roles.include?(role), :id => "user_spree_role_#{role.name}" %> - <%= label_tag role.name %> -
      • - <% end %> -
    - <%= hidden_field_tag 'user[spree_role_ids][]', '' %> -
    - -
    - -
    - <%= f.field_container :password do %> - <%= f.label :password, Spree.t(:password) %> - <%= f.password_field :password, :class => 'fullwidth' %> - <%= f.error_message_on :password %> - <% end %> - - <%= f.field_container :password do %> - <%= f.label :password_confirmation, Spree.t(:confirm_password) %> - <%= f.password_field :password_confirmation, :class => 'fullwidth' %> - <%= f.error_message_on :password_confirmation %> - <% end %> -
    -
    \ No newline at end of file diff --git a/app/views/spree/admin/users/_form.html.haml b/app/views/spree/admin/users/_form.html.haml new file mode 100644 index 0000000000..97e4a5d848 --- /dev/null +++ b/app/views/spree/admin/users/_form.html.haml @@ -0,0 +1,23 @@ +.row{"data-hook" => "admin_user_form_fields"} + .alpha.five.columns + = f.field_container :email do + = f.label :email, Spree.t(:email) + = f.email_field :email, class: "fullwidth" + = error_message_on :user, :email + .field{"data-hook" => "admin_user_form_roles"} + = label_tag nil, Spree.t(:roles) + %ul + - @roles.each do |role| + %li + = check_box_tag "user[spree_role_ids][]", role.id, @user.spree_roles.include?(role), id: "user_spree_role_#{role.name}" + = label_tag role.name + = hidden_field_tag "user[spree_role_ids][]", "" + .omega.five.columns + = f.field_container :password do + = f.label :password, Spree.t(:password) + = f.password_field :password, class: "fullwidth" + = f.error_message_on :password + = f.field_container :password do + = f.label :password_confirmation, Spree.t(:confirm_password) + = f.password_field :password_confirmation, class: "fullwidth" + = f.error_message_on :password_confirmation \ No newline at end of file From 9050a52cc2dfad03431e28e1e769fb406c43ceea Mon Sep 17 00:00:00 2001 From: Maikel Linke Date: Thu, 7 Jun 2018 14:35:12 +1000 Subject: [PATCH 54/71] Move admin user form override into view --- .../_form/add_enterprise_limit_form_element.html.haml.deface | 5 ----- app/views/spree/admin/users/_form.html.haml | 3 +++ 2 files changed, 3 insertions(+), 5 deletions(-) delete mode 100644 app/overrides/spree/admin/users/_form/add_enterprise_limit_form_element.html.haml.deface diff --git a/app/overrides/spree/admin/users/_form/add_enterprise_limit_form_element.html.haml.deface b/app/overrides/spree/admin/users/_form/add_enterprise_limit_form_element.html.haml.deface deleted file mode 100644 index 0b00900b5c..0000000000 --- a/app/overrides/spree/admin/users/_form/add_enterprise_limit_form_element.html.haml.deface +++ /dev/null @@ -1,5 +0,0 @@ -/ insert_bottom "div[data-hook='admin_user_form_fields'] div.alpha" - -= f.field_container :enterprise_limit do - = f.label :enterprise_limit, t(:enterprise_limit) - = f.text_field :enterprise_limit, :class => 'fullwidth' \ No newline at end of file diff --git a/app/views/spree/admin/users/_form.html.haml b/app/views/spree/admin/users/_form.html.haml index 97e4a5d848..a11624cfa2 100644 --- a/app/views/spree/admin/users/_form.html.haml +++ b/app/views/spree/admin/users/_form.html.haml @@ -12,6 +12,9 @@ = check_box_tag "user[spree_role_ids][]", role.id, @user.spree_roles.include?(role), id: "user_spree_role_#{role.name}" = label_tag role.name = hidden_field_tag "user[spree_role_ids][]", "" + = f.field_container :enterprise_limit do + = f.label :enterprise_limit, t(:enterprise_limit) + = f.text_field :enterprise_limit, class: "fullwidth" .omega.five.columns = f.field_container :password do = f.label :password, Spree.t(:password) From 8157897f766697fe4f232c1cc326b54ecc2a8226 Mon Sep 17 00:00:00 2001 From: Maikel Linke Date: Thu, 7 Jun 2018 14:36:31 +1000 Subject: [PATCH 55/71] Remove unused data hooks --- app/views/spree/admin/users/_form.html.haml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/views/spree/admin/users/_form.html.haml b/app/views/spree/admin/users/_form.html.haml index a11624cfa2..e495f29632 100644 --- a/app/views/spree/admin/users/_form.html.haml +++ b/app/views/spree/admin/users/_form.html.haml @@ -1,10 +1,10 @@ -.row{"data-hook" => "admin_user_form_fields"} +.row .alpha.five.columns = f.field_container :email do = f.label :email, Spree.t(:email) = f.email_field :email, class: "fullwidth" = error_message_on :user, :email - .field{"data-hook" => "admin_user_form_roles"} + .field = label_tag nil, Spree.t(:roles) %ul - @roles.each do |role| From 6a58deb436b853e5b731c67c76e71877556f5fad Mon Sep 17 00:00:00 2001 From: Maikel Linke Date: Thu, 7 Jun 2018 15:01:41 +1000 Subject: [PATCH 56/71] Add message about unconfirmed email to user form --- app/views/spree/admin/users/_email_confirmation.html.haml | 2 ++ app/views/spree/admin/users/edit.html.haml | 1 + config/locales/en.yml | 3 +++ spec/features/admin/users_spec.rb | 6 ++++++ 4 files changed, 12 insertions(+) create mode 100644 app/views/spree/admin/users/_email_confirmation.html.haml diff --git a/app/views/spree/admin/users/_email_confirmation.html.haml b/app/views/spree/admin/users/_email_confirmation.html.haml new file mode 100644 index 0000000000..65be495337 --- /dev/null +++ b/app/views/spree/admin/users/_email_confirmation.html.haml @@ -0,0 +1,2 @@ +%p.alert-box + = t(".confirmation_pending", address: @user.email) diff --git a/app/views/spree/admin/users/edit.html.haml b/app/views/spree/admin/users/edit.html.haml index 93ef949000..1457f1065a 100644 --- a/app/views/spree/admin/users/edit.html.haml +++ b/app/views/spree/admin/users/edit.html.haml @@ -9,6 +9,7 @@ = render partial: "spree/shared/error_messages", locals: { target: @user } %div{"data-hook" => "admin_user_edit_form"} = form_for [:admin, @user] do |f| + = render "email_confirmation" unless @user.confirmed? = render partial: "form", locals: { f: f } %div{"data-hook" => "admin_user_edit_form_button"} = render partial: "spree/admin/shared/edit_resource_links" diff --git a/config/locales/en.yml b/config/locales/en.yml index 0ebf5200c8..8b06972e95 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -2585,6 +2585,9 @@ See the %{link} to find out more about %{sitename}'s features and to start using shared: configuration_menu: stripe_connect: Stripe Connect + users: + email_confirmation: + confirmation_pending: "Email confirmation is pending. We've sent a confirmation email to %{address}." variants: autocomplete: producer_name: Producer diff --git a/spec/features/admin/users_spec.rb b/spec/features/admin/users_spec.rb index de62c84aa6..2e78a19500 100644 --- a/spec/features/admin/users_spec.rb +++ b/spec/features/admin/users_spec.rb @@ -7,6 +7,11 @@ feature "Managing users" do before { quick_login_as_admin } describe "creating a user" do + it "shows no confirmation message to start with" do + visit spree.new_admin_user_path + expect(page).to have_no_text "Email confirmation is pending" + end + it "works" do visit spree.new_admin_user_path fill_in "Email", with: "user1@example.org" @@ -16,6 +21,7 @@ feature "Managing users" do click_button "Create" end.to change { Spree::User.count }.by 1 expect(page).to have_text "Created Successfully" + expect(page).to have_text "Email confirmation is pending" end end end From 60b66540df241e2e0025b4c054a05029208eb15d Mon Sep 17 00:00:00 2001 From: Maikel Linke Date: Thu, 7 Jun 2018 18:09:35 +1000 Subject: [PATCH 57/71] Add resend email button to admin user form Fixes https://github.com/openfoodfoundation/openfoodnetwork/issues/1589 --- .../resend_user_email_confirmation.js.coffee | 19 +++++++++++++++++++ .../admin/openfoodnetwork.css.scss | 8 ++++++++ .../admin/users/_email_confirmation.html.haml | 4 +++- config/locales/en.yml | 5 +++++ 4 files changed, 35 insertions(+), 1 deletion(-) create mode 100644 app/assets/javascripts/admin/users/directives/resend_user_email_confirmation.js.coffee diff --git a/app/assets/javascripts/admin/users/directives/resend_user_email_confirmation.js.coffee b/app/assets/javascripts/admin/users/directives/resend_user_email_confirmation.js.coffee new file mode 100644 index 0000000000..b6361ec7ae --- /dev/null +++ b/app/assets/javascripts/admin/users/directives/resend_user_email_confirmation.js.coffee @@ -0,0 +1,19 @@ +angular.module("admin.users").directive "resendUserEmailConfirmation", ($http) -> + scope: + email: "@resendUserEmailConfirmation" + link: (scope, element, attrs) -> + sent = false + text = element.text() + sending = " " + t "js.admin.resend_user_email_confirmation.sending" + done = " " + t "js.admin.resend_user_email_confirmation.done" + failed = " " + t "js.admin.resend_user_email_confirmation.failed" + + element.bind "click", -> + return if sent + element.text(text + sending) + $http.post("/user/spree_user/confirmation", {spree_user: {email: scope.email}}).success (data) -> + sent = true + element.addClass "action--disabled" + element.text text + done + .error (data) -> + element.text text + failed diff --git a/app/assets/stylesheets/admin/openfoodnetwork.css.scss b/app/assets/stylesheets/admin/openfoodnetwork.css.scss index d10ca748d0..348b85820a 100644 --- a/app/assets/stylesheets/admin/openfoodnetwork.css.scss +++ b/app/assets/stylesheets/admin/openfoodnetwork.css.scss @@ -71,6 +71,14 @@ a { cursor:pointer; } +a.action--disabled { + cursor: default; + + &:hover { + color: #5498da; + } +} + form.order_cycle { h2 { margin-top: 2em; diff --git a/app/views/spree/admin/users/_email_confirmation.html.haml b/app/views/spree/admin/users/_email_confirmation.html.haml index 65be495337..fb2fd75232 100644 --- a/app/views/spree/admin/users/_email_confirmation.html.haml +++ b/app/views/spree/admin/users/_email_confirmation.html.haml @@ -1,2 +1,4 @@ -%p.alert-box +%p.alert-box{"ng-app" => "admin.users"} = t(".confirmation_pending", address: @user.email) + %a{"resend-user-email-confirmation" => @user.email} + = t(".resend") \ No newline at end of file diff --git a/config/locales/en.yml b/config/locales/en.yml index 8b06972e95..675ea554a5 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -2435,6 +2435,10 @@ See the %{link} to find out more about %{sitename}'s features and to start using resolve: Resolve new_tag_rule_dialog: select_rule_type: "Select a rule type:" + resend_user_email_confirmation: + sending: "..." + done: "done ✓" + failed: "failed ✗" out_of_stock: reduced_stock_available: Reduced stock available out_of_stock_text: > @@ -2588,6 +2592,7 @@ See the %{link} to find out more about %{sitename}'s features and to start using users: email_confirmation: confirmation_pending: "Email confirmation is pending. We've sent a confirmation email to %{address}." + resend: "Resend" variants: autocomplete: producer_name: Producer From 9020e7ddd99f67a2668a09785a54cdf55d965591 Mon Sep 17 00:00:00 2001 From: Maikel Linke Date: Thu, 7 Jun 2018 18:11:52 +1000 Subject: [PATCH 58/71] Import admin user list from Spree --- app/views/spree/admin/users/index.html.erb | 53 ++++++++++++++++++++++ 1 file changed, 53 insertions(+) create mode 100644 app/views/spree/admin/users/index.html.erb diff --git a/app/views/spree/admin/users/index.html.erb b/app/views/spree/admin/users/index.html.erb new file mode 100644 index 0000000000..a5373244e1 --- /dev/null +++ b/app/views/spree/admin/users/index.html.erb @@ -0,0 +1,53 @@ +<% content_for :page_title do %> + <%= Spree.t(:listing_users) %> +<% end %> + +<% content_for :page_actions do %> +
  • + <%= button_link_to Spree.t(:new_user), new_object_url, :icon => 'icon-plus', :id => 'admin_new_user_link' %> +
    • +<% end %> + + + + + + + + + + + + + + <% @users.each do |user|%> + + + + + <% end %> + +
    <%= sort_link @search,:email, Spree.t(:user), {}, {:title => 'users_email_title'} %>
    <%=link_to user.email, object_url(user) %> + <%= link_to_edit user, :no_text => true %> + <%= link_to_delete user, :no_text => true %> +
    + +<%= paginate @users %> + +<% content_for :sidebar_title do %> + <%= Spree.t(:search) %> +<% end %> + +<% content_for :sidebar do %> +
    + <%= search_form_for [:admin, @search] do |f| %> +
    + <%= f.label Spree.t(:email) %>
    + <%= f.text_field :email_cont, :class => 'fullwidth' %> +
    +
    + <%= button Spree.t(:search), 'icon-search' %> +
    + <% end %> +
    +<% end %> From 339b04a0445c75be65031ea0d6254fa0ff3554e8 Mon Sep 17 00:00:00 2001 From: Maikel Linke Date: Fri, 8 Jun 2018 15:00:55 +1000 Subject: [PATCH 59/71] Convert spree admin user list to haml --- app/views/spree/admin/users/index.html.erb | 53 --------------------- app/views/spree/admin/users/index.html.haml | 35 ++++++++++++++ 2 files changed, 35 insertions(+), 53 deletions(-) delete mode 100644 app/views/spree/admin/users/index.html.erb create mode 100644 app/views/spree/admin/users/index.html.haml diff --git a/app/views/spree/admin/users/index.html.erb b/app/views/spree/admin/users/index.html.erb deleted file mode 100644 index a5373244e1..0000000000 --- a/app/views/spree/admin/users/index.html.erb +++ /dev/null @@ -1,53 +0,0 @@ -<% content_for :page_title do %> - <%= Spree.t(:listing_users) %> -<% end %> - -<% content_for :page_actions do %> -
  • - <%= button_link_to Spree.t(:new_user), new_object_url, :icon => 'icon-plus', :id => 'admin_new_user_link' %> -
    • -<% end %> - - - - - - - - - - - - - - <% @users.each do |user|%> - - - - - <% end %> - -
    <%= sort_link @search,:email, Spree.t(:user), {}, {:title => 'users_email_title'} %>
    <%=link_to user.email, object_url(user) %> - <%= link_to_edit user, :no_text => true %> - <%= link_to_delete user, :no_text => true %> -
    - -<%= paginate @users %> - -<% content_for :sidebar_title do %> - <%= Spree.t(:search) %> -<% end %> - -<% content_for :sidebar do %> -
    - <%= search_form_for [:admin, @search] do |f| %> -
    - <%= f.label Spree.t(:email) %>
    - <%= f.text_field :email_cont, :class => 'fullwidth' %> -
    -
    - <%= button Spree.t(:search), 'icon-search' %> -
    - <% end %> -
    -<% end %> diff --git a/app/views/spree/admin/users/index.html.haml b/app/views/spree/admin/users/index.html.haml new file mode 100644 index 0000000000..023cb11ec6 --- /dev/null +++ b/app/views/spree/admin/users/index.html.haml @@ -0,0 +1,35 @@ +- content_for :page_title do + = Spree.t(:listing_users) +- content_for :page_actions do + %li + = button_link_to Spree.t(:new_user), new_object_url, icon: "icon-plus", id: "admin_new_user_link" +%table#listing_users.index{"data-hook" => ""} + %colgroup + %col{style: "width: 85%"} + %col{style: "width: 15%"} + %thead + %tr{"data-hook" => "admin_users_index_headers"} + %th= sort_link @search,:email, Spree.t(:user), {}, {title: "users_email_title"} + %th.actions{"data-hook" => "admin_users_index_header_actions"} + %tbody + - @users.each do |user| + - # HAML seems to have a bug that it can't parse `class cycle('odd', 'even')` on the element. + - # So we assign it first: + - row_class = cycle("odd", "even") + %tr{id: spree_dom_id(user), "data-hook" => "admin_users_index_rows", class: row_class} + %td.user_email= link_to user.email, object_url(user) + %td.actions{"data-hook" => "admin_users_index_row_actions"} + = link_to_edit user, no_text: true + = link_to_delete user, no_text: true += paginate @users +- content_for :sidebar_title do + = Spree.t(:search) +- content_for :sidebar do + .box.align-center{"data-hook" => "admin_users_index_search"} + = search_form_for [:admin, @search] do |f| + .field + = f.label Spree.t(:email) + %br + = f.text_field :email_cont, class: "fullwidth" + %div{"data-hook" => "admin_users_index_search_buttons"} + = button Spree.t(:search), "icon-search" From ab9975cb6cdd994eb1cf1c92daa15eedb63dad27 Mon Sep 17 00:00:00 2001 From: Maikel Linke Date: Fri, 8 Jun 2018 15:04:33 +1000 Subject: [PATCH 60/71] Move override into view --- .../index/add_enterprise_limit_column.html.haml.deface | 3 --- .../add_enterprise_limit_column_header.html.haml.deface | 3 --- .../index/reconfigure_column_spacing.html.haml.deface | 6 ------ app/views/spree/admin/users/index.html.haml | 7 +++++-- 4 files changed, 5 insertions(+), 14 deletions(-) delete mode 100644 app/overrides/spree/admin/users/index/add_enterprise_limit_column.html.haml.deface delete mode 100644 app/overrides/spree/admin/users/index/add_enterprise_limit_column_header.html.haml.deface delete mode 100644 app/overrides/spree/admin/users/index/reconfigure_column_spacing.html.haml.deface diff --git a/app/overrides/spree/admin/users/index/add_enterprise_limit_column.html.haml.deface b/app/overrides/spree/admin/users/index/add_enterprise_limit_column.html.haml.deface deleted file mode 100644 index d16e186be8..0000000000 --- a/app/overrides/spree/admin/users/index/add_enterprise_limit_column.html.haml.deface +++ /dev/null @@ -1,3 +0,0 @@ -/ insert_before "td[data-hook='admin_users_index_row_actions']" - -%td.user_enterprise_limit= user.enterprise_limit \ No newline at end of file diff --git a/app/overrides/spree/admin/users/index/add_enterprise_limit_column_header.html.haml.deface b/app/overrides/spree/admin/users/index/add_enterprise_limit_column_header.html.haml.deface deleted file mode 100644 index f2222ef012..0000000000 --- a/app/overrides/spree/admin/users/index/add_enterprise_limit_column_header.html.haml.deface +++ /dev/null @@ -1,3 +0,0 @@ -/ insert_before "th[data-hook='admin_users_index_header_actions']" - -%th= sort_link @search,:enterprise_limit, t(:enterprise_limit) \ No newline at end of file diff --git a/app/overrides/spree/admin/users/index/reconfigure_column_spacing.html.haml.deface b/app/overrides/spree/admin/users/index/reconfigure_column_spacing.html.haml.deface deleted file mode 100644 index d666e1b7c5..0000000000 --- a/app/overrides/spree/admin/users/index/reconfigure_column_spacing.html.haml.deface +++ /dev/null @@ -1,6 +0,0 @@ -/ replace "table#listing_users colgroup" - -%colgroup - %col{ style: "width: 65%" } - %col{ style: "width: 20%" } - %col{ style: "width: 15%" } \ No newline at end of file diff --git a/app/views/spree/admin/users/index.html.haml b/app/views/spree/admin/users/index.html.haml index 023cb11ec6..854e6f1a08 100644 --- a/app/views/spree/admin/users/index.html.haml +++ b/app/views/spree/admin/users/index.html.haml @@ -5,11 +5,13 @@ = button_link_to Spree.t(:new_user), new_object_url, icon: "icon-plus", id: "admin_new_user_link" %table#listing_users.index{"data-hook" => ""} %colgroup - %col{style: "width: 85%"} - %col{style: "width: 15%"} + %col{ style: "width: 65%" } + %col{ style: "width: 20%" } + %col{ style: "width: 15%" } %thead %tr{"data-hook" => "admin_users_index_headers"} %th= sort_link @search,:email, Spree.t(:user), {}, {title: "users_email_title"} + %th= sort_link @search,:enterprise_limit, t(:enterprise_limit) %th.actions{"data-hook" => "admin_users_index_header_actions"} %tbody - @users.each do |user| @@ -18,6 +20,7 @@ - row_class = cycle("odd", "even") %tr{id: spree_dom_id(user), "data-hook" => "admin_users_index_rows", class: row_class} %td.user_email= link_to user.email, object_url(user) + %td.user_enterprise_limit= user.enterprise_limit %td.actions{"data-hook" => "admin_users_index_row_actions"} = link_to_edit user, no_text: true = link_to_delete user, no_text: true From a347837044227dcdb62c4d4d3d2b4f4664d56fc0 Mon Sep 17 00:00:00 2001 From: Maikel Linke Date: Fri, 8 Jun 2018 15:08:32 +1000 Subject: [PATCH 61/71] Move override into view --- .../spree/admin/users/index/add_roles_link.html.haml.deface | 3 --- app/views/spree/admin/users/index.html.haml | 3 +++ 2 files changed, 3 insertions(+), 3 deletions(-) delete mode 100644 app/overrides/spree/admin/users/index/add_roles_link.html.haml.deface diff --git a/app/overrides/spree/admin/users/index/add_roles_link.html.haml.deface b/app/overrides/spree/admin/users/index/add_roles_link.html.haml.deface deleted file mode 100644 index f99a1ebff4..0000000000 --- a/app/overrides/spree/admin/users/index/add_roles_link.html.haml.deface +++ /dev/null @@ -1,3 +0,0 @@ -/ insert_before "table#listing_users" - -= render 'admin/shared/users_sub_menu' \ No newline at end of file diff --git a/app/views/spree/admin/users/index.html.haml b/app/views/spree/admin/users/index.html.haml index 854e6f1a08..1628126d97 100644 --- a/app/views/spree/admin/users/index.html.haml +++ b/app/views/spree/admin/users/index.html.haml @@ -3,6 +3,9 @@ - content_for :page_actions do %li = button_link_to Spree.t(:new_user), new_object_url, icon: "icon-plus", id: "admin_new_user_link" + += render "admin/shared/users_sub_menu" + %table#listing_users.index{"data-hook" => ""} %colgroup %col{ style: "width: 65%" } From 28c2ad71604084a32ac3b3075b465794eb147818 Mon Sep 17 00:00:00 2001 From: Maikel Linke Date: Fri, 8 Jun 2018 15:13:01 +1000 Subject: [PATCH 62/71] Move override into view --- .../index/replace_show_link_with_edit_link.html.haml.deface | 3 --- app/views/spree/admin/users/index.html.haml | 2 +- 2 files changed, 1 insertion(+), 4 deletions(-) delete mode 100644 app/overrides/spree/admin/users/index/replace_show_link_with_edit_link.html.haml.deface diff --git a/app/overrides/spree/admin/users/index/replace_show_link_with_edit_link.html.haml.deface b/app/overrides/spree/admin/users/index/replace_show_link_with_edit_link.html.haml.deface deleted file mode 100644 index 330e06ea9d..0000000000 --- a/app/overrides/spree/admin/users/index/replace_show_link_with_edit_link.html.haml.deface +++ /dev/null @@ -1,3 +0,0 @@ -/ replace "code[erb-loud]:contains('link_to user.email, object_url(user)')" - -= link_to user.email, edit_object_url(user) \ No newline at end of file diff --git a/app/views/spree/admin/users/index.html.haml b/app/views/spree/admin/users/index.html.haml index 1628126d97..91bffa6c96 100644 --- a/app/views/spree/admin/users/index.html.haml +++ b/app/views/spree/admin/users/index.html.haml @@ -22,7 +22,7 @@ - # So we assign it first: - row_class = cycle("odd", "even") %tr{id: spree_dom_id(user), "data-hook" => "admin_users_index_rows", class: row_class} - %td.user_email= link_to user.email, object_url(user) + %td.user_email= link_to user.email, edit_object_url(user) %td.user_enterprise_limit= user.enterprise_limit %td.actions{"data-hook" => "admin_users_index_row_actions"} = link_to_edit user, no_text: true From e37213f6a5c6088f5d14810f54922fc3cb05be48 Mon Sep 17 00:00:00 2001 From: Maikel Linke Date: Fri, 8 Jun 2018 15:14:26 +1000 Subject: [PATCH 63/71] Remove redundant edit action --- app/views/spree/admin/users/index.html.haml | 1 - 1 file changed, 1 deletion(-) diff --git a/app/views/spree/admin/users/index.html.haml b/app/views/spree/admin/users/index.html.haml index 91bffa6c96..8b3633887e 100644 --- a/app/views/spree/admin/users/index.html.haml +++ b/app/views/spree/admin/users/index.html.haml @@ -25,7 +25,6 @@ %td.user_email= link_to user.email, edit_object_url(user) %td.user_enterprise_limit= user.enterprise_limit %td.actions{"data-hook" => "admin_users_index_row_actions"} - = link_to_edit user, no_text: true = link_to_delete user, no_text: true = paginate @users - content_for :sidebar_title do From a531135804f0a5e66f93cd4d3e6b9e33e0e34e9c Mon Sep 17 00:00:00 2001 From: Maikel Linke Date: Fri, 8 Jun 2018 15:17:15 +1000 Subject: [PATCH 64/71] Remove unused data-hooks --- app/views/spree/admin/users/index.html.haml | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/app/views/spree/admin/users/index.html.haml b/app/views/spree/admin/users/index.html.haml index 8b3633887e..e7ef05d711 100644 --- a/app/views/spree/admin/users/index.html.haml +++ b/app/views/spree/admin/users/index.html.haml @@ -6,35 +6,35 @@ = render "admin/shared/users_sub_menu" -%table#listing_users.index{"data-hook" => ""} +%table#listing_users.index %colgroup %col{ style: "width: 65%" } %col{ style: "width: 20%" } %col{ style: "width: 15%" } %thead - %tr{"data-hook" => "admin_users_index_headers"} + %tr %th= sort_link @search,:email, Spree.t(:user), {}, {title: "users_email_title"} %th= sort_link @search,:enterprise_limit, t(:enterprise_limit) - %th.actions{"data-hook" => "admin_users_index_header_actions"} + %th.actions %tbody - @users.each do |user| - # HAML seems to have a bug that it can't parse `class cycle('odd', 'even')` on the element. - # So we assign it first: - row_class = cycle("odd", "even") - %tr{id: spree_dom_id(user), "data-hook" => "admin_users_index_rows", class: row_class} + %tr{id: spree_dom_id(user), class: row_class} %td.user_email= link_to user.email, edit_object_url(user) %td.user_enterprise_limit= user.enterprise_limit - %td.actions{"data-hook" => "admin_users_index_row_actions"} + %td.actions = link_to_delete user, no_text: true = paginate @users - content_for :sidebar_title do = Spree.t(:search) - content_for :sidebar do - .box.align-center{"data-hook" => "admin_users_index_search"} + .box.align-center = search_form_for [:admin, @search] do |f| .field = f.label Spree.t(:email) %br = f.text_field :email_cont, class: "fullwidth" - %div{"data-hook" => "admin_users_index_search_buttons"} + %div = button Spree.t(:search), "icon-search" From fbe2f7ab4c586d4fd99f93a7b746a35585672b97 Mon Sep 17 00:00:00 2001 From: Maikel Linke Date: Fri, 8 Jun 2018 17:52:59 +1000 Subject: [PATCH 65/71] Remove unused variable from mailer The second variable passed by Devise is actually a hash, not a token. --- app/mailers/spree/user_mailer_decorator.rb | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/app/mailers/spree/user_mailer_decorator.rb b/app/mailers/spree/user_mailer_decorator.rb index 991db0074c..778f4d421a 100644 --- a/app/mailers/spree/user_mailer_decorator.rb +++ b/app/mailers/spree/user_mailer_decorator.rb @@ -5,9 +5,10 @@ Spree::UserMailer.class_eval do :subject => t(:welcome_to) + Spree::Config[:site_name]) end - def confirmation_instructions(user, token) + # Overriding `Spree::UserMailer.confirmation_instructions` which is + # overriding `Devise::Mailer.confirmation_instructions`. + def confirmation_instructions(user, _opts) @user = user - @token = token subject = t('spree.user_mailer.confirmation_instructions.subject') mail(to: user.email, from: from_address, From 43883d1ab46c606d260e83061658fe41250ff920 Mon Sep 17 00:00:00 2001 From: Maikel Linke Date: Wed, 20 Jun 2018 14:40:48 +1000 Subject: [PATCH 66/71] Simplify email confirmation directive Making better use of Angular features. Adding a spec for this functionality. --- .../resend_user_email_confirmation.js.coffee | 12 +++++------- .../admin/users/_email_confirmation.html.haml | 3 +-- config/locales/en.yml | 8 ++++---- spec/features/admin/users_spec.rb | 15 +++++++++++++++ 4 files changed, 25 insertions(+), 13 deletions(-) diff --git a/app/assets/javascripts/admin/users/directives/resend_user_email_confirmation.js.coffee b/app/assets/javascripts/admin/users/directives/resend_user_email_confirmation.js.coffee index b6361ec7ae..a9ec8af2ec 100644 --- a/app/assets/javascripts/admin/users/directives/resend_user_email_confirmation.js.coffee +++ b/app/assets/javascripts/admin/users/directives/resend_user_email_confirmation.js.coffee @@ -1,19 +1,17 @@ angular.module("admin.users").directive "resendUserEmailConfirmation", ($http) -> + template: "{{ 'js.admin.resend_user_email_confirmation.' + status | t }}" scope: email: "@resendUserEmailConfirmation" link: (scope, element, attrs) -> sent = false - text = element.text() - sending = " " + t "js.admin.resend_user_email_confirmation.sending" - done = " " + t "js.admin.resend_user_email_confirmation.done" - failed = " " + t "js.admin.resend_user_email_confirmation.failed" + scope.status = "resend" element.bind "click", -> return if sent - element.text(text + sending) + scope.status = "sending" $http.post("/user/spree_user/confirmation", {spree_user: {email: scope.email}}).success (data) -> sent = true element.addClass "action--disabled" - element.text text + done + scope.status = "done" .error (data) -> - element.text text + failed + scope.status = "failed" diff --git a/app/views/spree/admin/users/_email_confirmation.html.haml b/app/views/spree/admin/users/_email_confirmation.html.haml index fb2fd75232..f851ea339e 100644 --- a/app/views/spree/admin/users/_email_confirmation.html.haml +++ b/app/views/spree/admin/users/_email_confirmation.html.haml @@ -1,4 +1,3 @@ %p.alert-box{"ng-app" => "admin.users"} = t(".confirmation_pending", address: @user.email) - %a{"resend-user-email-confirmation" => @user.email} - = t(".resend") \ No newline at end of file + %a{"resend-user-email-confirmation" => @user.email} \ No newline at end of file diff --git a/config/locales/en.yml b/config/locales/en.yml index 675ea554a5..67c8d17fd3 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -2436,9 +2436,10 @@ See the %{link} to find out more about %{sitename}'s features and to start using new_tag_rule_dialog: select_rule_type: "Select a rule type:" resend_user_email_confirmation: - sending: "..." - done: "done ✓" - failed: "failed ✗" + resend: "Resend" + sending: "Resend..." + done: "Resend done ✓" + failed: "Resend failed ✗" out_of_stock: reduced_stock_available: Reduced stock available out_of_stock_text: > @@ -2592,7 +2593,6 @@ See the %{link} to find out more about %{sitename}'s features and to start using users: email_confirmation: confirmation_pending: "Email confirmation is pending. We've sent a confirmation email to %{address}." - resend: "Resend" variants: autocomplete: producer_name: Producer diff --git a/spec/features/admin/users_spec.rb b/spec/features/admin/users_spec.rb index 2e78a19500..5a3666e2a8 100644 --- a/spec/features/admin/users_spec.rb +++ b/spec/features/admin/users_spec.rb @@ -24,5 +24,20 @@ feature "Managing users" do expect(page).to have_text "Email confirmation is pending" end end + + describe "resending confirmation email", js: true do + let(:user) { create :user, confirmed_at: nil } + + it "displays success" do + visit spree.edit_admin_user_path user + + # The `a` element doesn't have an href, so we can't use click_link. + find("a", text: "Resend").click + expect(page).to have_text "Resend done" + + # And it's successful. (testing it here for reduced test time) + expect(Delayed::Job.last.payload_object.method_name).to eq :send_confirmation_instructions_without_delay + end + end end end From d5d1eae715286a8121440dfaad1f2f3ce935d609 Mon Sep 17 00:00:00 2001 From: Maikel Linke Date: Fri, 29 Jun 2018 09:37:44 +1000 Subject: [PATCH 67/71] Improve spec description --- spec/features/admin/users_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/features/admin/users_spec.rb b/spec/features/admin/users_spec.rb index 5a3666e2a8..9af920577a 100644 --- a/spec/features/admin/users_spec.rb +++ b/spec/features/admin/users_spec.rb @@ -12,7 +12,7 @@ feature "Managing users" do expect(page).to have_no_text "Email confirmation is pending" end - it "works" do + it "confirms successful creation" do visit spree.new_admin_user_path fill_in "Email", with: "user1@example.org" fill_in "Password", with: "user1Secret" From 4ebcf4e3c3419863a1d774737e49456ffee41080 Mon Sep 17 00:00:00 2001 From: Kristina Lim Date: Fri, 13 Jul 2018 13:58:32 +0800 Subject: [PATCH 68/71] Add internationalisation info to CONTRIBUTING.md --- CONTRIBUTING.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 5ea968c65c..bb45f49082 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -30,6 +30,10 @@ If you want to run the whole test suite, we recommend using a free CI service to bundle exec rspec spec +## Internationalisation (i18n) + +The locale `en` is maintained in the source code, but other locales are managed at [Transifex][ofn-transifex]. Read more about [internationalisation][i18n] in the developer wiki. + ## Making a change Make your changes to the codebase. We recommend using TDD. Add a test, make changes and get the test suite back to green. @@ -66,3 +70,5 @@ From here, your pull request will progress through the [Review, Test, Merge & De [rebase]: https://www.atlassian.com/git/tutorials/merging-vs-rebasing/workflow-walkthrough [travis]: https://travis-ci.org/ [semaphore]: https://semaphoreci.com/ +[ofn-transifex]: https://www.transifex.com/open-food-foundation/open-food-network/ +[i18n]: https://github.com/openfoodfoundation/openfoodnetwork/wiki/i18n From e3b7e050a0c2a8fb39d9b057b802b7001d9333ba Mon Sep 17 00:00:00 2001 From: Frank West Date: Fri, 29 Jun 2018 06:46:30 -0700 Subject: [PATCH 69/71] Remove spree product navigation override This was here to override the original spree products navigation link, but since moving back to the index action this had the side effect of adding an additional navigation item instead. We can simply remove this override to get rid of the additional menu item. --- .../_product_sub_menu/add_products_tab.html.haml.deface | 4 ---- 1 file changed, 4 deletions(-) delete mode 100644 app/overrides/spree/admin/shared/_product_sub_menu/add_products_tab.html.haml.deface diff --git a/app/overrides/spree/admin/shared/_product_sub_menu/add_products_tab.html.haml.deface b/app/overrides/spree/admin/shared/_product_sub_menu/add_products_tab.html.haml.deface deleted file mode 100644 index 074a76f3ed..0000000000 --- a/app/overrides/spree/admin/shared/_product_sub_menu/add_products_tab.html.haml.deface +++ /dev/null @@ -1,4 +0,0 @@ -/ insert_bottom "[data-hook='admin_product_sub_tabs']" - -- if spree_current_user.admin? - = tab :spree_products, url: admin_products_path, :match_path => '/products' \ No newline at end of file From 4a444542100bdfa6f666a7c4e25c9398183770f3 Mon Sep 17 00:00:00 2001 From: luisramos0 Date: Tue, 19 Jun 2018 15:04:58 +0100 Subject: [PATCH 70/71] fixed number rounding issue in product bulk edit --- .../javascripts/admin/services/bulk_products.js.coffee | 7 ++++++- .../unit/admin/services/bulk_products_spec.js.coffee | 7 +++++++ 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/app/assets/javascripts/admin/services/bulk_products.js.coffee b/app/assets/javascripts/admin/services/bulk_products.js.coffee index 022345e9d8..a1a41b4763 100644 --- a/app/assets/javascripts/admin/services/bulk_products.js.coffee +++ b/app/assets/javascripts/admin/services/bulk_products.js.coffee @@ -66,8 +66,13 @@ angular.module("ofn.admin").factory "BulkProducts", (PagedFetcher, dataFetcher, variantUnitValue: (product, variant) -> if variant.unit_value? if product.variant_unit_scale - variant.unit_value / product.variant_unit_scale + @divideAsInteger variant.unit_value, product.variant_unit_scale else variant.unit_value else null + + # forces integer division to avoid javascript floating point imprecision + # using one billion as the multiplier so that it works for numbers with up to 9 decimal places + divideAsInteger: (a, b) -> + (a * 1000000000) / (b * 1000000000) diff --git a/spec/javascripts/unit/admin/services/bulk_products_spec.js.coffee b/spec/javascripts/unit/admin/services/bulk_products_spec.js.coffee index fc46c2a30d..95bdd63ba7 100644 --- a/spec/javascripts/unit/admin/services/bulk_products_spec.js.coffee +++ b/spec/javascripts/unit/admin/services/bulk_products_spec.js.coffee @@ -160,6 +160,13 @@ describe "BulkProducts service", -> BulkProducts.loadVariantUnitValues product, product.variants[0] expect(product.variants[0].unit_value_with_description).toEqual '2.5' + it "converts values from base value to chosen unit without breaking precision", -> + product = + variant_unit_scale: 0.001 + variants: [{id: 1, unit_value: 0.35}] + BulkProducts.loadVariantUnitValues product, product.variants[0] + expect(product.variants[0].unit_value_with_description).toEqual '350' + it "displays a unit_value of zero", -> product = variant_unit_scale: 1.0 From c16f4203f93dbc85da917251a2434c4beaeea25d Mon Sep 17 00:00:00 2001 From: Kristina Lim Date: Mon, 9 Jul 2018 14:50:05 +0800 Subject: [PATCH 71/71] Rename enterprise "Relationships" to "Permissions" This only updates en* translations, and does not affect URLs and other back-end references. --- config/locales/en.yml | 4 ++-- spec/features/admin/enterprise_relationships_spec.rb | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/config/locales/en.yml b/config/locales/en.yml index 67c8d17fd3..7e653ea078 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -1911,7 +1911,7 @@ See the %{link} to find out more about %{sitename}'s features and to start using you_have_no_orders_yet: "You have no orders yet" running_balance: "Running balance" outstanding_balance: "Outstanding balance" - admin_entreprise_relationships: "Enterprise Relationships" + admin_entreprise_relationships: "Enterprise Permissions" admin_entreprise_relationships_everything: "Everything" admin_entreprise_relationships_permits: "permits" admin_entreprise_relationships_seach_placeholder: "Search" @@ -2036,7 +2036,7 @@ See the %{link} to find out more about %{sitename}'s features and to start using order_cycle: "Order Cycle" order_cycles: "Order Cycles" enterprises: "Enterprises" - enterprise_relationships: "Enterprise relationships" + enterprise_relationships: "Enterprise permissions" remove_tax: "Remove tax" enterprise_terms_of_service: "Enterprise Terms of Service" enterprises_require_tos: "Enterprises must accept Terms of Service" diff --git a/spec/features/admin/enterprise_relationships_spec.rb b/spec/features/admin/enterprise_relationships_spec.rb index 85a0a87ee2..c40b8e8f3b 100644 --- a/spec/features/admin/enterprise_relationships_spec.rb +++ b/spec/features/admin/enterprise_relationships_spec.rb @@ -20,7 +20,7 @@ feature %q{ # When I go to the relationships page click_link 'Enterprises' - click_link 'Relationships' + click_link 'Permissions' # Then I should see the relationships within('table#enterprise-relationships') do