feature: Implement multiple option for select field

[zhephyn]

Description

Add a multiple boolean option to the select field, allowing for multiple selections.

Fixes #3426

Checklist:

• I have performed a self-review of my own code
• I have commented my code, particularly in hard-to-understand areas
• I have made corresponding changes to the documentation
• I have added tests that prove my fix is effective or that my feature works

Manual review steps

Added two extra tests(one for selecting multiple sizes when creating a new product, another for editing the sizes list of an existing product) to the select_field_spec.rb file. These all pass when i run the command "bin/test ./spec/feature/avo/select_field_spec.rb".

[codeclimate]

Code Climate has analyzed commit 557e303 and detected 0 issues on this pull request.

View more on Code Climate.

[Paul-Bob]

Hi @zhephyn

Thanks for this contribution. It's going on the right direction!

I left some comments on the code.

I also noticed that when multiple is true the show and index views are not showing the value as expected.

Let's address the code comments and the displaying issue before the next review.

Let me know if I can help.

Thank you!

[zhephyn]

Hey @Paul-Bob . I've updated the PR to reflect the changes you requested for, as highlighted below:

• Removed the include_hidden method which caused problems on deselection. Instead, i resorted to using a before_validation method in the product model file to remove the empty string from the sizes array, which had been the purpose of using include_hidden in the first place. To ensure that this works as expected, i added a test which caters to deselection, which passes signifying that on deselecting, we get an empty array([]) as the result.
• Also i edited the existing tests and the new test to use "product_sizes" instead of "product[sizes][]"
• Made edits related to the to_permitted_param and maintained the original syntax of "aria" in the edit component file.
  Kindly review it when you get some time and get back to me about what you think.

[Paul-Bob]

Hey @zhephyn, thanks for the updates!

There's a bit of an issue with the sanitization. It really should be handled automatically by Avo rather than shifting that responsibility to the user.

Other than that, we're pretty much at the finish line! 🚀

[zhephyn]

Hey @zhephyn, thanks for the updates!

There's a bit of an issue with the sanitization. It really should be handled automatically by Avo rather than shifting that responsibility to the user.

Other than that, we're pretty much at the finish line! 🚀

I've updated the PR to reflect the changes you requested for. In particular, the ones relating to sanitization.

[Paul-Bob]

I've updated the PR to reflect the changes you requested for. In particular, the ones relating to sanitization.

Awesome! Let's please ensure that lint and the test actions are not failing, you can check the details for each failing action, let me know if you need any help interpreting those.

[zhephyn]

Awesome! Let's please ensure that lint and the test actions are not failing, you can check the details for each failing action, let me know if you need any help interpreting those.

The current 3 tests I had written to test for the new and edit actions relating to the field all pass as expected. The last 3 tests in the select_field_spec are the ones I'm referring to. Are there any other tests that test for this field that I should ensure "work fine"?

[zhephyn]

Hey @Paul-Bob, I've updated the PR to fix the issues that were flagged by rubocop relating to double and single quote syntax

[zhephyn]

Hey @Paul-Bob, the issues I'm seeing on checking the PR, the one for rubocop seems understandable and im gonna fix it. As for the system and feature specs, the runner is failing at bundle install and throws an argument error saying that version 8.0 is not recognized with the highest version being 7.2. This doesn't quite make sense given the fact that, locally everything works and this is a rails 8 application. The tests are being run against rails version 7.2 yet the application's migration version is 8.0.

[zhephyn]

Hey @Paul-Bob. I've updated the branch to solve the migration version error plus the double string rubocop error.
PS: Sorry about the delay

[zhephyn]

I thought the migration version had to correspond to the rails version installed by the bundle command when running the CI test. That's why I had specified version 7.1, I'm going to update the PR and use rails version 6.1 instead. Can't wait for the PR to get merged 🤞.
PS: Thank you so much for your patience. 🙏

[Paul-Bob]

Hey @Paul-Bob. I've updated the branch to solve the migration version error plus the double string rubocop error.
PS: Sorry about the delay

Thanks @zhephyn, no problem!

