raggedstaff/openfoodnetwork
1
0
Fork 0
mirror of https://github.com/openfoodfoundation/openfoodnetwork synced 2026-02-05 22:26:07 +00:00
Code Issues Packages Projects Releases Wiki Activity

Merge pull request #11565 from dacook/buu-editing-part4-11059

Browse Source 
[BUU] Inline error messages and validation
This commit is contained in:
Rachel Arnould
2023-10-06 14:23:58 +02:00
committed by GitHub
parent 9e04dc36d1 117085aeba
commit abfd316b3d
11 changed files with 110 additions and 46 deletions
Download Patch File Download Diff File Expand all files Collapse all files

1
Gemfile
View File

@@ -32,6 +32,7 @@ gem "db2fog", github: "openfoodfoundation/db2fog", branch: "rails-7"
gem "fog-aws", "~> 2.0" # db2fog does not support v3
gem "mime-types" # required by fog
gem "validates_lengths_from_database"
gem "valid_email2"
gem "catalog", path: "./engines/catalog"

3
Gemfile.lock
View File

@@ -736,6 +736,8 @@ GEM
validate_url (1.0.15)
activemodel (>= 3.0.0)
public_suffix
validates_lengths_from_database (0.8.0)
activerecord (>= 4)
vcr (6.2.0)
view_component (3.6.0)
activesupport (>= 5.2.0, < 8.0)
@@ -909,6 +911,7 @@ DEPENDENCIES
stripe
timecop
valid_email2
validates_lengths_from_database
vcr
view_component
view_component_reflex (= 3.1.14.pre9)

3
app/helpers/spree/admin/base_helper.rb
View File

