Clickable uptime days - filter probe results by date

[lewispb]

Supports jumping from downtime on the uptime page to specific failure logs.

[Copilot]

Pull request overview

Adds a “date” filter to probe results and makes uptime-day bars clickable so users can jump from the uptime dashboard into a pre-filtered probe results view for a specific day.

Changes:

• Introduces date query param support (controller + model scope) and displays it in the results summary.
• Updates views to preserve/clear the date filter and to link uptime bars to the filtered results page.
• Adds styling for clickable uptime bars and an integration test for the date filter UI.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
test/integration/probe_results_controller_test.rb	Adds integration coverage for the new date filter UI.
app/views/upright/probe_results/index.html.erb	Preserves date in the filter form and adds a “Clear date filter” link.
app/views/upright/dashboards/_uptime_probe_row.html.erb	Makes uptime bars and probe name link to the site probe results with date applied; uses current_or_default_site.
app/models/upright/probe_result.rb	Adds by_date scope to filter results by created_at day range.
app/helpers/upright/probe_results_helper.rb	Appends the selected date to the results summary text.
app/controllers/upright/probe_results_controller.rb	Applies by_date to the probe results query.
app/assets/stylesheets/upright/uptime-bars.css	Adjusts cursor/styling for <a> vs <div> uptime bars.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

This link_to renders an empty <a> element (no text content). That makes the clickable uptime bar difficult/impossible to use with screen readers and can be hard to focus/navigate. Add an accessible name (e.g. aria-label describing the date/probe) or include visually-hidden text inside the link.

Suggested change

	title: uptime_bar_tooltip(date, uptime_percent, downtime_minutes) %>
	title: uptime_bar_tooltip(date, uptime_percent, downtime_minutes),
	aria: { label: uptime_bar_tooltip(date, uptime_percent, downtime_minutes) } %>

[Copilot]

The new date filter test only asserts that the summary text/link appears, but it doesn't verify that results are actually filtered by created_at. Consider creating at least one result outside the selected day (or adjusting fixtures) and asserting it's excluded when date is provided.

Suggested change

	get upright.site_root_path(date: Date.current.iso8601)
	assert_response :success
	assert_select "p", text: /on #{Date.current.to_fs(:long)}/
	assert_select "a.filter-clear", text: /Clear date filter/
	selected_date = Date.current
	Upright::ProbeResult.create!(
	probe_type: "http",
	probe_name: "Result on selected date",
	status: :ok,
	duration: 100,
	created_at: selected_date.in_time_zone.change(hour: 12),
	updated_at: selected_date.in_time_zone.change(hour: 12)
	)
	Upright::ProbeResult.create!(
	probe_type: "http",
	probe_name: "Result on different date",
	status: :ok,
	duration: 200,
	created_at: (selected_date - 1.day).in_time_zone.change(hour: 12),
	updated_at: (selected_date - 1.day).in_time_zone.change(hour: 12)
	)
	get upright.site_root_path(date: selected_date.iso8601)
	assert_response :success
	assert_select "p", text: /on #{selected_date.to_fs(:long)}/
	assert_select "a.filter-clear", text: /Clear date filter/
	# Only results from the selected date should be shown
	assert_select "*", text: "Result on selected date"
	assert_select "*", text: "Result on different date", count: 0

[Copilot]

by_date uses Date.parse(date) directly. Invalid or unexpected date params (e.g. user input) will raise ArgumentError and 500 the index. Consider strict parsing (e.g. Date.iso8601) and gracefully ignoring/handling invalid values (rescue and return all/none as appropriate), ideally using Time.zone for day boundaries.

Suggested change

	scope :by_date, ->(date) { where(created_at: Date.parse(date).all_day) if date.present? }
	scope :by_date, lambda { |date|
	next unless date.present?
	begin
	parsed_date = Date.iso8601(date.to_s)
	rescue ArgumentError
	nil
	else
	where(created_at: parsed_date.in_time_zone.all_day)
	end
	}

[Copilot]

results_summary calls Date.parse(params[:date]). If an invalid date is provided, this will raise and break page rendering. Use strict parsing with rescue (or a helper method shared with the model scope) and only append the "on …" fragment when parsing succeeds.

