From dfe25ed04e199770a5f9e9acb97e90a6003b352d Mon Sep 17 00:00:00 2001 From: andrea longhi Date: Fri, 5 Jun 2020 16:20:43 +0200 Subject: [PATCH 1/2] Specify user for new order link Given we're in an area specific for a given user, when we want to create a new order, it must be for that specific user. --- backend/app/views/spree/admin/users/items.html.erb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/backend/app/views/spree/admin/users/items.html.erb b/backend/app/views/spree/admin/users/items.html.erb index 4fdc1bd6527..41badbe7ddd 100644 --- a/backend/app/views/spree/admin/users/items.html.erb +++ b/backend/app/views/spree/admin/users/items.html.erb @@ -74,7 +74,7 @@
<%= render 'spree/admin/shared/no_objects_found', resource: Spree::Order, - new_resource_url: spree.new_admin_order_path %> + new_resource_url: spree.new_admin_order_path(user_id: @user.id) %>
<% end %> <%= paginate @orders, theme: "solidus_admin" %> From 48f0ed520c220e6fb5541e8b9a2e905ed4922ad9 Mon Sep 17 00:00:00 2001 From: Memo Toro Date: Fri, 5 Jun 2020 16:24:30 +0200 Subject: [PATCH 2/2] Adds cancancan validations (or more restrictive ones when already present) for the following resources in the backend: * Spree::Image * Spree::Order * Spree::Payment * Spree::ReturnAuthorization --- .../views/spree/admin/images/index.html.erb | 42 +++++------ .../views/spree/admin/orders/index.html.erb | 10 +-- .../views/spree/admin/payments/index.html.erb | 4 +- .../return_authorizations/index.html.erb | 2 +- .../views/spree/admin/users/items.html.erb | 8 ++- .../views/spree/admin/users/orders.html.erb | 8 ++- .../features/admin/orders/listing_spec.rb | 5 +- .../features/admin/orders/payments_spec.rb | 12 ++++ .../orders/return_authorizations_spec.rb | 71 ++++++++++++------- .../admin/products/edit/images_spec.rb | 19 ++++- backend/spec/features/admin/users_spec.rb | 15 ++++ 11 files changed, 134 insertions(+), 62 deletions(-) diff --git a/backend/app/views/spree/admin/images/index.html.erb b/backend/app/views/spree/admin/images/index.html.erb index 1e57a118c33..e8ec39f28ee 100644 --- a/backend/app/views/spree/admin/images/index.html.erb +++ b/backend/app/views/spree/admin/images/index.html.erb @@ -13,29 +13,31 @@ <%= render 'new', product: @product, image: Spree::Image.new(viewable: @product) %> -
- <%= t(".upload_images") %> +<% if can?(:create, Spree::Image) %> +
+ <%= t(".upload_images") %> -
- <%= form_for [:admin, @product, Spree::Image.new], - html: { multipart: true, id: 'upload-form' } do |f| %> - + <% end %> +
-
-
+
+
+<% end %> <% no_images = @product.gallery.images.empty? %> diff --git a/backend/app/views/spree/admin/orders/index.html.erb b/backend/app/views/spree/admin/orders/index.html.erb index 2b3063544a8..7df21a7abfd 100644 --- a/backend/app/views/spree/admin/orders/index.html.erb +++ b/backend/app/views/spree/admin/orders/index.html.erb @@ -7,7 +7,7 @@
  • <%= link_to t('spree.new_order'), new_admin_order_url, id: 'admin_new_order', class: 'btn btn-primary' %>
    • -<% end if can? :create, Spree::Order %> +<% end if can? :manage, Spree::Order %> <% content_for :table_filter_title do %> <%= t('spree.filter') %> @@ -197,9 +197,11 @@ <% else %>
    - <%= render 'spree/admin/shared/no_objects_found', - resource: Spree::Order, - new_resource_url: spree.new_admin_order_path %> + <% if can? :manage, Spree::Order %> + <%= render 'spree/admin/shared/no_objects_found', + resource: Spree::Order, + new_resource_url: spree.new_admin_order_path %> + <% end %>
    <% end %> diff --git a/backend/app/views/spree/admin/payments/index.html.erb b/backend/app/views/spree/admin/payments/index.html.erb index f7707b85db7..512450fd9b2 100644 --- a/backend/app/views/spree/admin/payments/index.html.erb +++ b/backend/app/views/spree/admin/payments/index.html.erb @@ -3,7 +3,9 @@ <% content_for :page_actions do %> <% if @order.outstanding_balance? %>
  • - <%= link_to t('spree.new_payment'), new_admin_order_payment_url(@order), class: 'btn btn-primary' %> + <% if can? :create, Spree::Payment %> + <%= link_to t('spree.new_payment'), new_admin_order_payment_url(@order), class: 'btn btn-primary' %> + <% end %>
    • <% end %> <% end %> diff --git a/backend/app/views/spree/admin/return_authorizations/index.html.erb b/backend/app/views/spree/admin/return_authorizations/index.html.erb index 0d0d4551e92..6384fccd113 100644 --- a/backend/app/views/spree/admin/return_authorizations/index.html.erb +++ b/backend/app/views/spree/admin/return_authorizations/index.html.erb @@ -3,7 +3,7 @@ <% content_for :page_actions do %> <% if @order.shipments.any? &:shipped? %>
  • - <% if can? :create, Spree::ReturnAuthorization %> + <% if can? :manage, Spree::ReturnAuthorization %> <%= link_to t('spree.new_return_authorization'), new_admin_order_return_authorization_url(@order), class: 'btn btn-primary' %> <% end %>
    • diff --git a/backend/app/views/spree/admin/users/items.html.erb b/backend/app/views/spree/admin/users/items.html.erb index 41badbe7ddd..0ae3456be6a 100644 --- a/backend/app/views/spree/admin/users/items.html.erb +++ b/backend/app/views/spree/admin/users/items.html.erb @@ -72,9 +72,11 @@ <% else %>
    - <%= render 'spree/admin/shared/no_objects_found', - resource: Spree::Order, - new_resource_url: spree.new_admin_order_path(user_id: @user.id) %> + <% if can? :manage, Spree::Order %> + <%= render 'spree/admin/shared/no_objects_found', + resource: Spree::Order, + new_resource_url: spree.new_admin_order_path(user_id: @user.id) %> + <% end %>
    <% end %> <%= paginate @orders, theme: "solidus_admin" %> diff --git a/backend/app/views/spree/admin/users/orders.html.erb b/backend/app/views/spree/admin/users/orders.html.erb index b4bbbe085de..29484ec6196 100644 --- a/backend/app/views/spree/admin/users/orders.html.erb +++ b/backend/app/views/spree/admin/users/orders.html.erb @@ -72,9 +72,11 @@ <% else %>
    - <%= render 'spree/admin/shared/no_objects_found', - resource: Spree::Order, - new_resource_url: spree.new_admin_order_path(user_id: @user.id) %> + <% if can? :manage, Spree::Order %> + <%= render 'spree/admin/shared/no_objects_found', + resource: Spree::Order, + new_resource_url: spree.new_admin_order_path(user_id: @user.id) %> + <% end %>
    <% end %> diff --git a/backend/spec/features/admin/orders/listing_spec.rb b/backend/spec/features/admin/orders/listing_spec.rb index aca08fe0423..7b208496d58 100644 --- a/backend/spec/features/admin/orders/listing_spec.rb +++ b/backend/spec/features/admin/orders/listing_spec.rb @@ -21,8 +21,9 @@ context 'without create permission' do custom_authorization! do |_user| - can :manage, Spree::Order - cannot :create, Spree::Order + cannot :manage, Spree::Order + can :admin, Spree::Order + can :display, Spree::Order end it 'does not display the new order button' do diff --git a/backend/spec/features/admin/orders/payments_spec.rb b/backend/spec/features/admin/orders/payments_spec.rb index dd86bb9870c..39dc4dd6f06 100644 --- a/backend/spec/features/admin/orders/payments_spec.rb +++ b/backend/spec/features/admin/orders/payments_spec.rb @@ -16,6 +16,18 @@ visit "/admin/orders/#{order.number}/payments" end + context "when the user cannot create payments" do + custom_authorization! do |_user| + cannot :create, Spree::Payment + end + + it "does not show the link for creating new payments" do + within "#content-header" do + expect(page).not_to have_content "New Payment" + end + end + end + # Regression tests for https://github.com/spree/spree/issues/1453 context 'with a check payment', js: true do let(:order) { create(:completed_order_with_totals, number: 'R100') } diff --git a/backend/spec/features/admin/orders/return_authorizations_spec.rb b/backend/spec/features/admin/orders/return_authorizations_spec.rb index ee5c0f39b2e..4441529522a 100644 --- a/backend/spec/features/admin/orders/return_authorizations_spec.rb +++ b/backend/spec/features/admin/orders/return_authorizations_spec.rb @@ -9,47 +9,66 @@ let!(:order) { create(:shipped_order) } - describe "create" do - def create_return_authorization - find("#select-all").click - select "NY Warehouse", from: "Stock Location" - click_button "Create" + context "when the user cannot manage return authorizations" do + custom_authorization! do |_user| + cannot :manage, Spree::ReturnAuthorization + can [:display, :admin], Spree::ReturnAuthorization end before do - visit spree.new_admin_order_return_authorization_path(order) + visit spree.admin_order_return_authorizations_path(order) end - it "creates a return authorization" do - create_return_authorization - - expect(page).to have_content "Return Authorization has been successfully created!" + it "does not show the link for creating new RMAs" do + within "#content-header" do + expect(page).not_to have_content "New RMA" + end end + end - it "disables the button at submit", :js do - page.execute_script "$('form').submit(function(e) { e.preventDefault()})" + context "when the user can manage return authorizations" do + describe "create" do + def create_return_authorization + find("#select-all").click + select "NY Warehouse", from: "Stock Location" + click_button "Create" + end - create_return_authorization + before do + visit spree.new_admin_order_return_authorization_path(order) + end - expect(page).to have_button("Create", disabled: true) - end - end + it "creates a return authorization" do + create_return_authorization - describe "when a return authorization exists" do - let!(:return_authorization) { create(:return_authorization, order: order) } + expect(page).to have_content "Return Authorization has been successfully created!" + end - it "can visit the return authorizations list page" do - visit spree.admin_order_return_authorizations_path(order) + it "disables the button at submit", :js do + page.execute_script "$('form').submit(function(e) { e.preventDefault()})" + + create_return_authorization + + expect(page).to have_button("Create", disabled: true) + end end - describe "edit" do - it "can visit the return authorizations edit page" do - visit spree.edit_admin_order_return_authorization_path(order, return_authorization) + describe "when a return authorization exists" do + let!(:return_authorization) { create(:return_authorization, order: order) } + + it "can visit the return authorizations list page" do + visit spree.admin_order_return_authorizations_path(order) end - it "return authorizations edit page has a data hook for extensions to add content above, below or within the RA form" do - visit spree.edit_admin_order_return_authorization_path(order, return_authorization) - expect(page).to have_selector("[data-hook=return-authorization-form-wrapper]") + describe "edit" do + it "can visit the return authorizations edit page" do + visit spree.edit_admin_order_return_authorization_path(order, return_authorization) + end + + it "return authorizations edit page has a data hook for extensions to add content above, below or within the RA form" do + visit spree.edit_admin_order_return_authorization_path(order, return_authorization) + expect(page).to have_selector("[data-hook=return-authorization-form-wrapper]") + end end end end diff --git a/backend/spec/features/admin/products/edit/images_spec.rb b/backend/spec/features/admin/products/edit/images_spec.rb index 0501c18e54f..6d482f082aa 100644 --- a/backend/spec/features/admin/products/edit/images_spec.rb +++ b/backend/spec/features/admin/products/edit/images_spec.rb @@ -16,15 +16,30 @@ end context "uploading, editing, and deleting an image", js: true do - it "should allow an admin to upload and edit an image for a product" do + before do Spree::Image.attachment_definitions[:attachment].delete :storage - create(:product) visit spree.admin_path click_nav "Products" click_icon(:edit) click_link "Images" + end + + context 'when the user cannot create images' do + custom_authorization! do |_user| + cannot :create, Spree::Image + end + + it "does not show links for creating images" do + within '#content-header' do + expect(page).not_to have_content 'New Image' + end + expect(page).not_to have_content 'Choose files to upload' + end + end + + it "should allow an admin to upload and edit an image for a product" do click_link "new_image_link" within_fieldset 'New Image' do attach_file('image_attachment', file_path) diff --git a/backend/spec/features/admin/users_spec.rb b/backend/spec/features/admin/users_spec.rb index 8fd1f088571..7418c9c1af0 100644 --- a/backend/spec/features/admin/users_spec.rb +++ b/backend/spec/features/admin/users_spec.rb @@ -22,6 +22,21 @@ let(:orders) { [order, order_2] } + describe 'the user items page' do + context 'when the user cannot manage orders' do + custom_authorization! do |_user| + cannot :manage, Spree::Order + can [:display, :admin], Spree::Order + end + + before { visit spree.items_admin_user_path(user_a) } + + it 'does not show the link for creating new orders' do + expect(page).not_to have_content 'No Orders found. Create One.' + end + end + end + shared_examples_for 'a user page' do it 'has lifetime stats' do orders