[#70336] Sections in meeting agenda not displayed correctly when moved

[mrmir]

Ticket

https://community.openproject.org/wp/70336

What approach did you choose and why?

I think the bug is related to rails/rails#6769, where the :includes is messing up the default ordering, and so using :preload instead fixes the issue

However, I don't understand it fully and this might just be a band aid fix

Merge checklist

• Added/updated tests

[cbliard]

Took me some time to figure it out but works well 👏

I have some suggestions about the test to make it more expressive.

[cbliard]

As it's used twice in the test, I would put this into show_page as an helper.

Also, I think it can select only the header element, so it's start_with?("meeting-section-header-container-")

And the condition can be expressed using css with [attr^='value'] which means "starts with".

it becomes:

sections = page.all(".op-meeting-section-container[data-test-selector^='meeting-section-header-container-']")

once you have extracted this into a #section_headers, you can then write the assertions like this:

expect(show_page.section_headers.map(&:text))
  .to eq(["Section A",
          "Section C",
          "Section B"])

[cbliard]

It may also be better to have show_page.section_headers return text directly. Actually I think anything returning capybara object should be suffixed with element, so if it returns the capybara objects, it should be show_page.section_header_elements.

Yep, probably better to have show_page.section_headers return text directly. If someone needs the elements at some point, they can refactor.

[mrmir]

Done, thanks for the feedback 👍🏽

[mrmir]

Merging as the spec is flaky, but is passing locally