I thought the migration version had to correspond to the rails version installed by the bundle command when running the CI test. That's why I had specified version 7.1, I'm going to update the PR and use rails version 6.1 instead. Can't wait for the PR to get merged 🤞.
PS: Thank you so much for your patience. 🙏

The version must be 6.1 or lower since that's the minimum version used for testing.

I've observed some test failures, could you please investigate those?

[zhephyn]

The tests are failing because rails possibly has no inbuilt provision for allowing multiple values for an enum declaration in a model. Rails expects that an enum should allow single and not multiple selection.
In the project model file, we define an enum called "Stage" and provide the options including "On hold, cancelled, drafting" and so on. Then in the schema.rb, we can see that the stage column of the projects table, has array set to "true", which is meant to say that it can accept an array/multiple values instead of a single value. However, in factories.rb, for tests that rely on creating a project first before the test is run, the stage field is populated with a single value instead of an array, which is why we see the 'Malformed array literal" error. To solve this we could EITHER,

• Do away with the enum altogether in the project model file and specify the options for the stage field in the project.rb resource file like we did for the "Sizes" column of the product table, similar to this:
  field :type, as: :select, options: { 'Large container': :large, 'Medium container': :medium, 'Tiny container': :tiny }, display_with_value: true, placeholder: 'Choose the type of the container.'
• OR we could manipulate the values sent after populating the "Stage" field in the project.rb model file, before saving which i think could allow us to keep the enum declaration.
• Another option would be to run a migration to change the Stage column to a non-array column. Though if my memory serves me right, initially the issue was related to making this particular field a multiple select.

Kindly let me know which approach is you'd prefer and i'll get to solving the error ASAP!

[Paul-Bob]

@zhephyn let's start by fixing the schema, the AddSizesToProducts migration is adding the sizes array column to the products table however the schema is also changing the stage column to array on the projects table.

Suggested change

	t.string "stage", default: [], array: true
	t.string "stage"

[Paul-Bob]

I'm directly committing this change to speed up the process

[zhephyn]

Okay no problem. Im not directly in contact with my computer at the moment

[zhephyn]

@Paul-Bob . Though I didn't quite understand what you referenced, you specified a migration named AddSizesToProducts, yet according to the suggested change, it's like you're changing the stage column to a non array column, and the stage column is part of the projects table not the products table. Could you kindly clarify if this is the case?

[Paul-Bob]

initially the issue was related to making this particular field a multiple select.

The spec/dummy directory contains the local application used for development and testing. The issue is about implementing the multiple option for select fields. All changes made to spec/dummy should be exclusively for testing this feature.

I mentioned that specific field in the video to illustrate how multiple select should work. However, it’s not necessary to modify that exact field or data structure. Since that field is already being used in other tests, it’s preferable to create a new field specifically for testing the multiple select use case, similar to how you created the sizes field for products.

Let me know if you have any questions.

[zhephyn]

Thanks for the feedback. As for what I meant by the statement, initially what we were aiming for is that for every resource that has a multiple select, whether it's Product or Project, any field of this resource can use the select option which can allow multiple options to be selected. And so far, we implemented it correctly for the sizes column of the products table. All the tests related to this pass. However, the problem came with the fact that the stage column of the projects table is set to accept array values, yet the Enumerable declaration for the stage in the project model doesn't allow this. This is a missing feature when working with Postgres backed columns in rails. The enum can't allow multiple values but rather a single value. Which is why those tests failed. And this is why I had suggested as one of the approaches that we could remove the "array: true" option from the stage column by running a migration, this will solve the issue. However, this would defeat the initial suggestion since you had said that we should make changes such that the we can select multiple stages whether two or more for a given project. By removing the "array: true", this wouldn't be possible. Should I create a demo with code and perhaps video to show what I'm referring to approach wise? Please let me know. Thanks for the feedback. I'm very grateful for the learning experience 🙏

[Paul-Bob]

@zhephyn, can we hop on a quick call to talk this through?

[zhephyn]

@zhephyn, can we hop on a quick call to talk this through?

Sure. Can we use zoom? Or there's another alternative?