Allow Rails 6.1

[robinboening]

For now, this PR is supposed to be a draft for investigating the extend of supporting Rails 6.1 and eventually getting it ready to be merged into main.

[robinboening]

Unfortunately there are some failing tests. I was looking into it focusing on the recurring undefined method prefix_locale? error and I went deep into a rabbit hole 🕳️ 🐇

Rails 6.1 includes changes related to controller helper methods which is causing the error. I opened an issue in the rspec-rails repo as they might need to adapt to the changes as well. Let's see if it can be solved easily.

[tvdeyen]

@robinboening used the GH Draft feature to note that this PR is still WIP. I hope you don't mind :)

[tvdeyen]

Now that a shoulda-matchers version has been released and merged in #2161 that supports Rails 6.1 and Ruby 3 maybe some of the failing specs pass now? A rebase with latest main will tell

[afdev82]

Any news on that? I could rebase myself, but I don't want to "steal" others work :P

[robinboening]

@tvdeyen @afdev82 I keep rebasing this branch every here and then. I am currently using this branch on a project I am working on and it works well so far (not in production yet).

I just rebased again.

[robinboening]

As far as I can tell almost all the tests are failing for the same reason. In some places in the specs we inject helper methods to the controller. This pattern doesn't seem to work any longer with Rails 6.1, very likely because the view context is already created at the time when the include is sent to the controller.

[tvdeyen]

What baffles me are that there are specs failing for Rails 6.0 as well that are passing in main, so there might be another issue?
Reagarding the view specs we might need to stub even more helper methods? Maybe Rails 6.1 stopped injecting any helpers into the view context? The same might be true with the helper specs.

[tvdeyen]

@robinboening I pushed a fix for the picture ingredient editor spec. 0a485ff

[tvdeyen]

UPDATE

Three failing specs for 6.1

1. spec/models/alchemy/element_spec.rb:1015
   Fails because the hidden element that should be excluded from the collection is now included (even the SQL is correct, WAT?)
2. spec/models/alchemy/page_version_spec.rb:78
   Fails because the record is not touched anymore. Even rewriting it to an expect {}.to change {} does not help.
3. alchemy/admin/attachments/show.html.erb
   Works locally

One failing spec in Rails 6.0

1. spec/views/admin/attachments/show_spec.rb:10
   Same as with Rails 6.1 it works locally

As always no clues in the change logs. And why is a 6.0 related spec failing.
Rails x.1 releases. Fun since v2.1. The tradition lives on. 🙄

[robinboening]

FYI: I just rebased with current main branch.

[robinboening]

A couple of weeks back I've added a commit just for testing if it fixes some helper tests. It did if I remember correctly, but it is considered a hacky approach and should be avoided as John Hawthorn states rails/rails#40204 (comment)

I haven't had the time to drill deeper into that so I haven't found a better solution.

[tvdeyen]

A couple of weeks back I've added a commit just for testing if it fixes some helper tests. It did if I remember correctly, but it is considered a hacky approach and should be avoided as John Hawthorn states rails/rails#40204 (comment)

I haven't had the time to drill deeper into that so I haven't found a better solution.

@robinboening I added two commits 2acb952 and 0915db8

that made that not necessary anymore

[tvdeyen]

I fixed a couple of specs, but there is one bug in Rails:

In spec/models/alchemy/element_spec.rb:1030 this is the SQL (postgres) generated

SELECT "alchemy_elements".* FROM "alchemy_elements" WHERE "alchemy_elements"."parent_element_id" = 91 AND "alchemy_elements"."public" = TRUE ORDER BY "alchemy_elements"."position" ASC;

This is the collection returned by Rails

[#<Alchemy::Element:0x00005621e3a0ae50
  id: 92,
  name: "article",
  position: 1,
  public: true,
  folded: false,
  unique: false,
  created_at: Thu, 02 Sep 2021 07:56:42.502623000 UTC +00:00,
  updated_at: Thu, 02 Sep 2021 07:56:42.502623000 UTC +00:00,
  creator_id: nil,
  updater_id: nil,
  parent_element_id: 91,
  fixed: false,
  page_version_id: 196,
  tag_names: nil>,
 #<Alchemy::Element:0x00005621e2cf3e60
  id: 93,
  name: "article",
  position: 2,
  public: false,
  folded: false,
  unique: false,
  created_at: Thu, 02 Sep 2021 07:56:42.507076556 UTC +00:00,
  updated_at: Thu, 02 Sep 2021 07:56:42.507076556 UTC +00:00,
  creator_id: nil,
  updater_id: nil,
  parent_element_id: 91,
  fixed: false,
  page_version_id: 196,
  tag_names: nil>]

You clearly can see that Rails is not respecting the public = TRUE from the SQL and returning a non public element.

Thank you, Rails!

[tvdeyen]

Fixed all specs besides one. The spec/views/admin/attachments/show_spec.rb fails if we run all specs, but passes if we run it in isolation. Meaning we somehow mutate global state somewhere.

[tvdeyen]

Wohooo. We got it. Thanks @robinboening for the initial work.