-
Notifications
You must be signed in to change notification settings - Fork 109
Conversation
The task list polls every 3s via a data-refresh attribute. This adds a UI toggle that persists the preference with ?refresh=false in the URL. Co-authored-by: Cursor <cursoragent@cursor.com>
Updated the task page to auto-refresh while there are active runs, ensuring real-time status updates. Added a toggle for enabling/disabling auto-refresh on both the task list and individual task pages, with appropriate links displayed based on the current state. Adjusted tests to verify the new behavior.
@adrianna-chang-shopify
adrianna-chang-shopify
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @knightq , thanks for contributing to the gem! This change makes sense to me, I left a few comments.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we move this whole block into the refreshed wrapper (<%= tag.div(data: { refresh: (@task.refresh? && @refresh) || "" }) do %>)? Otherwise, when a task finishes, we'll still show the disable / enable button even though @task.refresh? is false, because we won't have refreshed this part of the page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd prefer we follow the convention of the existing system tests and test the toggle behaviour in test/system/maintenance_tasks/tasks_test.rb.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should filter out refresh as a param here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we could name this something more intention revealing now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit, to differentiate it from the bg a bit more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Uh oh!
There was an error while loading. Please reload this page.
Closes #1490
Problem
The Maintenance Tasks web UI auto-refreshes pages every 3 seconds to keep run statuses up to date. This behavior is driven by JavaScript in the application layout: when an element with
data-refreshis present and truthy, the page periodically fetches the current URL and replaces the refreshed content in place.While this is useful when monitoring active runs, it can be disruptive in other situations:
Two pages are affected:
TasksController#index)@refreshwas hard-coded totrueTasksController#show)@task.refresh?)@task.refresh?There was no user-facing way to turn auto-refresh off on either page.
Proposed solution
Add a user-facing toggle on the task show page (when the task has active runs) to disable and re-enable auto-refresh. Respect a shared
?refresh=falseURL parameter in the controller so the preference applies consistently across both pages.Behavior
Task show page (primary UI)
The toggle is shown only when the task has active runs (
@task.refresh?). When there are no active runs, the page does not auto-refresh and no toggle is displayed.data-refreshattribute/maintenance_tasks/tasks/:idtrue(when active runs exist)/maintenance_tasks/tasks/:id?refresh=false?refresh=false.Task list page
The task list continues to auto-refresh by default and respects
?refresh=falsewhen present, but does not include a toggle control. Users who disable auto-refresh on a task show page can keeprefresh=falsein the URL when navigating back to the list, or append it manually.data-refreshattribute/maintenance_tasks/true/maintenance_tasks/?refresh=falseImplementation outline
Controller — Run
set_refreshon bothindexandshow, and read therefreshquery parameter:Task show view — In
app/views/maintenance_tasks/tasks/show.html.erb:@task.refresh?is true.data-refreshfrom@task.refresh? && @refreshso both active-run status and the user preference are respected.Task list view — In
app/views/maintenance_tasks/tasks/index.html.erb, continue settingdata-refreshfrom@refresh(no toggle UI).Tests — Integration tests in
test/integration/maintenance_tasks/tasks_controller_test.rbcovering:[data-refresh=true]present).refresh=falsedisables auto-refresh on the task show page ([data-refresh=true]absent).Documentation — Document the toggle and
?refresh=falseURL parameter in the README (under "Running a Task from the Web UI").Mocks
Scope
?refresh=falsesupport on both index and show actions.Acceptance criteria
?refresh=falseURL parameter.