Show both the delayed and external-lock stack banners at once

[ScottifyShopson]

What & why

On a stack's page, two banners can apply at the same time:

• "Continuous Delivery Delayed!" (blue) — CD is paused because checks haven't passed.
• "Stack has been locked by external system!" (orange) — the stack is failing its external deployment checks.

app/views/shipit/stacks/_banners.html.erb rendered these as an if / elsif, so whenever continuous_delivery_delayed? was true the orange banner was suppressed entirely:

<% if stack.continuous_delivery_delayed? %>
  ...blue...
<% elsif !stack.deployment_checks_passed? %>
  ...orange...
<% end %>

These banners convey complementary information. The blue banner only says, generically, that "the pre-deploy checks failed", while the orange banner is the only place the actual lock reason from the external system is surfaced (via deployment_checks_message, e.g. an Infra Central message explaining why deploys are locked). When the blue banner hid the orange one, users lost that reason.

This was observed on shopify/minerva/production, where the stack was both CD-delayed and locked by an external system — only the blue banner showed, hiding the lock reason.

What changed

Split the elsif into a separate if so the two conditions are evaluated independently and both banners can render when both apply. No change to either banner's content or styling.

Current behaviour (the two banners involved)

The orange external-lock banner (the one currently being hidden), which carries the actual lock reason:

The blue continuous-delivery-delayed banner, which currently takes precedence and hides the orange one:

After this change, both render at once when both conditions hold.

Testing

• Added a controller test (StacksControllerTest#show renders both the delayed and external-lock banners when both apply) that sets up a stack which is simultaneously continuous_delivery_delayed? and failing deployment_checks, and asserts both banner titles render.
• ERB and Ruby syntax verified locally. I was unable to run the full test suite locally due to a pg native-gem build issue in my environment; relying on CI to run the suite.

Notes

• The screenshots above are hosted on a throwaway branch in my fork (scottifyshopson/banner-screenshots-assets) purely so they render here; they are not part of this PR's diff and that branch can be deleted after merge.

🤖 Generated with anthropic/claude-opus-4-8 via pi

[tdickers]

Thank you very much!

---

Review assisted by pair-review

[tdickers]

💬 Suggestion: The new test covers the important positive case where delayed CD and failing deployment checks both render. A sibling test for delayed CD with passing deployment checks would harden the opposite side of the contract: the orange external-lock banner should not render just because delayed CD is active.

Suggestion: Add a sibling test that registers a passing fake deployment check while continuous_delivery_delayed? is true, then asserts the blue 'Continuous Delivery Delayed!' banner renders and the orange 'Stack has been locked by external system!' banner has count: 0.

[tdickers]

💡 Improvement: Consider tightening this regression test so it verifies the specific user-facing reason that motivated the PR. The external deployment-lock reason is surfaced through Shipit.deployment_checks.message(stack) when the checker responds to message. As written, the fake checker only returns false and the assertion only checks the orange banner title, so the test would still pass if the banner appeared but the external-system reason was missing or replaced by a generic message.

Suggestion: Define self.message(_stack) on failing_checks with a distinct sentinel string, then assert that string appears inside the orange banner text after get :show. That keeps the test aligned with the deployment_checks_message contract and the user-visible reason this PR is protecting.

[tdickers]

🎨 Code Style: The test mutates Shipit.deployment_checks, a module-level global. Resetting it to nil is equivalent today because the attribute has no default, but it could clobber suite-level or setup-provided configuration if that changes later. Restoring the previous value also matches the safer pattern already used for this same global elsewhere in the test suite.

Suggestion: Capture the previous value before assigning failing_checks, then restore that value in the ensure block instead of unconditionally setting Shipit.deployment_checks = nil.

Suggested change

	Shipit.deployment_checks = nil
	Shipit.deployment_checks = original_deployment_checks