diff --git a/.artifacts/adr-1-use-screen-behaviour-for-github-prs.md b/.artifacts/adr-1-use-screen-behaviour-for-github-prs.md new file mode 100644 index 0000000..8d0273a --- /dev/null +++ b/.artifacts/adr-1-use-screen-behaviour-for-github-prs.md @@ -0,0 +1,39 @@ +# ADR: Use Screen Behaviour Pattern for GitHub PRs Screen + +## Status + +Proposed + +## Context + +The ticket requests implementing a GitHub Assigned PRs screen for the TRMNL playlist. The ticket description suggests creating a new Phoenix controller (`TrmnlWeb.GithubPrsController`) and a separate template file. However, the existing codebase already has a well-defined pattern for screens: + +- Modules implement the `Trmnl.Screen` behaviour with a `render/1` callback +- `Trmnl.Playlist` maintains a list of screen modules +- `TrmnlWeb.ScreenController` delegates rendering to the current screen module via `ScreenHTML` +- The existing `Trmnl.Screens.HelloWorld` demonstrates this pattern using `Phoenix.Component` and HEEx + +Data is fetched via the `gh` CLI tool (`System.cmd/3`) to leverage existing system authentication, avoiding manual GitHub token management. + +## Decision + +1. **Follow the existing screen module pattern** rather than creating a new controller/template. The new screen will be `Trmnl.Screens.GithubPrs`, implementing the `Trmnl.Screen` behaviour with an inline HEEx template via `Phoenix.Component`. + +2. **Use `System.cmd/3` with the `gh` CLI** for data fetching, as specified in the ticket. The function will be extracted as a public `fetch_prs/0` for testability. + +3. **Implement a simple `time_ago/1` function** using `DateTime.diff/2` rather than adding a dependency like Timex, since the requirement is straightforward (seconds/minutes/hours/days/weeks/months ago). + +4. **Make the CLI runner injectable** by accepting an optional function parameter, enabling unit tests to mock the CLI output without actually calling `gh`. + +## Consequences + +### Positive +- Consistent with existing codebase patterns — no new routing or controller plumbing needed +- Simple, self-contained module that is easy to test and maintain +- No new dependencies required +- CLI-based auth avoids token management complexity + +### Negative +- Depends on `gh` CLI being installed and authenticated on the host system +- `System.cmd/3` is a blocking call during render — acceptable for the TRMNL use case where screens are pre-rendered to BMP +- The `time_ago/1` function is custom code that must be maintained, though it is simple enough to not warrant a library dependency diff --git a/.gitignore b/.gitignore index 003d884..ae95a97 100644 --- a/.gitignore +++ b/.gitignore @@ -40,4 +40,8 @@ npm-debug.log *.db *.db-* -.DS_Store \ No newline at end of file +.DS_Store +# UdinCode planning artifacts +.artifacts/* +!.artifacts/adr-*.md +.claude/ diff --git a/lib/trmnl/inventory/device.ex b/lib/trmnl/inventory/device.ex index dd9ca6e..d0a984d 100644 --- a/lib/trmnl/inventory/device.ex +++ b/lib/trmnl/inventory/device.ex @@ -50,7 +50,10 @@ defmodule Trmnl.Inventory.Device do defp upcase(changeset, fields) do Enum.reduce(fields, changeset, fn field, changeset -> - update_change(changeset, field, &String.upcase/1) + update_change(changeset, field, fn + nil -> nil + val -> String.upcase(val) + end) end) end end diff --git a/lib/trmnl/playlist.ex b/lib/trmnl/playlist.ex index 64db5c7..e377461 100644 --- a/lib/trmnl/playlist.ex +++ b/lib/trmnl/playlist.ex @@ -9,7 +9,8 @@ defmodule Trmnl.Playlist do def screens(_device) do # The device argument could be used to customize the playlist [ - Trmnl.Screens.HelloWorld + Trmnl.Screens.HelloWorld, + Trmnl.Screens.GithubPrs ] end diff --git a/lib/trmnl/screen.ex b/lib/trmnl/screen.ex index 70c6ff3..a82a219 100644 --- a/lib/trmnl/screen.ex +++ b/lib/trmnl/screen.ex @@ -26,13 +26,18 @@ defmodule Trmnl.Screen do def regenerate(device) do # --- Render the screen --- Logger.debug("Generating screen for device #{device.id}...") - screen_path = render_current_bmp(device) - # --- Update the device --- - Inventory.update_device(device, %{ - latest_screen: screen_path, - screen_generated_at: DateTime.utc_now() - }) + case render_current_bmp(device) do + {:ok, screen_path} -> + Inventory.update_device(device, %{ + latest_screen: screen_path, + screen_generated_at: DateTime.utc_now() + }) + + {:error, reason} -> + Logger.warning("Failed to generate screen for device #{device.id}: #{reason}") + {:error, reason} + end end # Hits the /screens/:api_key endpoint to get the current screen, then converts it to a BMP @@ -46,37 +51,43 @@ defmodule Trmnl.Screen do File.mkdir_p!(static_dir) - # --- Generate screenshot --- - System.cmd("puppeteer-img", [ - "--width", - to_string(@width), - "--height", - to_string(@height), - "--path", - screenshot_path, - current_screen_url - ]) - - # --- Convert screenshot to BMP --- - System.cmd("magick", [ - screenshot_path, - "-resize", - "#{@width}x#{@height}", - "-dither", - "FloydSteinberg", - "-remap", - "pattern:gray50", - "-depth", - "#{@color_depth}", - "-strip", - "-monochrome", - "bmp3:#{static_path}" - ]) - - # --- Remove temporary files --- - File.rm!(screenshot_path) - - # --- Return the path to the generated BMP --- - "/generated/#{basename}.bmp" + try do + # --- Generate screenshot --- + System.cmd("puppeteer-img", [ + "--width", + to_string(@width), + "--height", + to_string(@height), + "--path", + screenshot_path, + current_screen_url + ]) + + # --- Convert screenshot to BMP --- + System.cmd("magick", [ + screenshot_path, + "-resize", + "#{@width}x#{@height}", + "-dither", + "FloydSteinberg", + "-remap", + "pattern:gray50", + "-depth", + "#{@color_depth}", + "-strip", + "-monochrome", + "bmp3:#{static_path}" + ]) + + # --- Remove temporary files --- + File.rm!(screenshot_path) + + # --- Return the path to the generated BMP --- + {:ok, "/generated/#{basename}.bmp"} + rescue + e -> + File.rm(screenshot_path) + {:error, Exception.message(e)} + end end end diff --git a/lib/trmnl/screens/github_prs.ex b/lib/trmnl/screens/github_prs.ex new file mode 100644 index 0000000..48f2bfd --- /dev/null +++ b/lib/trmnl/screens/github_prs.ex @@ -0,0 +1,122 @@ +defmodule Trmnl.Screens.GithubPrs do + @behaviour Trmnl.Screen + + use Phoenix.Component + + @assignee "bars0udin" + @fetch_timeout_ms 10_000 + + def render(assigns) do + prs = fetch_prs() + + assigns = + assigns + |> Map.put(:count, length(prs)) + |> Map.put( + :prs, + Enum.map(prs, fn pr -> + ago = + case DateTime.from_iso8601(pr["createdAt"] || "") do + {:ok, created_at, _offset} -> time_ago(created_at) + {:error, _} -> "unknown" + end + + Map.put(pr, "ago", ago) + end) + ) + + ~H""" +
+
+