[claude]

🔴 The link_to on line 14 is called without a block or body text, so Rails uses the full URL string as the visible anchor text. This renders a long URL like http://example.com/upright?probe_type=http&probe_name=foo&date=2026-03-10 inside a 4-12px wide bar with no overflow hiding. Use the block form (link_to url, class: "...", title: "..." do end) to match the pattern on line 2 of this same file.

Extended reasoning...

What the bug is

The link_to call on lines 14-16 of _uptime_probe_row.html.erb is:

<%= link_to upright.site_root_url(subdomain: current_or_default_site.code, ...),
      class: "uptime-bar uptime-bar--#{uptime_label(uptime_percent)}",
      title: uptime_bar_tooltip(date, uptime_percent, downtime_minutes) %>

This calls link_to with one positional argument (the URL string) followed by keyword arguments (class:, title:). Without a block, Rails interprets this as link_to(name, options) where name is the URL string (used as visible text) and options is the hash {class: ..., title: ...} (used for URL generation, not as HTML attributes).

How it manifests

The rendered HTML will look something like:

<a href="/wrong-url">http://example.com/upright?probe_type=http&probe_name=foo&date=2026-03-10</a>

Two problems occur: (1) the full URL string becomes the visible link text inside a bar that is only 4-12px wide and 24px tall, and (2) the class and title are passed to url_for as route parameters instead of being applied as HTML attributes, so the <a> tag gets no CSS class (no colored bar styling) and no tooltip. The url_for({class: ..., title: ...}) call may also raise a routing error.

Why existing code doesn't prevent it

The CSS for .uptime-bar and a.uptime-bar has no overflow: hidden, font-size: 0, text-indent, or color: transparent that would hide overflow text. The old code used a plain <div class="uptime-bar ..."></div> which had no text content at all, so this was never an issue before.

Step-by-step proof

1. Rails link_to signature without a block: link_to(name = nil, options = nil, html_options = nil)
2. The call passes: name = upright.site_root_url(...) (a string), options = {class: "uptime-bar ...", title: "..."}
3. Rails calls url_for(options) to generate the href, passing {class: ..., title: ...} which are not valid route params
4. Rails uses name as the anchor body text, rendering the full URL inside the <a> tag
5. No html_options are set, so the <a> tag gets no class or title attributes
6. Compare with line 2 of the same file: link_to url, class: "data-table__probe" do ... end — the block form correctly swaps the arguments so class: becomes an HTML attribute

Impact

Every uptime bar with data will either render a broken link with the full URL as visible text (overflowing the tiny bar and breaking the layout), or raise a routing error from invalid url_for params, potentially crashing the entire dashboard page.

Fix

Use the block form with empty content to match the pattern on line 2:

<%= link_to upright.site_root_url(...),
      class: "uptime-bar uptime-bar--...",
      title: uptime_bar_tooltip(...) do %><% end %>

Or pass an empty string as the body: link_to "", upright.site_root_url(...), class: "...", title: "..."

[claude]

🟡 Nit: Date.parse(date) in the by_date scope (probe_result.rb:11) and results_summary (probe_results_helper.rb:47) will raise an unhandled Date::Error on malformed date params (e.g., ?date=notadate), resulting in a 500 error. Since the date param is user-controllable via URL, consider wrapping with rescue Date::Error to return the unfiltered scope / skip the summary fragment gracefully.

Extended reasoning...

What the bug is

The new by_date scope on line 11 of probe_result.rb and the summary line on line 47 of probe_results_helper.rb both call Date.parse(...) on the raw date URL parameter. The only guard is if date.present?, which checks for blank/nil but does not validate that the string is a parseable date. If the string is present but not a valid date (e.g., notadate, 2025-13-45, abc), Date.parse raises Date::Error.

How it manifests

A user (or bot/scanner) navigating to a URL like /probe_results?date=notadate will hit the by_date scope. Date.parse("notadate") raises Date::Error, which propagates up as an unhandled exception, resulting in a 500 Internal Server Error.

Why existing code does not prevent it

