Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[BUU2] Hide producer column when there's only one producer in the admin account #12854

Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 5 additions & 2 deletions app/helpers/admin/products_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,11 @@ def product_image_form_path(product)
end
end

def prepare_new_variant(product)
product.variants.build
def prepare_new_variant(product, producer_options)
# e.g producer_options = [['producer name', id]]
product.variants.build do |new_variant|
new_variant.supplier_id = producer_options.first.second if producer_options.one?
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If only one producer exists, the producer column will be hidden according to the issue. In that case, the producer should be auto-assigned so that UI doesn't show an error on saving the variant.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be nicer to have producer_options as an array of hashes: [ { name: 'producer name', id: id } ], anyway its fine to keep it like this.

end
end

def unit_value_with_description(variant)
Expand Down
2 changes: 1 addition & 1 deletion app/models/column_preference.rb
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ def self.for(user, action_name)
end

def self.valid_columns_for(action_name)
__send__("#{action_name}_columns").keys.map(&:to_s)
get_default_preferences(action_name, Spree::User.new).keys.map(&:to_s)
end

def self.known_actions
Expand Down
2 changes: 1 addition & 1 deletion app/views/admin/products_v3/_product_variant_row.html.haml
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
%tr.condensed{ id: dom_id(variant), 'data-controller': "variant", 'class': "nested-form-wrapper", 'data-new-record': variant.new_record? ? "true" : false }
= render partial: 'variant_row', locals: { variant:, f: variant_form, category_options:, tax_category_options:, producer_options: }

= form.fields_for("products][#{product_index}][variants_attributes][NEW_RECORD", prepare_new_variant(product)) do |new_variant_form|
= form.fields_for("products][#{product_index}][variants_attributes][NEW_RECORD", prepare_new_variant(product, producer_options)) do |new_variant_form|
%template{ 'data-nested-form-target': "template" }
%tr.condensed{ 'data-controller': "variant", 'class': "nested-form-wrapper", 'data-new-record': "true" }
= render partial: 'variant_row', locals: { variant: new_variant_form.object, f: new_variant_form, category_options:, tax_category_options:, producer_options: }
Expand Down
77 changes: 47 additions & 30 deletions spec/system/admin/products_v3/actions_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -34,42 +34,59 @@
describe "column selector" do
let!(:product) { create(:simple_product) }

before do
visit admin_products_url
end
context "with one producer only" do
before do
visit admin_products_url
end

it "hides column and remembers saved preference" do
# Name shows by default
expect(page).to have_checked_field "Name"
expect(page).to have_selector "th", text: "Name"
expect_other_columns_visible

it "hides column and remembers saved preference" do
# Name shows by default
expect(page).to have_checked_field "Name"
expect(page).to have_selector "th", text: "Name"
expect_other_columns_visible
# Producer is hidden by if only one producer is present
expect(page).not_to have_checked_field "Producer"
expect(page).not_to have_selector "th", text: "Producer"

# Name is hidden
ofn_drop_down("Columns").click
within ofn_drop_down("Columns") do
uncheck "Name"
# Name is hidden
ofn_drop_down("Columns").click
within ofn_drop_down("Columns") do
uncheck "Name"
end
expect(page).not_to have_selector "th", text: "Name"
expect_other_columns_visible

# Preference saved
click_on "Save as default"
expect(page).to have_content "Column preferences saved"
refresh

# Preference remembered
ofn_drop_down("Columns").click
within ofn_drop_down("Columns") do
expect(page).to have_unchecked_field "Name"
end
expect(page).not_to have_selector "th", text: "Name"
expect_other_columns_visible
rioug marked this conversation as resolved.
Show resolved Hide resolved
end
expect(page).not_to have_selector "th", text: "Name"
expect_other_columns_visible

# Preference saved
click_on "Save as default"
expect(page).to have_content "Column preferences saved"
refresh

# Preference remembered
ofn_drop_down("Columns").click
within ofn_drop_down("Columns") do
expect(page).to have_unchecked_field "Name"

def expect_other_columns_visible
expect(page).to have_selector "th", text: "Price"
expect(page).to have_selector "th", text: "On Hand"
end
expect(page).not_to have_selector "th", text: "Name"
expect_other_columns_visible
end

def expect_other_columns_visible
expect(page).to have_selector "th", text: "Producer"
expect(page).to have_selector "th", text: "Price"
expect(page).to have_selector "th", text: "On Hand"
context "with multiple producers" do
let!(:producer2) { create(:supplier_enterprise, owner: user) }

before { visit admin_products_url }

it "has selected producer column by default" do
# Producer shows by default
expect(page).to have_checked_field "Producer"
expect(page).to have_selector "th", text: "Producer"
end
end
end

Expand Down
1 change: 1 addition & 0 deletions spec/system/admin/products_v3/create_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
include WebHelper

let!(:supplier) { create(:supplier_enterprise) }
let!(:supplier2) { create(:supplier_enterprise) }
rioug marked this conversation as resolved.
Show resolved Hide resolved
let!(:taxon) { create(:taxon) }

describe "creating a new product" do
Expand Down
1 change: 0 additions & 1 deletion spec/system/admin/products_v3/index_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,6 @@
expect(page).to have_selector "th", text: "Unit"
expect(page).to have_selector "th", text: "Price"
expect(page).to have_selector "th", text: "On Hand"
expect(page).to have_selector "th", text: "Producer"
expect(page).to have_selector "th", text: "Category"
expect(page).to have_selector "th", text: "Tax Category"
expect(page).to have_selector "th", text: "Inherits Properties?"
Expand Down
3 changes: 2 additions & 1 deletion spec/system/admin/products_v3/update_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,8 @@
include FileHelper

let(:producer) { create(:supplier_enterprise) }
let(:user) { create(:user, enterprises: [producer]) }
let(:producer2) { create(:supplier_enterprise) }
let(:user) { create(:user, enterprises: [producer, producer2]) }

before do
login_as user
Expand Down
Loading