GitHub PRs

+ + {@count} + +
+ + <%= if @count == 0 do %> +
+ No open PRs assigned +
+ <% else %> +
+
+ <%= for pr <- @prs do %> +
+ + #{pr["number"]} {pr["title"]} + + + {pr["ago"]} + +
+ <% end %> +
+
+ <% end %> +
+ """ + end + + def fetch_prs(runner \\ &default_runner/2) do + case runner.("gh", [ + "pr", + "list", + "--assignee=#{@assignee}", + "--state=open", + "--json", + "number,title,url,createdAt" + ]) do + {json, 0} -> parse_prs(json) + _ -> [] + end + end + + defp default_runner(cmd, args) do + task = Task.async(fn -> System.cmd(cmd, args, stderr_to_stdout: true) end) + + case Task.yield(task, @fetch_timeout_ms) || Task.shutdown(task) do + {:ok, result} -> result + nil -> {"timeout", 1} + end + end + + def parse_prs(json) do + case Jason.decode(json) do + {:ok, prs} when is_list(prs) -> prs + _ -> [] + end + end + + def time_ago(datetime) do + diff = DateTime.diff(DateTime.utc_now(), datetime, :second) + + cond do + diff < 0 -> + "just now" + + diff < 60 -> + "#{diff} seconds ago" + + diff < 3600 -> + minutes = div(diff, 60) + if minutes == 1, do: "1 minute ago", else: "#{minutes} minutes ago" + + diff < 86400 -> + hours = div(diff, 3600) + if hours == 1, do: "1 hour ago", else: "#{hours} hours ago" + + diff < 86400 * 7 -> + days = div(diff, 86400) + if days == 1, do: "1 day ago", else: "#{days} days ago" + + diff < 86400 * 30 -> + weeks = div(diff, 86400 * 7) + if weeks == 1, do: "1 week ago", else: "#{weeks} weeks ago" + + true -> + months = div(diff, 86400 * 30) + if months == 1, do: "1 month ago", else: "#{months} months ago" + end + end +end diff --git a/lib/trmnl_web/components/core_components.ex b/lib/trmnl_web/components/core_components.ex index cb415cc..6667c01 100644 --- a/lib/trmnl_web/components/core_components.ex +++ b/lib/trmnl_web/components/core_components.ex @@ -156,8 +156,7 @@ defmodule TrmnlWeb.CoreComponents do phx-connected={hide("#client-error")} hidden > - Attempting to reconnect - <.icon name="hero-arrow-path" class="ml-1 h-3 w-3 animate-spin" /> + Attempting to reconnect <.icon name="hero-arrow-path" class="ml-1 h-3 w-3 animate-spin" /> <.flash diff --git a/mix.lock b/mix.lock index c6c9d62..25770b4 100644 --- a/mix.lock +++ b/mix.lock @@ -16,7 +16,7 @@ "file_system": {:hex, :file_system, "1.1.0", "08d232062284546c6c34426997dd7ef6ec9f8bbd090eb91780283c9016840e8f", [:mix], [], "hexpm", "bfcf81244f416871f2a2e15c1b515287faa5db9c6bcf290222206d120b3d43f6"}, "floki": {:hex, :floki, "0.37.0", "b83e0280bbc6372f2a403b2848013650b16640cd2470aea6701f0632223d719e", [:mix], [], "hexpm", "516a0c15a69f78c47dc8e0b9b3724b29608aa6619379f91b1ffa47109b5d0dd3"}, "hackney": {:hex, :hackney, "1.20.1", "8d97aec62ddddd757d128bfd1df6c5861093419f8f7a4223823537bad5d064e2", [:rebar3], [{:certifi, "~> 2.12.0", [hex: :certifi, repo: "hexpm", optional: false]}, {:idna, "~> 6.1.0", [hex: :idna, repo: "hexpm", optional: false]}, {:metrics, "~> 1.0.0", [hex: :metrics, repo: "hexpm", optional: false]}, {:mimerl, "~> 1.1", [hex: :mimerl, repo: "hexpm", optional: false]}, {:parse_trans, "3.4.1", [hex: :parse_trans, repo: "hexpm", optional: false]}, {:ssl_verify_fun, "~> 1.1.0", [hex: :ssl_verify_fun, repo: "hexpm", optional: false]}, {:unicode_util_compat, "~> 0.7.0", [hex: :unicode_util_compat, repo: "hexpm", optional: false]}], "hexpm", "fe9094e5f1a2a2c0a7d10918fee36bfec0ec2a979994cff8cfe8058cd9af38e3"}, - "heroicons": {:git, "https://github.com/tailwindlabs/heroicons.git", "88ab3a0d790e6a47404cba02800a6b25d2afae50", [tag: "v2.1.1", sparse: "optimized"]}, + "heroicons": {:git, "https://github.com/tailwindlabs/heroicons.git", "88ab3a0d790e6a47404cba02800a6b25d2afae50", [tag: "v2.1.1", sparse: "optimized", depth: 1]}, "hpax": {:hex, :hpax, "1.0.2", "762df951b0c399ff67cc57c3995ec3cf46d696e41f0bba17da0518d94acd4aac", [:mix], [], "hexpm", "2f09b4c1074e0abd846747329eaa26d535be0eb3d189fa69d812bfb8bfefd32f"}, "httpoison": {:hex, :httpoison, "2.2.1", "87b7ed6d95db0389f7df02779644171d7319d319178f6680438167d7b69b1f3d", [:mix], [{:hackney, "~> 1.17", [hex: :hackney, repo: "hexpm", optional: false]}], "hexpm", "51364e6d2f429d80e14fe4b5f8e39719cacd03eb3f9a9286e61e216feac2d2df"}, "idna": {:hex, :idna, "6.1.1", "8a63070e9f7d0c62eb9d9fcb360a7de382448200fbbd1b106cc96d3d8099df8d", [:rebar3], [{:unicode_util_compat, "~> 0.7.0", [hex: :unicode_util_compat, repo: "hexpm", optional: false]}], "hexpm", "92376eb7894412ed19ac475e4a86f7b413c1b9fbb5bd16dccd57934157944cea"}, diff --git a/test/support/fixtures/inventory_fixtures.ex b/test/support/fixtures/inventory_fixtures.ex index 09d54b4..edb1385 100644 --- a/test/support/fixtures/inventory_fixtures.ex +++ b/test/support/fixtures/inventory_fixtures.ex @@ -12,7 +12,16 @@ defmodule Trmnl.InventoryFixtures do @doc """ Generate a unique device mac_address. """ - def unique_device_mac_address, do: "some mac_address#{System.unique_integer([:positive])}" + def unique_device_mac_address do + n = System.unique_integer([:positive]) + # Generate a valid MAC address in AA:BB:CC:DD:EE:FF format + :io_lib.format("02:00:00:~2.16.0B:~2.16.0B:~2.16.0B", [ + rem(n, 256), + rem(div(n, 256), 256), + rem(div(n, 65536), 256) + ]) + |> IO.iodata_to_binary() + end @doc """ Generate a device. diff --git a/test/trmnl/inventory_test.exs b/test/trmnl/inventory_test.exs index 8490a4c..73f8419 100644 --- a/test/trmnl/inventory_test.exs +++ b/test/trmnl/inventory_test.exs @@ -8,7 +8,13 @@ defmodule Trmnl.InventoryTest do import Trmnl.InventoryFixtures - @invalid_attrs %{name: nil, api_key: nil, mac_address: nil, friendly_id: nil, refresh_interval: nil} + @invalid_attrs %{ + name: nil, + api_key: nil, + mac_address: nil, + friendly_id: nil, + refresh_interval: nil + } test "list_devices/0 returns all devices" do device = device_fixture() @@ -21,13 +27,19 @@ defmodule Trmnl.InventoryTest do end test "create_device/1 with valid data creates a device" do - valid_attrs = %{name: "some name", api_key: "some api_key", mac_address: "some mac_address", friendly_id: "some friendly_id", refresh_interval: 42} + valid_attrs = %{ + name: "some name", + api_key: "some api_key", + mac_address: "AA:BB:CC:DD:EE:FF", + friendly_id: "some friendly_id", + refresh_interval: 42 + } assert {:ok, %Device{} = device} = Inventory.create_device(valid_attrs) assert device.name == "some name" assert device.api_key == "some api_key" - assert device.mac_address == "some mac_address" - assert device.friendly_id == "some friendly_id" + assert device.mac_address == "AA:BB:CC:DD:EE:FF" + assert device.friendly_id == "SOME FRIENDLY_ID" assert device.refresh_interval == 42 end @@ -37,13 +49,20 @@ defmodule Trmnl.InventoryTest do test "update_device/2 with valid data updates the device" do device = device_fixture() - update_attrs = %{name: "some updated name", api_key: "some updated api_key", mac_address: "some updated mac_address", friendly_id: "some updated friendly_id", refresh_interval: 43} + + update_attrs = %{ + name: "some updated name", + api_key: "some updated api_key", + mac_address: "11:22:33:44:55:66", + friendly_id: "some updated friendly_id", + refresh_interval: 43 + } assert {:ok, %Device{} = device} = Inventory.update_device(device, update_attrs) assert device.name == "some updated name" assert device.api_key == "some updated api_key" - assert device.mac_address == "some updated mac_address" - assert device.friendly_id == "some updated friendly_id" + assert device.mac_address == "11:22:33:44:55:66" + assert device.friendly_id == "SOME UPDATED FRIENDLY_ID" assert device.refresh_interval == 43 end diff --git a/test/trmnl/screens/github_prs_test.exs b/test/trmnl/screens/github_prs_test.exs new file mode 100644 index 0000000..84bab8a --- /dev/null +++ b/test/trmnl/screens/github_prs_test.exs @@ -0,0 +1,98 @@ +defmodule Trmnl.Screens.GithubPrsTest do + use ExUnit.Case, async: true + + alias Trmnl.Screens.GithubPrs + + describe "parse_prs/1" do + test "parses JSON output from gh CLI command" do + json = """ + [ + {"number": 42, "title": "Add feature X", "url": "https://github.com/org/repo/pull/42", "createdAt": "2026-04-06T10:00:00Z"}, + {"number": 7, "title": "Fix bug Y", "url": "https://github.com/org/repo/pull/7", "createdAt": "2026-04-05T08:30:00Z"} + ] + """ + + prs = GithubPrs.parse_prs(json) + + assert length(prs) == 2 + assert %{"number" => 42, "title" => "Add feature X"} = Enum.at(prs, 0) + assert %{"number" => 7, "title" => "Fix bug Y"} = Enum.at(prs, 1) + end + + test "handles empty PR list response gracefully" do + assert GithubPrs.parse_prs("[]") == [] + end + + test "handles invalid JSON gracefully" do + assert GithubPrs.parse_prs("not json") == [] + end + end + + describe "time_ago/1" do + test "returns seconds ago for recent timestamps" do + now = DateTime.utc_now() + thirty_sec_ago = DateTime.add(now, -30, :second) + assert GithubPrs.time_ago(thirty_sec_ago) == "30 seconds ago" + end + + test "returns minutes ago" do + now = DateTime.utc_now() + five_min_ago = DateTime.add(now, -5 * 60, :second) + assert GithubPrs.time_ago(five_min_ago) == "5 minutes ago" + end + + test "returns singular minute" do + now = DateTime.utc_now() + one_min_ago = DateTime.add(now, -90, :second) + assert GithubPrs.time_ago(one_min_ago) == "1 minute ago" + end + + test "returns hours ago" do + now = DateTime.utc_now() + three_hours_ago = DateTime.add(now, -3 * 3600, :second) + assert GithubPrs.time_ago(three_hours_ago) == "3 hours ago" + end + + test "returns days ago" do + now = DateTime.utc_now() + two_days_ago = DateTime.add(now, -2 * 86400, :second) + assert GithubPrs.time_ago(two_days_ago) == "2 days ago" + end + + test "returns weeks ago" do + now = DateTime.utc_now() + two_weeks_ago = DateTime.add(now, -14 * 86400, :second) + assert GithubPrs.time_ago(two_weeks_ago) == "2 weeks ago" + end + + test "returns months ago" do + now = DateTime.utc_now() + sixty_days_ago = DateTime.add(now, -60 * 86400, :second) + assert GithubPrs.time_ago(sixty_days_ago) == "2 months ago" + end + + test "returns 'just now' for future datetimes" do + future = DateTime.add(DateTime.utc_now(), 60, :second) + assert GithubPrs.time_ago(future) == "just now" + end + end + + describe "fetch_prs/1" do + test "returns parsed PRs on successful CLI call" do + json = + ~s([{"number":1,"title":"Test PR","url":"https://example.com","createdAt":"2026-04-06T00:00:00Z"}]) + + runner = fn _cmd, _args -> {json, 0} end + + prs = GithubPrs.fetch_prs(runner) + assert length(prs) == 1 + assert %{"title" => "Test PR"} = hd(prs) + end + + test "returns empty list on CLI failure" do + runner = fn _cmd, _args -> {"error", 1} end + + assert GithubPrs.fetch_prs(runner) == [] + end + end +end diff --git a/test/trmnl_web/controllers/page_controller_test.exs b/test/trmnl_web/controllers/page_controller_test.exs index c805f60..6004c43 100644 --- a/test/trmnl_web/controllers/page_controller_test.exs +++ b/test/trmnl_web/controllers/page_controller_test.exs @@ -3,6 +3,6 @@ defmodule TrmnlWeb.PageControllerTest do test "GET /", %{conn: conn} do conn = get(conn, ~p"/") - assert html_response(conn, 200) =~ "Peace of mind from prototype to production" + assert redirected_to(conn, 302) == "/devices" end end diff --git a/test/trmnl_web/live/device_live_test.exs b/test/trmnl_web/live/device_live_test.exs index c96028f..106bf4f 100644 --- a/test/trmnl_web/live/device_live_test.exs +++ b/test/trmnl_web/live/device_live_test.exs @@ -4,9 +4,27 @@ defmodule TrmnlWeb.DeviceLiveTest do import Phoenix.LiveViewTest import Trmnl.InventoryFixtures - @create_attrs %{name: "some name", api_key: "some api_key", mac_address: "some mac_address", friendly_id: "some friendly_id", refresh_interval: 42} - @update_attrs %{name: "some updated name", api_key: "some updated api_key", mac_address: "some updated mac_address", friendly_id: "some updated friendly_id", refresh_interval: 43} - @invalid_attrs %{name: nil, api_key: nil, mac_address: nil, friendly_id: nil, refresh_interval: nil} + @create_attrs %{ + name: "some name", + api_key: "some api_key", + mac_address: "AA:BB:CC:DD:EE:FF", + friendly_id: "some friendly_id", + refresh_interval: 42 + } + @update_attrs %{ + name: "some updated name", + api_key: "some updated api_key", + mac_address: "11:22:33:44:55:66", + friendly_id: "some updated friendly_id", + refresh_interval: 43 + } + @invalid_attrs %{ + name: nil, + api_key: nil, + mac_address: nil, + friendly_id: nil, + refresh_interval: nil + } defp create_device(_) do device = device_fixture()