loading up the forgejo repo on tangled to test page performance
0
fork

Configure Feed

Select the types of activity you want to include in your feed.

feat(ui): Automatically refresh workflows in the "Actions" list (#7361)

- Make the "Actions" list (for example, https://codeberg.org/forgejo/forgejo/actions) dynamically refresh using htmx and partial page reloading. This addresses a pet peeve of mine, I find it common to end up on this page and have workflows in-progress, but not be able to monitor the workflows to success or failure from the page as it currently doesn't do any data refreshing.
- There are a few major risks involves with this change.
- Increased server-side load & network utilization. In order to mitigate this risk, I have configured the refresh to occur every 30 seconds **only** when the Page Visibility API indicates that the web page is currently visible to the end-user. It is still reasonable to assume this change will increase server-side load though.
- UI interactions on the page, such as the "Actor" and "Status" dropdown and the workflow dispatch form, would be replaced from the server with non-expanded UI during the refresh. This problem is prevented by stopping the refresh while these UIs are in their expanded states.
- E2E tests added.

Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/7361
Reviewed-by: Gusted <gusted@noreply.codeberg.org>
Co-authored-by: Mathieu Fenniak <mathieu@fenniak.net>
Co-committed-by: Mathieu Fenniak <mathieu@fenniak.net>

authored by

Mathieu Fenniak
Mathieu Fenniak
and committed by
Gusted
6ad706aa c977585e

+270 -98
+10 -3
routers/web/repo/actions/actions.go
··· 31 31 ) 32 32 33 33 const ( 34 - tplListActions base.TplName = "repo/actions/list" 35 - tplViewActions base.TplName = "repo/actions/view" 34 + tplListActions base.TplName = "repo/actions/list" 35 + tplListActionsInner base.TplName = "repo/actions/list_inner" 36 + tplViewActions base.TplName = "repo/actions/view" 36 37 ) 37 38 38 39 type Workflow struct { ··· 66 67 67 68 curWorkflow := ctx.FormString("workflow") 68 69 ctx.Data["CurWorkflow"] = curWorkflow 70 + 71 + listInner := ctx.FormBool("list_inner") 69 72 70 73 var workflows []Workflow 71 74 if empty, err := ctx.Repo.GitRepo.IsEmpty(); err != nil { ··· 250 253 ctx.Data["Page"] = pager 251 254 ctx.Data["HasWorkflowsOrRuns"] = len(workflows) > 0 || len(runs) > 0 252 255 253 - ctx.HTML(http.StatusOK, tplListActions) 256 + if listInner { 257 + ctx.HTML(http.StatusOK, tplListActionsInner) 258 + } else { 259 + ctx.HTML(http.StatusOK, tplListActions) 260 + } 254 261 } 255 262 256 263 // loadIsRefDeleted loads the IsRefDeleted field for each run in the list.
+30 -81
templates/repo/actions/list.tmpl
··· 4 4 <div class="ui container"> 5 5 {{template "base/alert" .}} 6 6 7 - {{if .HasWorkflowsOrRuns}} 8 - <div class="ui stackable grid"> 9 - <div class="four wide column"> 10 - <div class="ui fluid vertical menu"> 11 - <a class="item{{if not $.CurWorkflow}} active{{end}}" href="?actor={{$.CurActor}}&status={{$.CurStatus}}">{{ctx.Locale.Tr "actions.runs.all_workflows"}}</a> 12 - {{range .workflows}} 13 - <a class="item{{if eq .Entry.Name $.CurWorkflow}} active{{end}}" href="?workflow={{.Entry.Name}}&actor={{$.CurActor}}&status={{$.CurStatus}}">{{.Entry.Name}} 14 - {{if .ErrMsg}} 15 - <span data-tooltip-content="{{.ErrMsg}}"> 16 - {{svg "octicon-alert" 16 "text red"}} 17 - </span> 18 - {{end}} 7 + {{/* Refresh the list every interval (30s) unless the document isn't visible or a dropdown is open; refresh 8 + if visibility changes as well. simulate-polling-interval is a custom event used for e2e tests to mimic 9 + the polling interval and should be defined identically to the `every` clause for accurate testing. */}} 10 + <div 11 + hx-get="?workflow={{$.CurWorkflow}}&actor={{$.CurActor}}&status={{$.CurStatus}}&page={{$.Page.Paginater.Current}}&list_inner=true" 12 + hx-swap="morph:innerHTML" 13 + hx-trigger="every 30s [pollingOk()], visibilitychange[document.visibilityState === 'visible'] from:document, simulate-polling-interval[pollingOk()] from:document" 14 + hx-indicator="#reloading-indicator"> 15 + {{template "repo/actions/list_inner" .}} 16 + </div> 17 + </div> 18 + </div> 19 19 20 - {{if $.ActionsConfig.IsWorkflowDisabled .Entry.Name}} 21 - <div class="ui red label">{{ctx.Locale.Tr "disabled"}}</div> 22 - {{end}} 23 - </a> 24 - {{end}} 25 - </div> 26 - </div> 27 - <div class="twelve wide column content"> 28 - <div class="ui secondary filter menu tw-justify-end tw-flex tw-items-center"> 29 - <!-- Actor --> 30 - <div class="ui{{if not .Actors}} disabled{{end}} dropdown jump item"> 31 - <span class="text">{{ctx.Locale.Tr "actions.runs.actor"}}</span> 32 - {{svg "octicon-triangle-down" 14 "dropdown icon"}} 33 - <div class="menu"> 34 - <div class="ui icon search input"> 35 - <i class="icon">{{svg "octicon-search"}}</i> 36 - <input type="text" placeholder="{{ctx.Locale.Tr "actions.runs.actor"}}"> 37 - </div> 38 - <a class="item{{if not $.CurActor}} active{{end}}" href="?workflow={{$.CurWorkflow}}&status={{$.CurStatus}}&actor=0"> 39 - {{ctx.Locale.Tr "actions.runs.actors_no_select"}} 40 - </a> 41 - {{range .Actors}} 42 - <a class="item{{if eq .ID $.CurActor}} active{{end}}" href="?workflow={{$.CurWorkflow}}&actor={{.ID}}&status={{$.CurStatus}}"> 43 - {{ctx.AvatarUtils.Avatar . 20}} {{.GetDisplayName}} 44 - </a> 45 - {{end}} 46 - </div> 47 - </div> 48 - <!-- Status --> 49 - <div class="ui dropdown jump item"> 50 - <span class="text">{{ctx.Locale.Tr "actions.runs.status"}}</span> 51 - {{svg "octicon-triangle-down" 14 "dropdown icon"}} 52 - <div class="menu"> 53 - <div class="ui icon search input"> 54 - <i class="icon">{{svg "octicon-search"}}</i> 55 - <input type="text" placeholder="{{ctx.Locale.Tr "actions.runs.status"}}"> 56 - </div> 57 - <a class="item{{if not $.CurStatus}} active{{end}}" href="?workflow={{$.CurWorkflow}}&actor={{$.CurActor}}&status=0"> 58 - {{ctx.Locale.Tr "actions.runs.status_no_select"}} 59 - </a> 60 - {{range .StatusInfoList}} 61 - <a class="item{{if eq .Status $.CurStatus}} active{{end}}" href="?workflow={{$.CurWorkflow}}&actor={{$.CurActor}}&status={{.Status}}"> 62 - {{.DisplayedStatus}} 63 - </a> 64 - {{end}} 65 - </div> 66 - </div> 20 + <script type="text/javascript"> 67 21 68 - {{if .AllowDisableOrEnableWorkflow}} 69 - <button class="ui jump dropdown btn interact-bg tw-p-2"> 70 - {{svg "octicon-kebab-horizontal"}} 71 - <div class="menu"> 72 - <a class="item link-action" data-url="{{$.Link}}/{{if .CurWorkflowDisabled}}enable{{else}}disable{{end}}?workflow={{$.CurWorkflow}}&actor={{.CurActor}}&status={{$.CurStatus}}"> 73 - {{if .CurWorkflowDisabled}}{{ctx.Locale.Tr "actions.workflow.enable"}}{{else}}{{ctx.Locale.Tr "actions.workflow.disable"}}{{end}} 74 - </a> 75 - </div> 76 - </button> 77 - {{end}} 78 - </div> 22 + function pollingOk() { 23 + return document.visibilityState === 'visible' && noActiveDropdowns(); 24 + } 79 25 80 - {{if $.CurWorkflowDispatch}} 81 - {{template "repo/actions/dispatch" .}} 82 - {{end}} 26 + // Intent: If the "Actor" or "Status" dropdowns are currently open and being navigated, or the workflow dispatch 27 + // dropdown form is open, the htmx refresh would replace them with closed dropdowns. Instead this prevents the list 28 + // refresh from occurring while those dropdowns are open. 29 + // 30 + // Can't inline this into the `hx-trigger` above because using a left-brace ('[') breaks htmx's trigger parsing. 31 + function noActiveDropdowns() { 32 + if (document.querySelector('[aria-expanded=true]') !== null) 33 + return false; 34 + const dropdownForm = document.querySelector('#branch-dropdown-form'); 35 + if (dropdownForm !== null && dropdownForm.checkVisibility()) 36 + return false; 37 + return true; 38 + } 39 + </script> 83 40 84 - {{template "repo/actions/runs_list" .}} 85 - </div> 86 - </div> 87 - {{else}} 88 - {{template "repo/actions/no_workflows" .}} 89 - {{end}} 90 - </div> 91 - </div> 92 41 {{template "base/footer" .}}
+85
templates/repo/actions/list_inner.tmpl
··· 1 + {{if .HasWorkflowsOrRuns}} 2 + <div class="ui stackable grid"> 3 + <div class="four wide column"> 4 + <div class="ui fluid vertical menu"> 5 + <a class="item{{if not $.CurWorkflow}} active{{end}}" href="?actor={{$.CurActor}}&status={{$.CurStatus}}">{{ctx.Locale.Tr "actions.runs.all_workflows"}}</a> 6 + {{range .workflows}} 7 + <a class="item{{if eq .Entry.Name $.CurWorkflow}} active{{end}}" href="?workflow={{.Entry.Name}}&actor={{$.CurActor}}&status={{$.CurStatus}}">{{.Entry.Name}} 8 + {{if .ErrMsg}} 9 + <span data-tooltip-content="{{.ErrMsg}}"> 10 + {{svg "octicon-alert" 16 "text red"}} 11 + </span> 12 + {{end}} 13 + 14 + {{if $.ActionsConfig.IsWorkflowDisabled .Entry.Name}} 15 + <div class="ui red label">{{ctx.Locale.Tr "disabled"}}</div> 16 + {{end}} 17 + </a> 18 + {{end}} 19 + </div> 20 + </div> 21 + <div class="twelve wide column content"> 22 + <div class="ui secondary filter menu tw-justify-end tw-flex tw-items-center"> 23 + <div id="reloading-indicator" class="htmx-indicator"></div> 24 + 25 + <!-- Actor --> 26 + <div id="actor_dropdown" class="ui{{if not .Actors}} disabled{{end}} dropdown jump item"> 27 + <span class="text">{{ctx.Locale.Tr "actions.runs.actor"}}</span> 28 + {{svg "octicon-triangle-down" 14 "dropdown icon"}} 29 + <div class="menu"> 30 + <div class="ui icon search input"> 31 + <i class="icon">{{svg "octicon-search"}}</i> 32 + <input type="text" placeholder="{{ctx.Locale.Tr "actions.runs.actor"}}"> 33 + </div> 34 + <a class="item{{if not $.CurActor}} active{{end}}" href="?workflow={{$.CurWorkflow}}&status={{$.CurStatus}}&actor=0"> 35 + {{ctx.Locale.Tr "actions.runs.actors_no_select"}} 36 + </a> 37 + {{range .Actors}} 38 + <a class="item{{if eq .ID $.CurActor}} active{{end}}" href="?workflow={{$.CurWorkflow}}&actor={{.ID}}&status={{$.CurStatus}}"> 39 + {{ctx.AvatarUtils.Avatar . 20}} {{.GetDisplayName}} 40 + </a> 41 + {{end}} 42 + </div> 43 + </div> 44 + <!-- Status --> 45 + <div id="status_dropdown" class="ui dropdown jump item"> 46 + <span class="text">{{ctx.Locale.Tr "actions.runs.status"}}</span> 47 + {{svg "octicon-triangle-down" 14 "dropdown icon"}} 48 + <div class="menu"> 49 + <div class="ui icon search input"> 50 + <i class="icon">{{svg "octicon-search"}}</i> 51 + <input type="text" placeholder="{{ctx.Locale.Tr "actions.runs.status"}}"> 52 + </div> 53 + <a class="item{{if not $.CurStatus}} active{{end}}" href="?workflow={{$.CurWorkflow}}&actor={{$.CurActor}}&status=0"> 54 + {{ctx.Locale.Tr "actions.runs.status_no_select"}} 55 + </a> 56 + {{range .StatusInfoList}} 57 + <a class="item{{if eq .Status $.CurStatus}} active{{end}}" href="?workflow={{$.CurWorkflow}}&actor={{$.CurActor}}&status={{.Status}}"> 58 + {{.DisplayedStatus}} 59 + </a> 60 + {{end}} 61 + </div> 62 + </div> 63 + 64 + {{if .AllowDisableOrEnableWorkflow}} 65 + <button class="ui jump dropdown btn interact-bg tw-p-2"> 66 + {{svg "octicon-kebab-horizontal"}} 67 + <div class="menu"> 68 + <a class="item link-action" data-url="{{$.Link}}/{{if .CurWorkflowDisabled}}enable{{else}}disable{{end}}?workflow={{$.CurWorkflow}}&actor={{.CurActor}}&status={{$.CurStatus}}"> 69 + {{if .CurWorkflowDisabled}}{{ctx.Locale.Tr "actions.workflow.enable"}}{{else}}{{ctx.Locale.Tr "actions.workflow.disable"}}{{end}} 70 + </a> 71 + </div> 72 + </button> 73 + {{end}} 74 + </div> 75 + 76 + {{if $.CurWorkflowDispatch}} 77 + {{template "repo/actions/dispatch" .}} 78 + {{end}} 79 + 80 + {{template "repo/actions/runs_list" .}} 81 + </div> 82 + </div> 83 + {{else}} 84 + {{template "repo/actions/no_workflows" .}} 85 + {{end}}
+145 -14
tests/e2e/actions.test.e2e.ts
··· 9 9 // routers/web/repo/actions/** 10 10 // @watch end 11 11 12 - import {expect} from '@playwright/test'; 12 + import {expect, type Page, type TestInfo} from '@playwright/test'; 13 13 import {save_visual, test} from './utils_e2e.ts'; 14 14 15 15 const workflow_trigger_notification_text = 'This workflow has a workflow_dispatch event trigger.'; 16 + 17 + async function dispatchSuccess(page: Page, testInfo: TestInfo) { 18 + test.skip(testInfo.project.name === 'Mobile Safari', 'Flaky behaviour on mobile safari; see https://codeberg.org/forgejo/forgejo/pulls/3334#issuecomment-2033383'); 19 + await page.goto('/user2/test_workflows/actions?workflow=test-dispatch.yml&actor=0&status=0'); 20 + 21 + await page.locator('#workflow_dispatch_dropdown>button').click(); 22 + 23 + await page.fill('input[name="inputs[string2]"]', 'abc'); 24 + await save_visual(page); 25 + await page.locator('#workflow-dispatch-submit').click(); 26 + 27 + await expect(page.getByText('Workflow run was successfully requested.')).toBeVisible(); 28 + 29 + await expect(page.locator('.run-list>:first-child .run-list-meta', {hasText: 'now'})).toBeVisible(); 30 + await save_visual(page); 31 + } 32 + 16 33 test.describe('Workflow Authenticated user2', () => { 17 34 test.use({user: 'user2'}); 18 35 ··· 50 67 await save_visual(page); 51 68 }); 52 69 70 + // no assertions as the login in this test case is extracted for reuse 71 + // eslint-disable-next-line playwright/expect-expect 53 72 test('dispatch success', async ({page}, testInfo) => { 54 - test.skip(testInfo.project.name === 'Mobile Safari', 'Flaky behaviour on mobile safari; see https://codeberg.org/forgejo/forgejo/pulls/3334#issuecomment-2033383'); 73 + await dispatchSuccess(page, testInfo); 74 + }); 75 + }); 76 + 77 + test('workflow dispatch box not available for unauthenticated users', async ({page}) => { 78 + await page.goto('/user2/test_workflows/actions?workflow=test-dispatch.yml&actor=0&status=0'); 79 + 80 + await expect(page.locator('body')).not.toContainText(workflow_trigger_notification_text); 81 + await save_visual(page); 82 + }); 83 + 84 + async function completeDynamicRefresh(page: Page) { 85 + // Ensure that the reloading indicator isn't active, indicating that dynamic refresh is done. 86 + await expect(page.locator('#reloading-indicator')).not.toHaveClass(/(^|\s)is-loading(\s|$)/); 87 + } 88 + 89 + async function simulatePollingInterval(page: Page) { 90 + // In order to simulate the background page sitting around for > 30s, a custom event `simulate-polling-interval` is 91 + // fired into the document to mimic the polling interval expiring -- although this isn't a perfectly great E2E test 92 + // with this kind of mimicry, it's better than having multiple >30s execution-time tests. 93 + await page.evaluate(() => { 94 + document.dispatchEvent(new Event('simulate-polling-interval')); 95 + }); 96 + await completeDynamicRefresh(page); 97 + } 98 + 99 + test.describe('workflow list dynamic refresh', () => { 100 + test.use({user: 'user2'}); 101 + 102 + test('refreshes on visibility change', async ({page}, testInfo) => { 103 + // Test operates by creating two pages; one which is sitting idle on the workflows list (backgroundPage), and one 104 + // which triggers a workflow dispatch. Then a document visibilitychange event is fired on the background page to 105 + // mimic a user returning to the tab on their browser, which should trigger the workflow list to refresh and display 106 + // the newly dispatched workflow from the other page. 107 + 108 + const backgroundPage = await page.context().newPage(); 109 + await backgroundPage.goto('/user2/test_workflows/actions?workflow=test-dispatch.yml&actor=0&status=0'); 110 + 111 + // Mirror the `Workflow Authenticated user2 > dispatch success` test: 112 + await dispatchSuccess(page, testInfo); 113 + const latestDispatchedRun = await page.locator('.run-list>:first-child .flex-item-body>b').textContent(); 114 + expect(latestDispatchedRun).toMatch(/^#/); // workflow ID, eg. "#53" 115 + 116 + // Synthetically trigger a visibilitychange event, as if we were returning to backgroundPage: 117 + await backgroundPage.evaluate(() => { 118 + document.dispatchEvent(new Event('visibilitychange')); 119 + }); 120 + await completeDynamicRefresh(page); 121 + await expect(backgroundPage.locator('.run-list>:first-child .flex-item-body>b', {hasText: latestDispatchedRun})).toBeVisible(); 122 + await save_visual(backgroundPage); 123 + }); 124 + 125 + test('refreshes on interval', async ({page}, testInfo) => { 126 + // Test operates by creating two pages; one which is sitting idle on the workflows list (backgroundPage), and one 127 + // which triggers a workflow dispatch. After the polling, the page should refresh and show the newly dispatched 128 + // workflow from the other page. 129 + 130 + const backgroundPage = await page.context().newPage(); 131 + await backgroundPage.goto('/user2/test_workflows/actions?workflow=test-dispatch.yml&actor=0&status=0'); 132 + 133 + // Mirror the `Workflow Authenticated user2 > dispatch success` test: 134 + await dispatchSuccess(page, testInfo); 135 + const latestDispatchedRun = await page.locator('.run-list>:first-child .flex-item-body>b').textContent(); 136 + expect(latestDispatchedRun).toMatch(/^#/); // workflow ID, eg. "#53" 137 + 138 + await simulatePollingInterval(backgroundPage); 139 + await expect(backgroundPage.locator('.run-list>:first-child .flex-item-body>b', {hasText: latestDispatchedRun})).toBeVisible(); 140 + await save_visual(backgroundPage); 141 + }); 142 + 143 + test('post-refresh the dropdowns continue to operate', async ({page}, testInfo) => { 144 + // Verify that after the page is dynamically refreshed, the 'Actor', 'Status', and 'Run workflow' dropdowns work 145 + // correctly -- that the htmx morph hasn't messed up any JS event handlers. 55 146 await page.goto('/user2/test_workflows/actions?workflow=test-dispatch.yml&actor=0&status=0'); 56 147 148 + // Mirror the `Workflow Authenticated user2 > dispatch success` test -- this creates data for the 'Actor' dropdown 149 + await dispatchSuccess(page, testInfo); 150 + 151 + // Perform a dynamic refresh before checking the functionality of each dropdown. 152 + await simulatePollingInterval(page); 153 + 154 + // Workflow run dialog 155 + await expect(page.locator('input[name="inputs[string2]"]')).toBeHidden(); 156 + await page.locator('#workflow_dispatch_dropdown>button').click(); 157 + await expect(page.locator('input[name="inputs[string2]"]')).toBeVisible(); 57 158 await page.locator('#workflow_dispatch_dropdown>button').click(); 58 159 59 - await page.fill('input[name="inputs[string2]"]', 'abc'); 60 - await save_visual(page); 61 - await page.locator('#workflow-dispatch-submit').click(); 160 + // Status dropdown 161 + await expect(page.getByText('Waiting')).toBeHidden(); 162 + await expect(page.getByText('Failure')).toBeHidden(); 163 + await page.locator('#status_dropdown').click(); 164 + await expect(page.getByText('Waiting')).toBeVisible(); 165 + await expect(page.getByText('Failure')).toBeVisible(); 166 + 167 + // Actor dropdown 168 + await expect(page.getByText('All actors')).toBeHidden(); 169 + await page.locator('#actor_dropdown').click(); 170 + await expect(page.getByText('All Actors')).toBeVisible(); 171 + }); 172 + 173 + test('refresh does not break interacting with open drop-downs', async ({page}, testInfo) => { 174 + // Verify that if the polling refresh occurs while interacting with any multi-step dropdown on the page, the 175 + // multi-step interaction continues to be visible and functional. This is implemented by preventing the refresh, 176 + // but that isn't the subject of the test here -- as long as the dropdown isn't broken by the refresh, that's fine. 177 + await page.goto('/user2/test_workflows/actions?workflow=test-dispatch.yml&actor=0&status=0'); 62 178 63 - await expect(page.getByText('Workflow run was successfully requested.')).toBeVisible(); 179 + // Mirror the `Workflow Authenticated user2 > dispatch success` test -- this creates data for the 'Actor' dropdown 180 + await dispatchSuccess(page, testInfo); 64 181 65 - await expect(page.locator('.run-list>:first-child .run-list-meta', {hasText: 'now'})).toBeVisible(); 66 - await save_visual(page); 67 - }); 68 - }); 182 + // Workflow run dialog 183 + await expect(page.locator('input[name="inputs[string2]"]')).toBeHidden(); 184 + await page.locator('#workflow_dispatch_dropdown>button').click(); 185 + await expect(page.locator('input[name="inputs[string2]"]')).toBeVisible(); 186 + await simulatePollingInterval(page); 187 + await expect(page.locator('input[name="inputs[string2]"]')).toBeVisible(); 69 188 70 - test('workflow dispatch box not available for unauthenticated users', async ({page}) => { 71 - await page.goto('/user2/test_workflows/actions?workflow=test-dispatch.yml&actor=0&status=0'); 189 + // Status dropdown 190 + await expect(page.getByText('Waiting')).toBeHidden(); 191 + await expect(page.getByText('Failure')).toBeHidden(); 192 + await page.locator('#status_dropdown').click(); 193 + await expect(page.getByText('Waiting')).toBeVisible(); 194 + await expect(page.getByText('Failure')).toBeVisible(); 195 + await simulatePollingInterval(page); 196 + await expect(page.getByText('Waiting')).toBeVisible(); 197 + await expect(page.getByText('Failure')).toBeVisible(); 72 198 73 - await expect(page.locator('body')).not.toContainText(workflow_trigger_notification_text); 74 - await save_visual(page); 199 + // Actor dropdown 200 + await expect(page.getByText('All actors')).toBeHidden(); 201 + await page.locator('#actor_dropdown').click(); 202 + await expect(page.getByText('All Actors')).toBeVisible(); 203 + await simulatePollingInterval(page); 204 + await expect(page.getByText('All Actors')).toBeVisible(); 205 + }); 75 206 });