Removing 'none' from shipping categories from product edit page

[vviekk]

• Closes [Edit Product] User can select shipping category 'none' even though this doesn't get saved and shouldn't be used #10515

Simply removing the 'none' option from shipping categories drop down from product edit page as it cannot be saved set to none.

What should we test?

• Go to products under admin section
• Go to edit page of any product and see that there is no none as one of the options under Shipping Categories drop down.

Release notes

Changelog Category: User facing changes | Technical changes

[mkllnk]

Nice! So simple. I think in this case it's okay to not have a spec because we don't need to test absent code. A spec for the present select options would have been nice though. Maybe when we replace select2 then we can also write better specs.

[jibees]

Thanks a lot! 🙏

[drummer83]

Hi @vviekk,
Thanks for yet another PR! 💪

Before the PR

I have verified that shipping category 'none' can be selected when editing a product

• as super admin
• as enterprise user
  

I have also verified that 'none' cannot be selected when creating new products.

After the PR

Peek.2023-03-19.20-25.mp4

I can confirm that

• shipping category 'none' is not available anymore for
  • super admin user,
  • enterprise user,
• the dropdown work well (selecting and saving),
• nothing has changed when creating new products.

BUT: The design of the dropdown has changed. It's probably the new design we will want to use at some point, but I am not so sure if we want to have this one dropdown looking different than all the others.

Requesting feedback from @openfoodfoundation/train-drivers-product-owners.

Result

I am adding the feedback-needed label to see what people think about the design.

Thanks again!

[RachL]

FYI the work on BUU won't change anything to the product edit page apart from colors, we only focus on the product list for now.

@jibees do you think the color change will harmonize the dropdowns?

[jibees]

@RachL

To me the best solution is to: harmonize it on this PR. It will be easier when changing the color, later on.

[RachL]

@vviekk would you still be available to make the change? Ie keep the previous color style for this dropdown?

[vviekk]

@RachL will do.

[vviekk]

@jibees ,
Quick clarification: We want to have a common colour config for all the drop downs on this particular page for uniformity, correct?

[jibees]

I think the design issue is pretty straightforward with my following suggestion.

[jibees]

You removed the third parameters (previously was { :include_blank => t(:none) }), so the 4th parameter is considered as the third.
Anyway, you probably should remplace { :include_blank => t(:none) } by { }, which is the following suggestion:

Suggested change

	= f.collection_select(:shipping_category_id, @shipping_categories, :id, :name, { :class => 'select2' })
	= f.collection_select(:shipping_category_id, @shipping_categories, :id, :name, {} , { :class => 'select2' })

[vviekk]

Awesome. Thank you!

[vviekk]

Do you think there should be a spec for this?
Can there be, with reasonable amount of work?

[jibees]

I don't think this small modification deserves a spec. I might be wrong, maybe let's @filipefurtad0 answer this.

By the way, I think this PR needs commits to be squashed to have a clean git history.

[filipefurtad0]

I don't think this small modification deserves a spec.

Agree @jibees @vviekk 👍

[mkllnk]

I agree, please rebase. You can use pick and fixup to combine your commits into one.

[jibees]

Thanks!

[drummer83]

Hi @vviekk,
Thanks for following up and pushing this over the line! 💪

I can now confirm, that we have the previous dropdown design back, 'none' is not shown anymore and functionality is still given as tested in my previous notes.

Great work! Merging! 🥂 🎆