Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -856,6 +856,11 @@ end

You can run your new Task by accessing the Web UI and clicking on "Run".

A Task's page auto-refreshes in the background while it has active runs, so the
progress and run statuses stay up to date. You can turn this off with the
"Disable auto-refresh" toggle on the Task page, which adds a `?refresh=false`
parameter to the URL. Use "Enable auto-refresh" to turn it back on.

#### Running a Task from the command line

Alternatively, you can run your Task in the command line:
Expand Down
4 changes: 2 additions & 2 deletions app/controllers/maintenance_tasks/tasks_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ module MaintenanceTasks
#
# @api private
class TasksController < ApplicationController
before_action :set_refresh, only: [:index]
before_action :set_refresh, only: [:index, :show]

# Renders the maintenance_tasks/tasks page, displaying
# available tasks to users, grouped by category.
Expand All @@ -28,7 +28,7 @@ def show
private

def set_refresh
@refresh = true
@refresh = params[:refresh] != "false"

Copy link
Copy Markdown
Contributor

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.

Suggested change
@refresh = params[:refresh] != "false"
@auto_refresh_enabled = params[:refresh] != "false"

end
end
end
11 changes: 10 additions & 1 deletion app/views/maintenance_tasks/tasks/show.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,16 @@
</details>
<% end %>

<%= tag.div(data: { refresh: @task.refresh? || "" }) do %>
<% if @task.refresh? %>

Copy link
Copy Markdown
Contributor

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.

<div class="is-flex is-justify-content-flex-end mb-2">
<% if @refresh %>
<%= link_to "Disable auto-refresh", task_path(@task, refresh: false), class: "button is-small" %>

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
<%= link_to "Disable auto-refresh", task_path(@task, refresh: false), class: "button is-small" %>
<%= link_to "Disable auto-refresh", task_path(@task, refresh: false), class: "button is-small is-light" %>

Nit, to differentiate it from the bg a bit more.

<% else %>
<%= link_to "Enable auto-refresh", task_path(@task), class: "button is-small" %>

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
<%= link_to "Enable auto-refresh", task_path(@task), class: "button is-small" %>
<%= link_to "Enable auto-refresh", task_path(@task), class: "button is-small is-light" %>

<% end %>
</div>
<% end %>
<%= tag.div(data: { refresh: (@task.refresh? && @refresh) || "" }) do %>
<% if @task.active_runs.any? %>
<hr>

Expand Down
47 changes: 47 additions & 0 deletions test/integration/maintenance_tasks/tasks_controller_test.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
# frozen_string_literal: true

require "test_helper"

module MaintenanceTasks
class TasksControllerTest < ActionDispatch::IntegrationTest

Copy link
Copy Markdown
Contributor

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.

# Maintenance::UpdatePostsTask has a paused (active) run, so its show page
# refreshes by default.
ACTIVE_TASK = "Maintenance::UpdatePostsTask"

test "task page auto-refreshes by default when there are active runs" do
get maintenance_tasks.task_path(ACTIVE_TASK)

assert_response :success
assert_select "[data-refresh=true]"
end

test "task page does not auto-refresh when refresh=false is passed" do
get maintenance_tasks.task_path(ACTIVE_TASK), params: { refresh: "false" }

assert_response :success
assert_select "[data-refresh=true]", false
end

test "task page shows a link to disable auto-refresh when refreshing" do
get maintenance_tasks.task_path(ACTIVE_TASK)

assert_response :success
assert_select "a[href*='refresh=false']", text: "Disable auto-refresh"
end

test "task page shows a link to enable auto-refresh when not refreshing" do
get maintenance_tasks.task_path(ACTIVE_TASK), params: { refresh: "false" }

assert_response :success
assert_select "a", text: "Enable auto-refresh"
end

test "task page shows no auto-refresh toggle when there are no active runs" do
get maintenance_tasks.task_path("Maintenance::ImportPostsTask")

assert_response :success
assert_select "a", text: "Disable auto-refresh", count: 0
assert_select "a", text: "Enable auto-refresh", count: 0
end
end
end
Loading