Page publishing makes Ingredient::Boolean store String type

[robinboening]

Steps to reproduce

1. Define an element with Boolean ingredient
2. Render el.element.ingredient_by_type(:boolean)[:value] in the element view
3. Place the Element on a Page
4. Publish the Page
5. Visit the Page outside Alchemy

Expected behavior

The view should render true or false, based on the previous user input (or nil if it was just created without a default)

Actual behavior

No matter what the user input was, the view always renders the value t (String), hence it always casts to true in Alchemy::Ingredients::Boolean#value.

System configuration

• Alchemy Version: 6.0.0-rc1
• Rails Version: 6.1

[robinboening]

Note: it seems the value t only gets set when publishing the Page because in the preview frame it works as expected.

[robinboening]

This is confusing 😕

I just wasn't able to reproduce it locally, but I keep seeing this in production. The PublishPageJob worker runs in sidekiq and succeeds. The result is the t value in the Ingredient.

If I run the same locally in development (config.active_job.queue_adapter = :inline) it seems to works without an issue.

[tvdeyen]

What database are you using locally vs. remotely? Maybe it casts booleans differently?

[robinboening]

I am using postgres in both environments. I even attached the production database locally to see if I can replicate the issue locally, but no.

[robinboening]

I just attached my production redis locally and let the job be queued with sidekiq and I am seeing the issue now when publishing the page from my local machine (but PublishPageJob processed on production).

[robinboening]

I have tried to isolate the possible options that could cause the issue.

1. I run my app with RAILS_ENV=production locally and make it connect to my production postgres and memcache at heroku
2. I run sidekiq locally (the exact same command as on heroku) and make it connect to my production redis at heroku

Then, I replay the scenario by saving the element with the Boolean ingredient and publish the Page. And it works fine. 🤷

But if I publish the Page in my production heroku app using the IDENTICAL database, redis and memcache it does NOT work and stores a t as the value instead of 0 or 1 in the ingredient.

It must be related to the processing of the Job in heroku, but its not related to the database, redis, memcache. The funny (or sad) part is that I start the sidekiq worker on heroku bundle exec sidekiq the same way as I just did locally RAILS_ENV=production bundle exec sidekiq.

I checked if I have some strange ENV var set on heroku, but I could not find anything suspicious.

It sounds just impossible, but it's true. Any idea what could potentially be different and causing this on heroku?

[robinboening]

Now I've found something, but it doesn't yet help me understand the actual issue...

I had a default in elements.yml for the ingredient set to true. The t value was the first character drawn from that. I've tested with a default false and it made the ingredient store f as the value.

Okay, so that miracle is solved, but the issue with the ingredient for the element of the published Page remains. Note that the value for the Ingredient for the element on the PageVersion is always correct.

The following 3 scenarios describe the behaviour of the Ingredient attached to the Element that is attached to the published Page:

1. If I set a default of 1, the stored value will always be 1, even if the checkbox was not ticked.
2. If I remove the default, and untick the checkbox and publish the Page, the value is nil. I didn't notice this earlier because nil is falsy and due to how I define the control flow in my app it might behave correctly.
3. If I set a default of 0 it finally works as expected!

My conclusion: Right now there is something very strange going on with Boolean ingredients when they are created for the published Page and it's somehow related to default values defined in elements.yml. They only fully work as expected if they have a default value 0 defined.

[tvdeyen]

I think this is a form transmission issue:

An unticked checkbox is not transmitted by the form so the param is empty in the controller/model so the default is used. Only the ticked checkbox is transmitted and then this value is used.

We can fix this easily by adding an hidden field with the value of 0. This is usually what rails does if we use the form.checkbox helper. Maybe we don't use it properly. Will check now.

[tvdeyen]

Ok, that's not it. The hidden field is rendered out properly. Maybe it's how we set the default in the ingredient.

[tvdeyen]

I cannot reproduce the issue. I created a test in #2192 Feel free to make that test fail so we can reproduce the issue.

[robinboening]

The element/ingredient associated to the PageVersion works fine. It only happens when publishing a Page, so I excluded a form issue. I suspect the PagePublishJob -> DuplicateElement to be related or at least the entry point for digging deeper, but I haven't yet been able to find what might cause the issue. I also suspect this issue might be related to #2189

Thanks for adding the test. I think, though, since the test is a unit test and covers a creation of the ingredient without a wider context, it does not reflect the Page publishing scenario in which the issue appears to happen. I'll play around with it a bit more.

[robinboening]

I created a branch with a test showcasing the behaviour in a more integrative way using the Alchemy::Page::Publisher. This test is not intended to get merged. Its' sole purpose is to showcase and discuss the issue. I removed all the other tests from the file as the test setup was interfering and I wanted to focus only on this one test case.

If you checkout the branch and run the test locally (bundle exec rspec spec/models/alchemy/page/publisher_spec.rb), you'll see the following output

old raw value: nil
old casted value: nil
new raw value: t
new casted value: true

The issue

The boolean ingredient has a default value defined in elements.yml. I would expect this to be applied on the initial creation of the ingredient, but that is not the case. Funnily, it seems to get applied when the Page gets published and the element/ingredients get copied. The ingredient copy should really be a clone and must not have an altered value.

Some other things around this issue to discuss

• Due to the schema, ingredients always store values as String. Imho this is not ideal for the Boolean case as it stores "1" or "0" instead of real boolean values, but it's a trade off coming with STI and something we have to accept, I suppose.
• Currently, we allow 3 states for Boolean values. Do we want that? I prefer allowing false/true (or "0"/"1") only and prevent nil values by always casting those into false (or "0") before saving.

[robinboening]

@tvdeyen I just reassigned you just in case you haven't seen my last two comments after you unassigned yourself from the issue.

[tvdeyen]

@robinboening thanks, but I do not know why the ingredient does not have the default value in your case. As you can see in #2192 the default works just fine.

Due to the schema, ingredients always store values as String. Imho this is not ideal for the Boolean case as it stores "1" or "0" instead of real boolean values, but it's a trade off coming with STI and something we have to accept, I suppose.

Yes, I agree. This is a tradeoff. We could look into typecasting the values (like in https://github.com/byroot/activerecord-typedstore). It shouldnt be that hard to roll our own solution using ActiveRecords built in typecasting.

Currently, we allow 3 states for Boolean values. Do we want that? I prefer allowing false/true (or "0"/"1") only and prevent nil values by always casting those into false (or "0") before saving.

Yeah, I do not like three state booleans as well, but this is also due to the fact that this is a STI table and without type casting we need to deal with.

[tvdeyen]

Just a heads up here. We are in deed typecasting Boolean (and Date) values of ingredients.

But we only do this while reading the value, we should probably typecast the attribute before writing it to the database.

[tvdeyen]

@robinboening now that #2257 got merged, has this also been fixed?

[robinboening]

I haven't been able to test yet. I'll report back once I got to it.

[github-actions]

This issue has not seen any activity in a long time.
Please create a pull request with a fix or ask someone of the community if they can help.
This issue will be closed in 7 days if no further activity happens.

[github-actions]

This issue has not seen any activity in a long time.
If the issue described still exists in recent versions of Alchemy,
please open a new issue with. Thanks for reporting.