The other filter scopes (by_type, by_status, by_name) pass values directly into where() clauses, which gracefully return empty result sets for invalid values. The by_date scope is unique in that it calls Date.parse before constructing the query, introducing a parsing step that can raise. There is no rescue_from in the controller hierarchy -- Upright::ProbeResultsController inherits from Upright::ApplicationController, which has no global exception handling for Date::Error or ArgumentError.

Step-by-step proof

1. A user visits /?date=notadate.
2. probe_results_controller.rb:15 calls .by_date(params[:date]) with params[:date] = "notadate".
3. In probe_result.rb:11, "notadate".present? returns true, so the scope body executes.
4. Date.parse("notadate") raises Date::Error: invalid date.
5. No rescue block or rescue_from catches this, so Rails returns a 500 error.
6. Even if the scope were rescued, the same issue exists in probe_results_helper.rb:47 where Date.parse(params[:date]).to_fs(:long) would also raise during view rendering.

Impact

This is a minor robustness issue. In normal usage, the date parameter is generated by the application itself via date.iso8601 in the uptime bar links, so valid dates are always provided through the UI. However, the parameter is user-controllable through the URL bar, and a 500 on user-supplied input is not ideal -- especially since bots and scanners may produce arbitrary query strings. The impact is limited since this is an authenticated internal monitoring tool.

Suggested fix

The simplest fix is to rescue Date::Error in both locations. For the scope, the inline rescue will cause it to return nil (skipping the filter). For the helper, wrap it similarly. Alternatively, a before_action in the controller could validate the date param and set it to nil if invalid, which would be cleaner and handle both locations at once.

[claude]

🟣 Pre-existing issue: _matrix.html.erb:17 still uses Upright.sites.first.code while this PR correctly updated the identical pattern in _uptime_probe_row.html.erb to current_or_default_site.code. When Upright::Current.site is set to something other than the first site, the matrix partial will generate links pointing to the wrong site. Since the PR was already fixing this pattern, it would be worth updating the matrix partial as well.

Extended reasoning...

What the bug is

The file app/views/upright/dashboards/probe_statuses/_matrix.html.erb at line 17 uses Upright.sites.first.code to build a URL subdomain. This PR specifically changed the identical pattern in app/views/upright/dashboards/_uptime_probe_row.html.erb (line 2) from Upright.sites.first.code to current_or_default_site.code, but missed the occurrence in the matrix partial.

The code path

The helper current_or_default_site is defined in app/helpers/upright/application_helper.rb:3 as:

Upright::Current.site || Upright.sites.first

This means current_or_default_site.code will return the current site's code when one is set via Upright::Current.site, and only falls back to Upright.sites.first when no current site is set. The hardcoded Upright.sites.first.code in the matrix partial always returns the first configured site, ignoring any current site context.

Step-by-step proof

1. Assume the application has two sites configured: ams (first) and chi (second).
2. A user navigates to the chi site dashboard, setting Upright::Current.site to the chi site.
3. In _uptime_probe_row.html.erb, the PR now correctly generates links like upright.site_root_url(subdomain: "chi", ...) because current_or_default_site.code returns "chi".
4. However, in _matrix.html.erb:17, the link is still generated as upright.site_root_url(subdomain: Upright.sites.first.code, ...) which always resolves to "ams" regardless of the current site.
5. The user clicks a probe link in the matrix view and is unexpectedly taken to the ams site's probe results instead of chi's.

Impact

When a user is viewing a site other than the first configured site, probe links in the matrix partial will navigate to the wrong site's results. This creates an inconsistent experience since the uptime row partial (fixed in this PR) now correctly respects the site context while the matrix partial does not.

How to fix

In app/views/upright/dashboards/probe_statuses/_matrix.html.erb:17, change:

Upright.sites.first.code

to:

current_or_default_site.code

This is the exact same change this PR already made in _uptime_probe_row.html.erb.

Classification

This is a pre-existing issue — the matrix partial was not modified by this PR. However, since the PR explicitly fixed this exact pattern in a sibling partial, it would be natural and low-risk to include the matrix partial fix in the same PR.