@@ -6,9 +6,6 @@ module Spree
def field_container(model, method, options = {}, &)
css_classes = options[:class].to_a
css_classes << 'field'
if error_message_on(model, method).present?
css_classes << 'withError'
end
content_tag(:div,
capture(&),
class: css_classes.join(' '),

1
app/models/spree/product.rb
View File

@@ -54,6 +54,7 @@ module Spree
has_many :variant_images, -> { order(:position) }, source: :images,
through: :variants
validates_lengths_from_database
validates :name, presence: true
validates :variant_unit, presence: true

3
app/models/spree/variant.rb
View File

@@ -61,6 +61,7 @@ module Spree
localize_number :price, :weight
validates_lengths_from_database
validate :check_currency
validates :price, numericality: { greater_than_or_equal_to: 0 }, presence: true
validates :tax_category, presence: true,
@@ -80,8 +81,8 @@ module Spree
before_validation :ensure_shipping_category
before_validation :ensure_unit_value
before_validation :update_weight_from_unit_value, if: ->(v) { v.product.present? }
before_validation :convert_variant_weight_to_decimal
before_save :convert_variant_weight_to_decimal
before_save :assign_units, if: ->(variant) {
variant.new_record? || variant.changed_attributes.keys.intersection(NAME_FIELDS).any?
}

33
app/views/admin/products_v3/_table.html.haml
View File

@@ -38,13 +38,15 @@
= form.fields_for("products", product, index: nil) do |product_form|
%tbody.relaxed{ 'data-record-id': product_form.object.id }
%tr
%td.align-left.header
%td.field.align-left.header
= product_form.hidden_field :id
= product_form.text_field :name, 'aria-label': t('admin.products_page.columns.name')
%td.align-right
= error_message_on product, :name
%td.field
= product_form.text_field :sku, 'aria-label': t('admin.products_page.columns.sku')
= error_message_on product, :sku
%td.align-right
.line-clamp-1
.content
= product.variant_unit.upcase_first
/ TODO: properly handle custom unit names
= WeightsAndMeasures::UNITS[product.variant_unit] && "(" + WeightsAndMeasures::UNITS[product.variant_unit][product.variant_unit_scale]["name"] + ")"
@@ -52,36 +54,39 @@
-# empty
%td.align-right
-# TODO: new requirement "DISPLAY ON DEMAND IF ALL VARIANTS ARE ON DEMAND". And translate value
.line-clamp-1= if product.variants.all?(&:on_demand) then "On demand" else product.on_hand || 0 end
.content= if product.variants.all?(&:on_demand) then "On demand" else product.on_hand || 0 end
%td.align-left
.line-clamp-1= product.supplier.name
.content= product.supplier.name
%td.align-left
.line-clamp-1= product.primary_taxon.name
.content= product.primary_taxon.name
%td.align-left
%td.align-left
.line-clamp-1= product.inherits_properties ? 'YES' : 'NO' #TODO: consider using https://github.com/RST-J/human_attribute_values, else use I18n.t (also below)
.content= product.inherits_properties ? 'YES' : 'NO' #TODO: consider using https://github.com/RST-J/human_attribute_values, else use I18n.t (also below)
%td.align-right
= render partial: 'admin/products_v3/components/product_actions', locals: { product: product }
- product.variants.each do |variant|
= form.fields_for("products][][variants_attributes][", variant, index: nil) do |variant_form|
%tr.condensed
%td.align-left
%td.field
= variant_form.hidden_field :id
= variant_form.text_field :display_name, 'aria-label': t('admin.products_page.columns.name'), placeholder: product.name
%td.align-right
= error_message_on variant, :display_name
%td.field
= variant_form.text_field :sku, 'aria-label': t('admin.products_page.columns.sku')
= error_message_on variant, :sku
%td.align-right
.line-clamp-1= variant.unit_to_display
%td.align-right
.content= variant.unit_to_display
%td.field
= variant_form.text_field :price, 'aria-label': t('admin.products_page.columns.price'), value: number_to_currency(variant.price, unit: '')&.strip # TODO: add a spec to prove that this formatting is necessary. If so, it should be in a shared form helper for currency inputs
= error_message_on variant, :price
%td.align-right
.line-clamp-1= variant.on_hand || 0 #TODO: spec for this according to requirements.
.content= variant.on_hand || 0 #TODO: spec for this according to requirements.
%td.align-left
.line-clamp-1= variant.product.supplier.name # same as product
.content= variant.product.supplier.name # same as product
%td.align-left
-# empty
%td.align-left
.line-clamp-1= variant.tax_category&.name || "None" # TODO: convert to dropdown, else translate hardcoded string.
.content= variant.tax_category&.name || "None" # TODO: convert to dropdown, else translate hardcoded string.
%td.align-left
-# empty
%td.align-right

8
app/webpacker/css/admin/products_v3.scss
View File

@@ -62,8 +62,16 @@
th,
td {
vertical-align: top;
padding: $padding-tbl-cell;
border: none;
.content {
// Plain content fields need help to align with text in inputs (due to vertical-align)
margin: $vpadding-txt+1px 0;
@extend .line-clamp-1;
}
}
th {

26
app/webpacker/css/admin/shared/forms.scss
View File

@@ -97,21 +97,21 @@ span.info {
}
}
&.withError {
.field_with_errors {
label {
color: $color-error;
}
input {
border-color: $color-error;
}
}
.formError {
// Errors described by default form builder
.field_with_errors {
label {
color: $color-error;
font-style: italic;
font-size: 85%;
}
input {
border-color: $color-error;
}
}
// Errors described by Spree::Admin::BaseHelper
.formError {
color: $color-error;
font-style: italic;
font-size: 85%;
}
}

37
app/webpacker/css/admin_v3/shared/forms.scss
View File

@@ -108,20 +108,31 @@ span.info {
}
}
&.withError {
.field_with_errors {
label {
color: $color-error;
}
input {
border-color: $color-error;
}
}
.formError {
// Errors
.field_with_errors {
label {
color: $color-error;
font-style: italic;
font-size: 85%;
}
input {
border-color: $color-error;
}
}
// Inline error message
.formError {
display: inline-block;
color: $color-error;
font-size: 85%;
// Icon on left, with subsequent lines indented
text-indent: -0.6em;
margin-left: 1.2em;
@extend .icon-remove-sign;
@extend [class^="icon-"];
&:before {
margin-right: 0.3em;
}
}
}

4
spec/models/spree/product_spec.rb
View File

@@ -163,6 +163,10 @@ module Spree
expect(build(:product)).to be_valid
end
it { is_expected.to validate_presence_of :name }
it { is_expected.to validate_length_of(:name).is_at_most(255) }
it { is_expected.to validate_length_of(:sku).is_at_most(255) }
it "requires a primary taxon" do
expect(build(:simple_product, primary_taxon: nil)).not_to be_valid
end

37
spec/system/admin/products_v3/products_spec.rb
View File

@@ -201,7 +201,7 @@ describe 'As an admin, I can see the new product page' do
visit admin_products_v3_index_url
end
it "can update product and variant fields" do
it "updates product and variant fields" do
within row_containing_name("Apples") do
fill_in "Name", with: "Pommes"
fill_in "SKU", with: "POM-00"
@@ -236,7 +236,7 @@ describe 'As an admin, I can see the new product page' do
expect(page).to have_content "Changes saved"
end
it "can discard changes and reload latest data" do
it "discards changes and reloads latest data" do
within row_containing_name("Apples") do
fill_in "Name", with: "Pommes"
end
@@ -262,6 +262,39 @@ describe 'As an admin, I can see the new product page' do
expect(page).to have_field "SKU", with: "APL-10" # Updated value shown
end
end
it "shows errors for both product and variant fields" do
within row_containing_name("Apples") do
fill_in "Name", with: ""
fill_in "SKU", with: "A" * 256
end
within row_containing_name("Medium box") do
fill_in "Name", with: "L" * 256
fill_in "SKU", with: "1" * 256
end
expect {
click_button "Save changes"
product_a.reload
}.to_not change { product_a.name }
# (there's no identifier displayed, so the user must remember which product it is..)
within row_containing_name("") do
expect(page).to have_field "Name", with: ""
expect(page).to have_content "can't be blank"
expect(page).to have_field "SKU", with: "A" * 256
expect(page).to have_content "is too long"
end
pending
within row_containing_name("L" * 256) do
expect(page).to have_field "Name", with: "L" * 256
expect(page).to have_field "SKU", with: "1" * 256
expect(page).to have_content "is too long"
expect(page).to have_field "Price", with: "10.25"
end
expect(page).to have_content "Please review the errors and try again"
end
end
def expect_page_to_be(page_number)