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

[Admin] - 2nd Option Type is lost, can't add to existing product #5751

Open
adomokos opened this issue May 8, 2024 · 5 comments
Open

[Admin] - 2nd Option Type is lost, can't add to existing product #5751

adomokos opened this issue May 8, 2024 · 5 comments

Comments

@adomokos
Copy link

adomokos commented May 8, 2024

Solidus Version:
4.3.4

To Reproduce
Create a product in the new Solidus Admin, assigning two Option Types works. Don't change anything on the Product form, click update and the 2nd Option Type is removed. When I try to add it back, it's not persisted.

  1. Go to Solidus Admin
  2. Add a Product with two Option Types
  3. Edit the product
  4. Hit save
  5. 2nd Option Type is removed (lost)

Current behavior
The 2nd Option Type is removed (lost) when I edit the product.

Expected behavior
The 2nd Option Type should be kept when I edit the product. I should also be able to add the 2nd Option Type.

Screenshots

solidus-admin-option-type-not-saved.mov

Desktop (please complete the following information):

  • OS: Mac OS Sonoma 14.4.1
  • Browser Chrome
  • Version 124.0.6367.119 (Official Build) (arm64)

Additional context
Legacy admin works as a workaround.

Rails logs:

Started PATCH "/admin/products/t-shirt-bsmg-tshirt" for ::1 at 2024-05-08 10:11:41 -0500
Processing by SolidusAdmin::ProductsController#update as HTML
  Parameters: {"authenticity_token"=>"[FILTERED]", "product"=>{"name"=>"t-shirt", "slug"=>"t-shirt-bsmg-tshirt", "description"=>"Amazing 100% cotton t-shirts.", "store_ids"=>"1", "taxon_ids"=>"", "option_type_ids"=>"1,2", "meta_tit
le"=>"", "meta_keywords"=>"[FILTERED]", "meta_description"=>"", "price"=>"0.00", "cost_price"=>"", "cost_currency"=>"USD", "available_on"=>"2024/03/01", "discontinue_on"=>"", "promotionable"=>"1", "sku"=>"BSMG-TSHIRT", "shipping_ca
tegory_id"=>"1", "tax_category_id"=>""}, "button"=>"", "id"=>"t-shirt-bsmg-tshirt"}
  Spree::User Load (0.8ms)  SELECT "spree_users".* FROM "spree_users" WHERE "spree_users"."deleted_at" IS NULL AND "spree_users"."id" = $1 ORDER BY "spree_users"."id" ASC LIMIT $2  [["id", 1], ["LIMIT", 1]]
  ↳ config/initializers/spree_authentication.rb:7:in `spree_current_user'
  Spree::Role Load (0.6ms)  SELECT "spree_roles".* FROM "spree_roles" INNER JOIN "spree_roles_users" ON "spree_roles"."id" = "spree_roles_users"."role_id" WHERE "spree_roles_users"."user_id" = $1  [["user_id", 1]]
  Spree::Product Load (1.9ms)  SELECT "spree_products".* FROM "spree_products" WHERE "spree_products"."deleted_at" IS NULL AND "spree_products"."slug" = $1 LIMIT $2  [["slug", "t-shirt-bsmg-tshirt"], ["LIMIT", 1]]
  TRANSACTION (1.1ms)  BEGIN
  Spree::Store Load (0.9ms)  SELECT "spree_stores".* FROM "spree_stores" WHERE "spree_stores"."id" = $1  [["id", 1]]
  Spree::Store Load (1.0ms)  SELECT "spree_stores".* FROM "spree_stores" INNER JOIN "bsmg_product_stores" ON "spree_stores"."id" = "bsmg_product_stores"."store_id" WHERE "bsmg_product_stores"."product_id" = $1  [["product_id", 31]]
  Spree::Taxon Load (3.3ms)  SELECT "spree_taxons".* FROM "spree_taxons" INNER JOIN "spree_products_taxons" ON "spree_taxons"."id" = "spree_products_taxons"."taxon_id" WHERE "spree_products_taxons"."product_id" = $1  [["product_id"
, 31]]
  Spree::OptionType Load (1.0ms)  SELECT "spree_option_types".* FROM "spree_option_types" WHERE "spree_option_types"."id" = $1 ORDER BY "spree_option_types"."position" ASC  [["id", 1]]
  Spree::OptionType Load (0.9ms)  SELECT "spree_option_types".* FROM "spree_option_types" INNER JOIN "spree_product_option_types" ON "spree_option_types"."id" = "spree_product_option_types"."option_type_id" WHERE "spree_product_opt
ion_types"."product_id" = $1 ORDER BY "spree_option_types"."position" ASC  [["product_id", 31]]
  Spree::ProductOptionType Delete All (1.5ms)  DELETE FROM "spree_product_option_types" WHERE "spree_product_option_types"."product_id" = $1 AND "spree_product_option_types"."option_type_id" = $2  [["product_id", 31], ["option_type
_id", 2]]
  Spree::Variant Load (1.1ms)  SELECT "spree_variants".* FROM "spree_variants" WHERE "spree_variants"."product_id" = $1 AND "spree_variants"."is_master" = $2 LIMIT $3  [["product_id", 31], ["is_master", true], ["LIMIT", 1]]
  Spree::Price Load (1.6ms)  SELECT "spree_prices".* FROM "spree_prices" WHERE "spree_prices"."variant_id" = $1  [["variant_id", 164]]
  Spree::Variant Exists? (1.8ms)  SELECT 1 AS one FROM "spree_variants" WHERE "spree_variants"."sku" = $1 AND "spree_variants"."id" != $2 AND "spree_variants"."deleted_at" IS NULL LIMIT $3  [["sku", "BSMG-TSHIRT"], ["id", 164], ["L
IMIT", 1]]
  Spree::Product Exists? (0.5ms)  SELECT 1 AS one FROM "spree_products" WHERE "spree_products"."slug" = $1 AND "spree_products"."id" != $2 LIMIT $3  [["slug", "t-shirt-bsmg-tshirt"], ["id", 31], ["LIMIT", 1]]
  FriendlyId::Slug Load (2.2ms)  SELECT "friendly_id_slugs".* FROM "friendly_id_slugs" WHERE "friendly_id_slugs"."sluggable_id" = $1 AND "friendly_id_slugs"."sluggable_type" = $2 ORDER BY "friendly_id_slugs"."id" DESC LIMIT $3  [["
sluggable_id", 31], ["sluggable_type", "Spree::Product"], ["LIMIT", 1]]
  TRANSACTION (3.5ms)  COMMIT
Redirected to http://localhost:3100/admin/products/t-shirt-bsmg-tshirt
Completed 303 See Other in 64ms (ActiveRecord: 27.8ms | Allocations: 28599)
@thomas-profitt
Copy link

thomas-profitt commented May 10, 2024

This same problem happens in our freshly-built solidus 4.3.4 & rails 7.1.3.2 project when we try to add multiple taxons.

Legacy admin works as a workaround.

Thank you!

nvandoorn added a commit to nvandoorn/solidus that referenced this issue Aug 8, 2024
As reported in solidusio#5751, the new admin UI has a bug with the option type
selectors: only one option type can be added.

We narrowed this down to an issue handling option type params in the new
admin products controller. We noticed legacy controller contains a
before action to handle the incoming option type params, but the new one
did not. Re-adding the before action solves the bug. This is required
because the form input for option types contains comma separated values
within a string:

`<input value="1,2"type="hidden">`

Co-authored-by: An Stewart <[email protected]>
nvandoorn added a commit to nvandoorn/solidus that referenced this issue Aug 8, 2024
As reported in solidusio#5751, the new admin UI has a bug with the option type
selectors: only one option type can be added.

We narrowed this down to an issue handling option type params in the new
admin products controller. We noticed legacy controller contains a
before action to handle the incoming option type params, but the new one
did not. Re-adding the before action solves the bug. This is required
because the form input for option types contains comma separated values
within a string:

`<input value="1,2" type="hidden">`

Co-authored-by: An Stewart <[email protected]>
@nvandoorn
Copy link
Contributor

Hey folks, I believe this issue is fixed in #5816. I'm hoping it will get merged relatively soon so folks can test it easier, but feel free to pull my branch to test.

nvandoorn added a commit to nvandoorn/solidus that referenced this issue Aug 8, 2024
As reported in solidusio#5751, the new admin UI has a bug with the option type
selectors: only one option type can be added.

We narrowed this down to an issue handling option type params in the new
admin products controller. We noticed legacy controller contains a
before action to handle the incoming option type params, but the new one
did not. Re-adding the before action solves the bug. This is required
because the form input for option types contains comma separated values
within a string:

`<input value="1,2" type="hidden">`

Co-authored-by: An Stewart <[email protected]>
Co-authored-by: Kendra Chateau <[email protected]>
@nvandoorn
Copy link
Contributor

Hey @adomokos, hope you are well. This change got merged into main. Are you able to test and confirm if this fixes the bug?

@nvandoorn
Copy link
Contributor

Hey @adomokos following up here again. I believe this issue has been fixed in #5816. Looking to close this issue if possible.

Thanks!

@adomokos
Copy link
Author

Sure thing. It's kinda hard for me to test it, so I am going to 👍 for you.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants