github-personal/firezone
1
0
Fork 0
mirror of https://github.com/outbackdingo/firezone.git synced 2026-01-27 18:18:55 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix(portal): Show reload button when table data is stale (#8143)

Browse Source 
Sentry uncovered a bug in the resources index liveview where it looks
like some code copy-pasted from the policies index view wasn't updated
properly to work in the resources live view, causing the view to crash
if an admin was viewing the table while the resources are changed in
another page.

In debugging that, I realized the best UX when viewing these tables is
usually just to show a `Reload` button and not update the data live
while the admin is viewing it, as this can cause missed clicks and other
annoyances.

This PR adds an optional `stale` component attribute that, if true, will
render a `Reload` button in the live table which upon clicking will
reload the live table.

Not all index views are updated with this - in some views there is
already logic to handle making an intelligent update without breaking
the view if the data is updated - for example for the clients table.

Ideally, we live-update things that don't reflow layout inline (such as
`online/offline` presence) and for things that do cause layout reflow
(create/delete), we show the `Reload` button.

However that work is saved for a future PR as this one fixes the
immediate bug and this is not the highest priority.

<img width="1195" alt="Screenshot 2025-02-16 at 8 44 43 AM"
src="https://github.com/user-attachments/assets/114efffa-85ea-490d-9cea-78c607081ce3"
/>
<img width="401" alt="Screenshot 2025-02-16 at 9 59 53 AM"
src="https://github.com/user-attachments/assets/8a570213-d4ec-4b6c-a489-dcd9ad1c351c"
/>
This commit is contained in:
Jamil
2025-02-24 07:39:16 -08:00
committed by GitHub
parent 29f0ac0a00
commit c5929d4063
5 changed files with 137 additions and 43 deletions
Download Patch File Download Diff File Expand all files Collapse all files

27
elixir/apps/web/lib/web/live/policies/index.ex
View File

@@ -9,6 +9,7 @@ defmodule Web.Policies.Index do
socket =
socket
|> assign(stale: false)
|> assign(page_title: "Policies")
|> assign_live_table("policies",
query_module: Policies.Policy.Query,
@@ -64,6 +65,7 @@ defmodule Web.Policies.Index do
<:content>
<.flash_group flash={@flash} />
<.live_table
stale={@stale}
id="policies"
rows={@policies}
row_id={&"policies-#{&1.id}"}
@@ -117,28 +119,11 @@ defmodule Web.Policies.Index do
"""
end
def handle_event(event, params, socket) when event in ["paginate", "order_by", "filter"],
do: handle_live_table_event(event, params, socket)
def handle_info({:create_policy, _policy_id}, socket) do
{:noreply, socket}
end
def handle_info({:delete_policy, policy_id}, socket) do
if Enum.find(socket.assigns.policies, fn policy -> policy.id == policy_id end) do
policies =
Enum.filter(socket.assigns.policies, fn
policy -> policy.id != policy_id
end)
{:noreply, assign(socket, policies: policies)}
else
{:noreply, socket}
end
end
def handle_event(event, params, socket)
when event in ["paginate", "order_by", "filter", "reload"],
do: handle_live_table_event(event, params, socket)
def handle_info({_action, _policy_id}, socket) do
socket = reload_live_table!(socket, "policies")
{:noreply, socket}
{:noreply, assign(socket, stale: true)}
end
end

26
elixir/apps/web/lib/web/live/resources/index.ex
View File

@@ -15,6 +15,7 @@ defmodule Web.Resources.Index do
socket =
socket
|> assign(stale: false)
|> assign(page_title: "Resources")
|> assign(internet_site: internet_site)
|> assign_live_table("resources",
@@ -87,6 +88,7 @@ defmodule Web.Resources.Index do
<:content>
<.flash_group flash={@flash} />
<.live_table
stale={@stale}
id="resources"
rows={@resources}
row_id={&"resource-#{&1.id}"}
@@ -161,27 +163,11 @@ defmodule Web.Resources.Index do
"""
end
def handle_event(event, params, socket) when event in ["paginate", "order_by", "filter"],
do: handle_live_table_event(event, params, socket)
def handle_info({:create_resource, _resource_id}, socket) do
{:noreply, socket}
end
def handle_info({:delete_resource, resource_id}, socket) do
if Enum.find(socket.assigns.policies, fn resource -> resource.id == resource_id end) do
policies =
Enum.filter(socket.assigns.policies, fn resource ->
resource.id != resource_id
end)
{:noreply, assign(socket, policies: policies)}
else
{:noreply, socket}
end
end
def handle_event(event, params, socket)
when event in ["paginate", "order_by", "filter", "reload"],
do: handle_live_table_event(event, params, socket)
def handle_info({_action, _resource_id}, socket) do
{:noreply, reload_live_table!(socket, "resources")}
{:noreply, assign(socket, stale: true)}
end
end

25
elixir/apps/web/lib/web/live_table.ex
View File

@@ -15,6 +15,7 @@ defmodule Web.LiveTable do
attr :ordered_by, :any, required: true, doc: "the current order for the table"
attr :filters, :list, required: true, doc: "the query filters enabled for the table"
attr :filter, :map, required: true, doc: "the filter form for the table"
attr :stale, :boolean, default: false, doc: "hint to the UI that the table data is stale"
attr :metadata, :map,
required: true,
@@ -40,7 +41,7 @@ defmodule Web.LiveTable do
def live_table(assigns) do
~H"""
<.resource_filter live_table_id={@id} form={@filter} filters={@filters} />
<.resource_filter stale={@stale} live_table_id={@id} form={@filter} filters={@filters} />
<div class="overflow-x-auto">
<table class={["w-full text-sm text-left text-neutral-500 table-fixed"]} id={@id}>
<.table_header table_id={@id} columns={@col} actions={@action} ordered_by={@ordered_by} />
@@ -152,7 +153,21 @@ defmodule Web.LiveTable do
>
<.input type="hidden" name="table_id" value={@live_table_id} />
<div class="mb-2 space-y-2 md:space-y-0 md:gap-2 md:flex md:justify-end">
<div class="mb-2 space-y-2 md:space-y-0 md:gap-2 md:flex md:justify-between">
<div>
<.button
:if={@stale}
id={"#{@live_table_id}-reload-btn"}
type="button"
style="info"
title="The table data has changed."
phx-click="reload"
phx-value-table_id={@live_table_id}
>
<.icon name="hero-arrow-path" class="mr-1 w-3.5 h-3.5" /> Reload
</.button>
</div>
<.filter
:for={filter <- @filters}
live_table_id={@live_table_id}
@@ -536,6 +551,8 @@ defmodule Web.LiveTable do
callback = Map.fetch!(socket.assigns.callback_by_table_id, id)
list_opts = Map.get(socket.assigns[:list_opts_by_table_id] || %{}, id, [])
socket = assign(socket, stale: false)
case callback.(socket, list_opts) do
{:error, _reason} ->
uri = URI.parse(socket.assigns.uri)
@@ -776,6 +793,10 @@ defmodule Web.LiveTable do
end
end
def handle_live_table_event("reload", %{"table_id" => id}, socket) do
{:noreply, reload_live_table!(socket, id)}
end
def handle_live_table_event("paginate", %{"table_id" => id, "cursor" => cursor}, socket) do
update_query_params(socket, fn query_params ->
put_cursor_to_params(query_params, id, cursor)

51
elixir/apps/web/test/web/live/policies/index_test.exs
View File

@@ -77,4 +77,55 @@ defmodule Web.Live.Policies.IndexTest do
assert rendered_policy["group"] =~ policy.actor_group.name
assert rendered_policy["resource"] =~ policy.resource.name
end
describe "handle_info/2" do
test "Shows reload button when policy is created", %{
account: account,
identity: identity,
conn: conn
} do
{:ok, lv, html} =
conn
|> authorize_conn(identity)
|> live(~p"/#{account}/policies")
refute html =~ "The table data has changed."
refute html =~ "reload-btn"
Fixtures.Policies.create_policy(account: account, description: "foo bar")
reload_btn =
lv
|> element("#policies-reload-btn")
|> render()
assert reload_btn
end
test "Shows reload button when policy is deleted", %{
account: account,
identity: identity,
conn: conn
} do
policy = Fixtures.Policies.create_policy(account: account, description: "foo bar")
subject = Fixtures.Auth.create_subject(identity: identity)
{:ok, lv, html} =
conn
|> authorize_conn(identity)
|> live(~p"/#{account}/policies")
refute html =~ "The table data has changed."
refute html =~ "reload-btn"
Domain.Policies.delete_policy(policy, subject)
reload_btn =
lv
|> element("#policies-reload-btn")
|> render()
assert reload_btn
end
end
end

51
elixir/apps/web/test/web/live/resources/index_test.exs
View File

@@ -205,4 +205,55 @@ defmodule Web.Live.Resources.IndexTest do
assert row["authorized groups"] =~ "and 1 more"
end)
end
describe "handle_info/2" do
test "Shows reload button when resource is created", %{
account: account,
identity: identity,
conn: conn
} do
{:ok, lv, html} =
conn
|> authorize_conn(identity)
|> live(~p"/#{account}/resources")
refute html =~ "The table data has changed."
refute html =~ "reload-btn"
Fixtures.Resources.create_resource(account: account)
reload_btn =
lv
|> element("#resources-reload-btn")
|> render()
assert reload_btn
end
test "Shows reload button when resource is deleted", %{
account: account,
identity: identity,
conn: conn
} do
resource = Fixtures.Resources.create_resource(account: account)
subject = Fixtures.Auth.create_subject(identity: identity)
{:ok, lv, html} =
conn
|> authorize_conn(identity)
|> live(~p"/#{account}/resources")
refute html =~ "The table data has changed."
refute html =~ "reload-btn"
Domain.Resources.delete_resource(resource, subject)
reload_btn =
lv
|> element("#resources-reload-btn")
|> render()
assert reload_btn
end
end
end