From f8d2289e79008d76eb5b31b693086b7e391635af Mon Sep 17 00:00:00 2001 From: cmaddox5 Date: Mon, 25 Nov 2024 13:38:52 -0500 Subject: [PATCH 1/3] Append flex_zone pages to response for Mercury screens. --- lib/screens/v2/screen_data.ex | 15 ++++++++++- .../controllers/v2/screen_api_controller.ex | 18 ++++++++++--- test/screens/v2/screen_data_test.exs | 27 +++++++++++++++++++ 3 files changed, 55 insertions(+), 5 deletions(-) diff --git a/lib/screens/v2/screen_data.ex b/lib/screens/v2/screen_data.ex index 4625a7e9f..09ca59809 100644 --- a/lib/screens/v2/screen_data.ex +++ b/lib/screens/v2/screen_data.ex @@ -16,6 +16,7 @@ defmodule Screens.V2.ScreenData do @type t :: %{type: atom()} @type simulation_data :: %{full_page: t(), flex_zone: [t()]} + @type data_with_flex_zone :: {t(), [t()]} @type variants(data) :: {data, %{String.t() => data}} @type screen_id :: String.t() @type options :: [ @@ -31,6 +32,12 @@ defmodule Screens.V2.ScreenData do select_variant(screen_id, opts, &layout_to_data/2) end + @spec get_with_flex_zone(screen_id()) :: data_with_flex_zone() + @spec get_with_flex_zone(screen_id(), options()) :: data_with_flex_zone() + def get_with_flex_zone(screen_id, opts \\ []) do + select_variant(screen_id, opts, &layout_to_data_with_flex_zone/2) + end + @spec simulation(screen_id()) :: simulation_data() @spec simulation(screen_id(), options()) :: simulation_data() def simulation(screen_id, opts \\ []) do @@ -50,7 +57,7 @@ defmodule Screens.V2.ScreenData do end @spec select_variant(screen_id(), options(), (Layout.t(), Screen.t() -> data)) :: data - when data: t() | simulation_data() + when data: t() | simulation_data() | data_with_flex_zone() defp select_variant(screen_id, opts, then_fn) do config = get_config(screen_id, opts) selected_variant = Keyword.get(opts, :generator_variant) @@ -98,6 +105,12 @@ defmodule Screens.V2.ScreenData do layout |> resolve_paging(config) |> serialize() end + @spec layout_to_data(Layout.t(), Screen.t()) :: data_with_flex_zone() + defp layout_to_data_with_flex_zone(layout, config) do + {layout |> resolve_paging(config) |> serialize(), + serialize_paged_slots(layout, config.app_id)} + end + @spec layout_to_simulation_data(Layout.t(), Screen.t()) :: simulation_data() defp layout_to_simulation_data(layout, config) do %{ diff --git a/lib/screens_web/controllers/v2/screen_api_controller.ex b/lib/screens_web/controllers/v2/screen_api_controller.ex index c8d39c9c0..27422ba06 100644 --- a/lib/screens_web/controllers/v2/screen_api_controller.ex +++ b/lib/screens_web/controllers/v2/screen_api_controller.ex @@ -8,7 +8,7 @@ defmodule ScreensWeb.V2.ScreenApiController do alias Screens.V2.{ScreenAudioData, ScreenData} alias ScreensConfig.Screen - @base_response %{data: nil, disabled: false, force_reload: false} + @base_response %{data: nil, disabled: false, force_reload: false, flex_zone: nil} @disabled_response %{@base_response | disabled: true} @outdated_response %{@base_response | force_reload: true} @@ -84,19 +84,29 @@ defmodule ScreensWeb.V2.ScreenApiController do response = screen_id - |> screen_response(variant, run_all_variants?: true, update_visible_alerts?: true) + |> screen_response(variant, screen, + run_all_variants?: true, + update_visible_alerts?: true + ) |> put_extra_fields(screen_id, screen) json(conn, response) end end - defp screen_response(screen_id, "all", opts) do + defp screen_response(screen_id, "all", _, opts) do {default, variants} = ScreenData.variants(screen_id, opts) Map.put(%{@base_response | data: default}, :variants, variants) end - defp screen_response(screen_id, variant, opts) do + defp screen_response(screen_id, variant, %Screen{vendor: :mercury}, opts) do + {data, flex_zone} = + ScreenData.get_with_flex_zone(screen_id, Keyword.put(opts, :generator_variant, variant)) + + %{@base_response | data: data, flex_zone: flex_zone} + end + + defp screen_response(screen_id, variant, _, opts) do data = ScreenData.get(screen_id, Keyword.put(opts, :generator_variant, variant)) %{@base_response | data: data} end diff --git a/test/screens/v2/screen_data_test.exs b/test/screens/v2/screen_data_test.exs index c4bef8863..8c359802a 100644 --- a/test/screens/v2/screen_data_test.exs +++ b/test/screens/v2/screen_data_test.exs @@ -132,6 +132,33 @@ defmodule Screens.V2.ScreenDataTest do end end + describe "get_with_flex_zone/2" do + setup do + stub(@parameters, :refresh_rate, fn _app_id -> 0 end) + :ok + end + + defp build_config(attrs) do + struct!( + %Screen{app_id: :test_app, app_params: %{}, device_id: "", name: "", vendor: :mercury}, + attrs + ) + end + + test "gets widget data and all flex_zone pages for a screen ID" do + expect(@config_cache, :screen, fn "test_id" -> build_config(%{app_id: :test_app}) end) + + expect( + @parameters, + :candidate_generator, + fn %Screen{app_id: :test_app}, nil -> GrayGenerator end + ) + + assert ScreenData.get_with_flex_zone("test_id") == + {%{type: :normal, main: %{type: :placeholder, color: :gray}}, []} + end + end + describe "variants/2" do setup do stub(@parameters, :refresh_rate, fn _app_id -> 0 end) From 9b3fc4af2525b4adde411e7f328725d39680ca75 Mon Sep 17 00:00:00 2001 From: cmaddox5 Date: Tue, 26 Nov 2024 13:49:13 -0500 Subject: [PATCH 2/3] Use existing simulation logic instead of creating new duplicate functions. --- lib/screens/v2/screen_data.ex | 15 +---------- .../controllers/v2/screen_api_controller.ex | 16 +++++------ test/screens/v2/screen_data_test.exs | 27 ------------------- 3 files changed, 9 insertions(+), 49 deletions(-) diff --git a/lib/screens/v2/screen_data.ex b/lib/screens/v2/screen_data.ex index 09ca59809..4625a7e9f 100644 --- a/lib/screens/v2/screen_data.ex +++ b/lib/screens/v2/screen_data.ex @@ -16,7 +16,6 @@ defmodule Screens.V2.ScreenData do @type t :: %{type: atom()} @type simulation_data :: %{full_page: t(), flex_zone: [t()]} - @type data_with_flex_zone :: {t(), [t()]} @type variants(data) :: {data, %{String.t() => data}} @type screen_id :: String.t() @type options :: [ @@ -32,12 +31,6 @@ defmodule Screens.V2.ScreenData do select_variant(screen_id, opts, &layout_to_data/2) end - @spec get_with_flex_zone(screen_id()) :: data_with_flex_zone() - @spec get_with_flex_zone(screen_id(), options()) :: data_with_flex_zone() - def get_with_flex_zone(screen_id, opts \\ []) do - select_variant(screen_id, opts, &layout_to_data_with_flex_zone/2) - end - @spec simulation(screen_id()) :: simulation_data() @spec simulation(screen_id(), options()) :: simulation_data() def simulation(screen_id, opts \\ []) do @@ -57,7 +50,7 @@ defmodule Screens.V2.ScreenData do end @spec select_variant(screen_id(), options(), (Layout.t(), Screen.t() -> data)) :: data - when data: t() | simulation_data() | data_with_flex_zone() + when data: t() | simulation_data() defp select_variant(screen_id, opts, then_fn) do config = get_config(screen_id, opts) selected_variant = Keyword.get(opts, :generator_variant) @@ -105,12 +98,6 @@ defmodule Screens.V2.ScreenData do layout |> resolve_paging(config) |> serialize() end - @spec layout_to_data(Layout.t(), Screen.t()) :: data_with_flex_zone() - defp layout_to_data_with_flex_zone(layout, config) do - {layout |> resolve_paging(config) |> serialize(), - serialize_paged_slots(layout, config.app_id)} - end - @spec layout_to_simulation_data(Layout.t(), Screen.t()) :: simulation_data() defp layout_to_simulation_data(layout, config) do %{ diff --git a/lib/screens_web/controllers/v2/screen_api_controller.ex b/lib/screens_web/controllers/v2/screen_api_controller.ex index 27422ba06..080f89d59 100644 --- a/lib/screens_web/controllers/v2/screen_api_controller.ex +++ b/lib/screens_web/controllers/v2/screen_api_controller.ex @@ -8,7 +8,7 @@ defmodule ScreensWeb.V2.ScreenApiController do alias Screens.V2.{ScreenAudioData, ScreenData} alias ScreensConfig.Screen - @base_response %{data: nil, disabled: false, force_reload: false, flex_zone: nil} + @base_response %{data: nil, disabled: false, force_reload: false} @disabled_response %{@base_response | disabled: true} @outdated_response %{@base_response | force_reload: true} @@ -84,7 +84,7 @@ defmodule ScreensWeb.V2.ScreenApiController do response = screen_id - |> screen_response(variant, screen, + |> screen_response(screen, variant, run_all_variants?: true, update_visible_alerts?: true ) @@ -94,19 +94,19 @@ defmodule ScreensWeb.V2.ScreenApiController do end end - defp screen_response(screen_id, "all", _, opts) do + defp screen_response(screen_id, _, "all", opts) do {default, variants} = ScreenData.variants(screen_id, opts) Map.put(%{@base_response | data: default}, :variants, variants) end - defp screen_response(screen_id, variant, %Screen{vendor: :mercury}, opts) do - {data, flex_zone} = - ScreenData.get_with_flex_zone(screen_id, Keyword.put(opts, :generator_variant, variant)) + defp screen_response(screen_id, %Screen{vendor: :mercury}, variant, opts) do + %{full_page: data, flex_zone: flex_zone} = + ScreenData.simulation(screen_id, Keyword.put(opts, :generator_variant, variant)) - %{@base_response | data: data, flex_zone: flex_zone} + Map.merge(%{@base_response | data: data}, %{flex_zone: flex_zone}) end - defp screen_response(screen_id, variant, _, opts) do + defp screen_response(screen_id, _, variant, opts) do data = ScreenData.get(screen_id, Keyword.put(opts, :generator_variant, variant)) %{@base_response | data: data} end diff --git a/test/screens/v2/screen_data_test.exs b/test/screens/v2/screen_data_test.exs index 8c359802a..c4bef8863 100644 --- a/test/screens/v2/screen_data_test.exs +++ b/test/screens/v2/screen_data_test.exs @@ -132,33 +132,6 @@ defmodule Screens.V2.ScreenDataTest do end end - describe "get_with_flex_zone/2" do - setup do - stub(@parameters, :refresh_rate, fn _app_id -> 0 end) - :ok - end - - defp build_config(attrs) do - struct!( - %Screen{app_id: :test_app, app_params: %{}, device_id: "", name: "", vendor: :mercury}, - attrs - ) - end - - test "gets widget data and all flex_zone pages for a screen ID" do - expect(@config_cache, :screen, fn "test_id" -> build_config(%{app_id: :test_app}) end) - - expect( - @parameters, - :candidate_generator, - fn %Screen{app_id: :test_app}, nil -> GrayGenerator end - ) - - assert ScreenData.get_with_flex_zone("test_id") == - {%{type: :normal, main: %{type: :placeholder, color: :gray}}, []} - end - end - describe "variants/2" do setup do stub(@parameters, :refresh_rate, fn _app_id -> 0 end) From 8479f827d96014a4a2deb6f740bc571e0e1ab1a2 Mon Sep 17 00:00:00 2001 From: cmaddox5 Date: Mon, 2 Dec 2024 11:27:31 -0500 Subject: [PATCH 3/3] Added tests. --- test/fixtures/config.json | 63 +++++++++++++++ test/screens/v2/screen_data_test.exs | 29 +------ .../v2/screen_api_controller_test.exs | 79 +++++++++++++++++++ test/support/candidate_generator_stub.ex | 26 ++++++ 4 files changed, 169 insertions(+), 28 deletions(-) create mode 100644 test/screens_web/controllers/v2/screen_api_controller_test.exs create mode 100644 test/support/candidate_generator_stub.ex diff --git a/test/fixtures/config.json b/test/fixtures/config.json index 76192bf24..513d14d63 100644 --- a/test/fixtures/config.json +++ b/test/fixtures/config.json @@ -174,6 +174,69 @@ "refresh_if_loaded_before": null, "tags": [], "vendor": "lg_mri" + }, + "EIG-604": { + "disabled": false, + "name": null, + "app_id": "gl_eink_v2", + "app_params": { + "header": { + "direction_id": 0, + "route_id": "Green-E" + }, + "departures": { + "sections": [ + { + "header": { + "title": null, + "arrow": null, + "read_as": null + }, + "filters": { + "max_minutes": null, + "route_directions": null + }, + "query": { + "opts": { + "include_schedules": false + }, + "params": { + "direction_id": 0, + "stop_ids": ["place-nuniv"], + "route_ids": ["Green-E"], + "route_type": null + } + }, + "layout": { + "max": null, + "min": 1, + "base": null, + "include_later": false + }, + "bidirectional": false + } + ] + }, + "footer": { + "stop_id": "place-nuniv" + }, + "alerts": { + "stop_id": "70243" + }, + "evergreen_content": [], + "line_map": { + "stop_id": "70243", + "direction_id": 0, + "route_id": "Green-E", + "station_id": "place-nuniv" + }, + "platform_location": "front" + }, + "device_id": null, + "hidden_from_screenplay": false, + "refresh_if_loaded_before": null, + "tags": [], + "vendor": "mercury" } } } diff --git a/test/screens/v2/screen_data_test.exs b/test/screens/v2/screen_data_test.exs index c4bef8863..ac135abfc 100644 --- a/test/screens/v2/screen_data_test.exs +++ b/test/screens/v2/screen_data_test.exs @@ -1,30 +1,3 @@ -defmodule Screens.V2.ScreenDataTest.Stub do - defmacro candidate_generator(name, instances_fn) do - quote do - defmodule unquote(name) do - alias Screens.V2.CandidateGenerator - alias Screens.V2.Template.Builder - alias Screens.V2.WidgetInstance.Placeholder - - @behaviour CandidateGenerator - - @impl CandidateGenerator - def screen_template(), do: Builder.build_template({:screen, %{normal: [:main]}}) - - @impl CandidateGenerator - def candidate_instances(config) do - unquote(instances_fn).(config) - end - - @impl CandidateGenerator - def audio_only_instances(_widgets, _config), do: [] - - defp placeholder(color), do: %Placeholder{color: color, slot_names: [:main]} - end - end - end -end - defmodule Screens.V2.ScreenDataTest do use ExUnit.Case, async: true @@ -32,7 +5,7 @@ defmodule Screens.V2.ScreenDataTest do alias Screens.V2.ScreenData alias Screens.V2.WidgetInstance.MockWidget alias Screens.V2.WidgetInstance.Placeholder - alias Screens.V2.ScreenDataTest.Stub + alias Screens.TestSupport.CandidateGeneratorStub, as: Stub import ExUnit.CaptureLog import Screens.Inject diff --git a/test/screens_web/controllers/v2/screen_api_controller_test.exs b/test/screens_web/controllers/v2/screen_api_controller_test.exs new file mode 100644 index 000000000..f9fcbc062 --- /dev/null +++ b/test/screens_web/controllers/v2/screen_api_controller_test.exs @@ -0,0 +1,79 @@ +defmodule ScreensWeb.V2.ScreenApiControllerTest do + use ScreensWeb.ConnCase + + alias ScreensConfig.Screen + alias Screens.TestSupport.CandidateGeneratorStub, as: Stub + + import Screens.Inject + import Mox + setup :verify_on_exit! + + @cache injected(Screens.Config.Cache) + @parameters injected(Screens.V2.ScreenData.Parameters) + + require Stub + + Stub.candidate_generator(MercuryGenerator, fn _ -> [placeholder(:green)] end) + Stub.candidate_generator(LgMriGenerator, fn _ -> [placeholder(:red)] end) + + describe "show/2" do + test "only returns flex_zone for Mercury screens", %{conn: conn} do + expect(@cache, :screen, fn + "EIG-604" -> + struct(Screen, app_id: :gl_eink_v2, vendor: :mercury) + end) + + expect(@parameters, :variants, fn _ -> [nil] end) + + stub( + @parameters, + :candidate_generator, + fn %Screen{vendor: :mercury}, nil -> MercuryGenerator end + ) + + stub(@parameters, :refresh_rate, fn _app_id -> 0 end) + + conn = get(conn, "/v2/api/screen/EIG-604?last_refresh=2024-12-02T00:00:00Z") + + assert %{ + "audio_data" => "", + "data" => %{ + "main" => %{"color" => "green", "type" => "placeholder"}, + "type" => "normal" + }, + "disabled" => false, + "flex_zone" => [], + "force_reload" => false, + "last_deploy_timestamp" => nil + } == json_response(conn, 200) + end + + test "omits flex_zone from non-Mercury screens", %{conn: conn} do + expect(@cache, :screen, fn + "1401" -> + struct(Screen, app_id: :bus_shelter_v2, vendor: :lg_mri) + end) + + expect(@parameters, :variants, fn _ -> [nil] end) + + stub( + @parameters, + :candidate_generator, + fn %Screen{vendor: :lg_mri}, nil -> LgMriGenerator end + ) + + stub(@parameters, :refresh_rate, fn _app_id -> 0 end) + + conn = get(conn, "/v2/api/screen/1401?last_refresh=2024-12-02T00:00:00Z") + + assert %{ + "data" => %{ + "main" => %{"color" => "red", "type" => "placeholder"}, + "type" => "normal" + }, + "disabled" => false, + "force_reload" => false + } == json_response(conn, 200) + end + end +end diff --git a/test/support/candidate_generator_stub.ex b/test/support/candidate_generator_stub.ex new file mode 100644 index 000000000..1eb3cdacb --- /dev/null +++ b/test/support/candidate_generator_stub.ex @@ -0,0 +1,26 @@ +defmodule Screens.TestSupport.CandidateGeneratorStub do + defmacro candidate_generator(name, instances_fn) do + quote do + defmodule unquote(name) do + alias Screens.V2.CandidateGenerator + alias Screens.V2.Template.Builder + alias Screens.V2.WidgetInstance.Placeholder + + @behaviour CandidateGenerator + + @impl CandidateGenerator + def screen_template(), do: Builder.build_template({:screen, %{normal: [:main]}}) + + @impl CandidateGenerator + def candidate_instances(config) do + unquote(instances_fn).(config) + end + + @impl CandidateGenerator + def audio_only_instances(_widgets, _config), do: [] + + defp placeholder(color), do: %Placeholder{color: color, slot_names: [:main]} + end + end + end +end