Fix merge trains checkbox race on project settings page
What does this MR do and why?
This MR removes the GraphQL fetch from ee/app/assets/javascripts/pages/projects/edit/merge_options.js and reads the checkbox state that the Rails form already renders on the page. The merge trains checkbox's disabled state and the change listener on the merged results pipelines checkbox are now set synchronously, instead of inside a resolved Apollo promise.
This closes a race that showed a stale disabled state on the merge trains checkbox and made clicks on the merged results pipelines checkbox do nothing until the query resolved. It also fixes the flaky feature spec reported in the linked issue.
The fetch's now-unused error alert and its translation string were removed, along with the file's entry in .rubocop_todo/rspec/avoid_wait_for_requests.yml, since the spec no longer needs wait_for_requests.
Root cause
Project#merge_pipelines_enabled and #merge_trains_enabled are delegated to ci_cd_settings, and the GraphQL fields ciCdSettings.mergePipelinesEnabled / mergeTrainsEnabled resolve to the same boolean columns. The Rails form already renders both checkboxes' checked attribute from that data, so the GraphQL query in merge_options.js was redundant.
The original implementation (commit ec9a5dc8, January 2021) attached the change listener synchronously at module scope. Commit 50466932 ("Refactor init logic of merge request options on project settings page", August 2021) moved both the listener and the initial disabled state into the resolved GraphQL promise. That introduced a window, between page load and query resolution, during which the merge trains checkbox showed the wrong disabled state, clicks on the merged results pipelines checkbox had no effect, and the resolving query overwrote anything the user had toggled in that window.
The spec compounded this by sampling state through non-waiting Capybara node predicates (find('#id').disabled?), which observe a single instant with no retry.
Note that waiting matchers alone could not have made this spec reliable. When merged results pipelines is enabled, the pre-init and post-init DOM are identical, so no assertion can distinguish "initialized" from "not yet initialized". The server-rendered disabled attribute that would have provided that signal was deliberately removed in commit 5bba3aa9 ("Do not disable Merge Pipelines checkbox initially", August 2021). Removing the race was therefore the only reliable fix.
javascript_include_tag is overridden in this codebase to always set defer: true (see app/helpers/gitlab_script_tag_helper.rb), so page entrypoints run before DOMContentLoaded. Reading the server-rendered state and wiring up the listener synchronously means initialization completes before the page is interactive, which removes the race for real users and for Capybara, since Selenium's visit waits for the load event.
How to verify
- Open a project's Settings > Merge requests page.
- With "Enable merged results pipelines" unchecked, confirm "Enable merge trains" is disabled immediately on page load, with no flash of the wrong state.
- Check "Enable merged results pipelines" and confirm "Enable merge trains" becomes enabled immediately. Uncheck it again and confirm "Enable merge trains" becomes disabled and unchecked.
- Run the Jest spec:
ee/spec/frontend/pages/projects/edit/merge_options_spec.js. - Run the feature specs:
ee/spec/features/projects/settings/merge_requests/disable_merge_trains_setting_spec.rb,user_manages_merge_trains_spec.rbanduser_manages_merge_pipelines_spec.rb.
Results locally: Jest passed 7 examples with 0 failures. The target feature spec passed 21 of 21 across three different seeds, and 27 of 27 when run together with the two adjacent settings specs.
Before the fix, run against a build containing the old asynchronous code, the target spec failed 3 of 21 examples: the three that uncheck the merged results pipelines checkbox. Two of those correspond to the failures reported in the issue (former lines 64 and 114). The third reported failure (former line 88) is the initial-state assertion in the "merge pipelines is disabled" context, which the non-waiting predicate caused and the waiting matcher fixes.
Removing the per-page-load GraphQL round trip also roughly halved the spec file's runtime.
Related issues
Fixes https://gitlab.com/gitlab-org/quality/test-failure-issues/-/work_items/44470