From 298be837dd508cf22b5145546c5f5b12a4cddfb5 Mon Sep 17 00:00:00 2001 From: Tom Konidas Date: Sat, 12 Aug 2023 17:48:29 -0400 Subject: [PATCH] Refactor Rating to use Rating Type (#350) --- lib/plexus/apps.ex | 32 +++++------ lib/plexus/query_helpers.ex | 4 +- lib/plexus/ratings.ex | 2 +- lib/plexus/schemas/rating.ex | 4 +- lib/plexus/schemas/score.ex | 2 +- lib/plexus_web/controllers/api/v1/app_json.ex | 12 ++--- .../controllers/api/v1/rating_controller.ex | 6 +-- .../controllers/api/v1/rating_json.ex | 2 +- .../live/admin/app_live/index.html.heex | 8 +-- lib/plexus_web/live/admin/app_live/show.ex | 2 +- .../live/admin/app_live/show.html.heex | 6 +-- .../live/admin/rating_live/index.ex | 14 +++-- .../live/admin/rating_live/index.html.heex | 4 +- lib/plexus_web/router.ex | 2 +- .../20230301014024_create_ratings.exs | 6 +-- priv/repo/seeds.exs | 44 +++++++++++++++ test/plexus/apps_test.exs | 53 ++++++++++--------- test/plexus/ratings_test.exs | 6 +-- .../api/v1/app_controller_test.exs | 14 ++--- .../api/v1/rating_controller_test.exs | 8 +-- test/support/fixtures/ratings_fixtures.ex | 2 +- 21 files changed, 138 insertions(+), 95 deletions(-) diff --git a/lib/plexus/apps.ex b/lib/plexus/apps.ex index 51b624cb..abfc115e 100644 --- a/lib/plexus/apps.ex +++ b/lib/plexus/apps.ex @@ -93,34 +93,30 @@ defmodule Plexus.Apps do end defp with_scores(query) do - google_lib_micro_g = from Rating, where: [google_lib: :micro_g] - google_lib_none = from Rating, where: [google_lib: :none] + micro_g = from Rating, where: [rating_type: :micro_g] + native = from Rating, where: [rating_type: :native] query - |> join(:left, [a], r_micro_g in subquery(google_lib_micro_g), - on: a.package == r_micro_g.app_package - ) - |> join(:left, [a, _], r_none in subquery(google_lib_none), - on: a.package == r_none.app_package - ) - |> group_by([a, r_micro_g, r_none], [a.package, r_micro_g.google_lib, r_none.google_lib]) - |> select_merge([a, r_micro_g, r_none], %{ - scores: [ - %Score{ + |> join(:left, [a], r_micro_g in subquery(micro_g), on: a.package == r_micro_g.app_package) + |> join(:left, [a, _], r_native in subquery(native), on: a.package == r_native.app_package) + |> group_by([a, r_micro_g, r_native], [a.package, r_micro_g.rating_type, r_native.rating_type]) + |> select_merge([a, r_micro_g, r_native], %{ + scores: %{ + native: %Score{ app_package: a.package, - google_lib: :none, + rating_type: :native, numerator: - fragment("CAST(ROUND(AVG(COALESCE(?, 0))::numeric, 2) AS FLOAT)", r_none.score), - total_count: count(r_none.id, :distinct) + fragment("CAST(ROUND(AVG(COALESCE(?, 0))::numeric, 2) AS FLOAT)", r_native.score), + total_count: count(r_native.id, :distinct) }, - %Score{ + micro_g: %Score{ app_package: a.package, - google_lib: :micro_g, + rating_type: :micro_g, numerator: fragment("CAST(ROUND(AVG(COALESCE(?, 0))::numeric, 2) AS FLOAT)", r_micro_g.score), total_count: count(r_micro_g.id, :distinct) } - ] + } }) end diff --git a/lib/plexus/query_helpers.ex b/lib/plexus/query_helpers.ex index c101abd6..9a35661b 100644 --- a/lib/plexus/query_helpers.ex +++ b/lib/plexus/query_helpers.ex @@ -24,8 +24,8 @@ defmodule Plexus.QueryHelpers do {:preload, associations}, query -> from q in query, preload: ^associations - {:google_lib, google_lib}, query -> - from q in query, where: q.google_lib == ^google_lib + {:rating_type, rating_type}, query -> + from q in query, where: q.rating_type == ^rating_type _, query -> query diff --git a/lib/plexus/ratings.ex b/lib/plexus/ratings.ex index 33f2733a..5ea73c6b 100644 --- a/lib/plexus/ratings.ex +++ b/lib/plexus/ratings.ex @@ -53,7 +53,7 @@ defmodule Plexus.Ratings do rom_name: String.t(), rom_build: String.t(), installation_source: String.t(), - google_lib: atom(), + rating_type: atom(), score: pos_integer() }) :: {:ok, Rating.t()} | {:error, Ecto.Changeset.t()} def create_rating(params) do diff --git a/lib/plexus/schemas/rating.ex b/lib/plexus/schemas/rating.ex index 04efe866..84e65a25 100644 --- a/lib/plexus/schemas/rating.ex +++ b/lib/plexus/schemas/rating.ex @@ -11,7 +11,7 @@ defmodule Plexus.Schemas.Rating do field :android_version, :string field :rom_name, :string field :rom_build, :string - field :google_lib, Ecto.Enum, values: [:none, :micro_g] + field :rating_type, Ecto.Enum, values: [:native, :micro_g] field :installation_source, :string field :notes, :string field :score, :integer @@ -31,7 +31,7 @@ defmodule Plexus.Schemas.Rating do :app_build_number, :rom_name, :rom_build, - :google_lib, + :rating_type, :installation_source, :score ] diff --git a/lib/plexus/schemas/score.ex b/lib/plexus/schemas/score.ex index d8f6153b..3973851b 100644 --- a/lib/plexus/schemas/score.ex +++ b/lib/plexus/schemas/score.ex @@ -4,7 +4,7 @@ defmodule Plexus.Schemas.Score do @primary_key false embedded_schema do field :app_package, :string - field :google_lib, Ecto.Enum, values: [:none, :micro_g] + field :rating_type, Ecto.Enum, values: [:native, :micro_g] field :numerator, :integer, default: 0 field :denominator, :integer, default: 4 field :total_count, :integer, default: 0 diff --git a/lib/plexus_web/controllers/api/v1/app_json.ex b/lib/plexus_web/controllers/api/v1/app_json.ex index 85a07375..290c079c 100644 --- a/lib/plexus_web/controllers/api/v1/app_json.ex +++ b/lib/plexus_web/controllers/api/v1/app_json.ex @@ -26,20 +26,16 @@ defmodule PlexusWeb.API.V1.AppJSON do defp data(%Score{} = score) do %{ - google_lib: score.google_lib, + rating_type: score.rating_type, numerator: score.numerator, denominator: score.denominator, total_count: score.total_count } end - defp merge_scores(payload, scores) when is_list(scores) do - scores - |> Enum.reverse() - |> Enum.reduce(payload, fn score, acc -> - score_data = data(score) - Map.update(acc, :scores, [score_data], &[score_data | &1]) - end) + defp merge_scores(payload, scores) when is_map(scores) do + data = Map.new(scores, fn {rating_type, score} -> {rating_type, data(score)} end) + Map.put(payload, :scores, data) end defp merge_scores(payload, _scores), do: payload diff --git a/lib/plexus_web/controllers/api/v1/rating_controller.ex b/lib/plexus_web/controllers/api/v1/rating_controller.ex index fbacf06f..8ea5fc2d 100644 --- a/lib/plexus_web/controllers/api/v1/rating_controller.ex +++ b/lib/plexus_web/controllers/api/v1/rating_controller.ex @@ -24,7 +24,7 @@ defmodule PlexusWeb.API.V1.RatingController do rom_name: {:string, [required: true]}, rom_build: {:string, [required: true]}, installation_source: {:string, [required: true]}, - google_lib: {google_lib_enum(), [required: true]}, + rating_type: {rating_type_enum(), [required: true]}, notes: {:string, []}, score: {:integer, [required: true]} } @@ -45,8 +45,8 @@ defmodule PlexusWeb.API.V1.RatingController do render(conn, :show, rating: rating) end - defp google_lib_enum do - values = Ecto.Enum.values(Plexus.Schemas.Rating, :google_lib) + defp rating_type_enum do + values = Ecto.Enum.values(Plexus.Schemas.Rating, :rating_type) {:parameterized, Ecto.Enum, Ecto.Enum.init(values: values)} end diff --git a/lib/plexus_web/controllers/api/v1/rating_json.ex b/lib/plexus_web/controllers/api/v1/rating_json.ex index f3733959..b06cf56d 100644 --- a/lib/plexus_web/controllers/api/v1/rating_json.ex +++ b/lib/plexus_web/controllers/api/v1/rating_json.ex @@ -26,7 +26,7 @@ defmodule PlexusWeb.API.V1.RatingJSON do installation_source: rating.installation_source, score: %{numerator: rating.score, denominator: 4}, notes: rating.notes, - google_lib: rating.google_lib + rating_type: rating.rating_type } end end diff --git a/lib/plexus_web/live/admin/app_live/index.html.heex b/lib/plexus_web/live/admin/app_live/index.html.heex index 5febb7fd..70c96b6d 100644 --- a/lib/plexus_web/live/admin/app_live/index.html.heex +++ b/lib/plexus_web/live/admin/app_live/index.html.heex @@ -27,11 +27,11 @@ <:col :let={{_dom_id, app}} label="Package">

<%= app.package %>

- <:col :let={{_dom_id, %{scores: [native_score, _micro_g_score]}}} label="Native"> - <.badge score={native_score} /> + <:col :let={{_dom_id, app}} label="Native"> + <.badge score={app.scores.native} /> - <:col :let={{_dom_id, %{scores: [_native_score, micro_g_score]}}} label="MicroG"> - <.badge score={micro_g_score} /> + <:col :let={{_dom_id, app}} label="MicroG"> + <.badge score={app.scores.micro_g} /> <:action :let={{_dom_id, app}}>
diff --git a/lib/plexus_web/live/admin/app_live/show.ex b/lib/plexus_web/live/admin/app_live/show.ex index 0166bbbc..1a637e77 100644 --- a/lib/plexus_web/live/admin/app_live/show.ex +++ b/lib/plexus_web/live/admin/app_live/show.ex @@ -45,7 +45,7 @@ defmodule PlexusWeb.Admin.AppLive.Show do assign( assigns, :score, - Enum.find(assigns.app.scores, &(&1.google_lib == assigns.google_lib)) + assigns.app.scores[assigns.rating_type] ) ~H""" diff --git a/lib/plexus_web/live/admin/app_live/show.html.heex b/lib/plexus_web/live/admin/app_live/show.html.heex index 7411ae20..a7b1af60 100644 --- a/lib/plexus_web/live/admin/app_live/show.html.heex +++ b/lib/plexus_web/live/admin/app_live/show.html.heex @@ -21,16 +21,16 @@ <:item title="Native"> - <.score app={@app} google_lib={:none} /> + <.score app={@app} rating_type={:native} /> <.link - navigate={~p"/admin/apps/#{@app}/ratings/none"} + navigate={~p"/admin/apps/#{@app}/ratings/native"} class="block text-sm font-semibold leading-6 text-zinc-900 hover:text-zinc-700" > View ratings <:item title="MicroG"> - <.score app={@app} google_lib={:micro_g} /> + <.score app={@app} rating_type={:micro_g} /> <.link navigate={~p"/admin/apps/#{@app}/ratings/micro_g"} class="block text-sm font-semibold leading-6 text-zinc-900 hover:text-zinc-700 self-end" diff --git a/lib/plexus_web/live/admin/rating_live/index.ex b/lib/plexus_web/live/admin/rating_live/index.ex index 0feee663..3c9daab6 100644 --- a/lib/plexus_web/live/admin/rating_live/index.ex +++ b/lib/plexus_web/live/admin/rating_live/index.ex @@ -5,20 +5,24 @@ defmodule PlexusWeb.Admin.RatingLive.Index do alias Plexus.Ratings @impl Phoenix.LiveView - def mount(%{"package" => package, "google_lib" => google_lib}, _session, socket) do + def mount(%{"package" => package, "rating_type" => rating_type}, _session, socket) do if connected?(socket), do: Apps.subscribe(package) + rating_type = String.to_existing_atom(rating_type) page = Ratings.list_ratings(package, - google_lib: google_lib, + rating_type: rating_type, page_size: 9999, order_by: [desc: :app_build_number, desc: :updated_at] ) + app = Apps.get_app!(package, scores: true) + {:ok, socket - |> assign(:google_lib, google_lib) - |> assign(:app, Apps.get_app!(package, scores: true)) + |> assign(:rating_type, rating_type) + |> assign(:app, app) + |> assign(:score, app.scores[rating_type]) |> stream(:ratings, page.entries)} end @@ -53,7 +57,7 @@ defmodule PlexusWeb.Admin.RatingLive.Index do end def handle_info({:app_rating_updated, rating}, socket) do - if rating.google_lib == String.to_existing_atom(socket.assigns.google_lib) do + if rating.rating_type == String.to_existing_atom(socket.assigns.rating_type) do {:noreply, socket |> put_flash(:info, "Rating Added") diff --git a/lib/plexus_web/live/admin/rating_live/index.html.heex b/lib/plexus_web/live/admin/rating_live/index.html.heex index e51aa027..290f8bda 100644 --- a/lib/plexus_web/live/admin/rating_live/index.html.heex +++ b/lib/plexus_web/live/admin/rating_live/index.html.heex @@ -1,7 +1,7 @@ <.header> - <%= @app.name %> <%= if @google_lib == "none", do: "Native", else: "MicroG" %> Ratings + <%= @app.name %> <%= if @rating_type == :native, do: "Native", else: "MicroG" %> Ratings <:subtitle> - <.badge score={Enum.find(@app.scores, &(to_string(&1.google_lib) == @google_lib))} /> + <.badge score={@score} /> diff --git a/lib/plexus_web/router.ex b/lib/plexus_web/router.ex index ed26ef8a..fb83e6c4 100644 --- a/lib/plexus_web/router.ex +++ b/lib/plexus_web/router.ex @@ -30,7 +30,7 @@ defmodule PlexusWeb.Router do live "/apps/:package", AppLive.Show, :show live "/apps/:package/show/edit", AppLive.Show, :edit - live "/apps/:package/ratings/:google_lib", RatingLive.Index, :index + live "/apps/:package/ratings/:rating_type", RatingLive.Index, :index end scope "/api/v1", PlexusWeb.API.V1 do diff --git a/priv/repo/migrations/20230301014024_create_ratings.exs b/priv/repo/migrations/20230301014024_create_ratings.exs index 357d73a5..240c13ed 100644 --- a/priv/repo/migrations/20230301014024_create_ratings.exs +++ b/priv/repo/migrations/20230301014024_create_ratings.exs @@ -9,7 +9,7 @@ defmodule Plexus.Repo.Migrations.CreateRatings do add :app_build_number, :integer add :rom_name, :string add :rom_build, :string - add :google_lib, :string, null: false + add :rating_type, :string, null: false add :score, :integer, null: false add :installation_source, :text add :notes, :text @@ -22,7 +22,7 @@ defmodule Plexus.Repo.Migrations.CreateRatings do end create index(:ratings, [:app_package]) - create index(:ratings, [:google_lib]) - create index(:ratings, [:app_package, :google_lib]) + create index(:ratings, [:rating_type]) + create index(:ratings, [:app_package, :rating_type]) end end diff --git a/priv/repo/seeds.exs b/priv/repo/seeds.exs index bfd185e3..9fe92bce 100644 --- a/priv/repo/seeds.exs +++ b/priv/repo/seeds.exs @@ -9,3 +9,47 @@ # # We recommend using the bang functions (`insert!`, `update!` # and so on) as they will fail if something goes wrong. +alias Plexus.Schemas.App + +for params <- [ + %{ + package: "com.beemdevelopment.aegis", + name: "Aegis", + icon_url: + "https://play-lh.googleusercontent.com/qDzsOmoRT9o6QTElCaRJElAtfYW-nnOadwIInb6bXSEKexB211SsEtSeZrF5xm_lKUY" + }, + %{ + package: "com.aurora.store", + name: "Aurora Store", + icon_url: + "https://f-droid.org/repo/com.aurora.store/en-US/icon_tbAhwq51NNd0liZcTg0cQNNvazxrWlj7bPCe_1TkCV8=.png" + }, + %{ + package: "com.x8bit.bitwarden", + name: "Bitwarden", + icon_url: + "https://play-lh.googleusercontent.com/-jz18EgBYlmeHlnsq_iltq6uLnYFtXAVR_gi_d0qEj0pANQ1MtrJIstJoCQtImlWKwc" + }, + %{ + package: "org.thoughtcrime.securesms", + name: "Signal", + icon_url: + "https://play-lh.googleusercontent.com/jCln_XT8Ruzp7loH1S6yM-ZzzpLP1kZ3CCdXVEo0tP2w5HNtWQds6lo6aLxLIjiW_X8" + }, + %{ + package: "com.google.android.youtube", + name: "YouTube", + icon_url: + "https://play-lh.googleusercontent.com/lMoItBgdPPVDJsNOVtP26EKHePkwBg-PkuY9NOrc-fumRtTFP4XhpUNk_22syN4Datc" + } + ] do + app = + case Plexus.Repo.get(App, params.package) do + nil -> %App{package: params.package} + app -> app + end + + app + |> App.changeset(params) + |> Plexus.Repo.insert_or_update!() +end diff --git a/test/plexus/apps_test.exs b/test/plexus/apps_test.exs index ecaec602..10a7b8b3 100644 --- a/test/plexus/apps_test.exs +++ b/test/plexus/apps_test.exs @@ -12,16 +12,20 @@ defmodule Plexus.AppsTest do describe "list_apps/1" do test "returns a page of apps" do - app = app_fixture() - assert %Scrivener.Page{entries: [^app]} = Apps.list_apps() + assert %Scrivener.Page{entries: apps} = Apps.list_apps() + + for app <- apps do + assert is_struct(app, App) + end end test "with scores" do app_fixture() %Scrivener.Page{entries: apps} = Apps.list_apps(scores: true) - for app <- apps do - assert [%Score{google_lib: :none}, %Score{google_lib: :micro_g}] = app.scores + for %App{scores: scores} <- apps do + assert %Score{rating_type: :native} = scores.native + assert %Score{rating_type: :micro_g} = scores.micro_g end end end @@ -34,30 +38,29 @@ defmodule Plexus.AppsTest do test "with scores" do %{package: app_package} = app_fixture() - rating_fixture(%{app_package: app_package, score: 3, google_lib: :micro_g}) - rating_fixture(%{app_package: app_package, score: 4, google_lib: :micro_g}) - rating_fixture(%{app_package: app_package, score: 4, google_lib: :micro_g}) - rating_fixture(%{app_package: app_package, score: 1, google_lib: :none}) - rating_fixture(%{app_package: app_package, score: 2, google_lib: :none}) + rating_fixture(%{app_package: app_package, score: 3, rating_type: :micro_g}) + rating_fixture(%{app_package: app_package, score: 4, rating_type: :micro_g}) + rating_fixture(%{app_package: app_package, score: 4, rating_type: :micro_g}) + rating_fixture(%{app_package: app_package, score: 1, rating_type: :native}) + rating_fixture(%{app_package: app_package, score: 2, rating_type: :native}) app = Apps.get_app!(app_package, scores: true) - assert [ - %Score{ - app_package: ^app_package, - google_lib: :none, - numerator: 1.5, - denominator: 4, - total_count: 2 - }, - %Score{ - app_package: ^app_package, - google_lib: :micro_g, - numerator: 3.67, - denominator: 4, - total_count: 3 - } - ] = app.scores + assert %Score{ + app_package: ^app_package, + rating_type: :native, + numerator: 1.5, + denominator: 4, + total_count: 2 + } = app.scores.native + + assert %Score{ + app_package: ^app_package, + rating_type: :micro_g, + numerator: 3.67, + denominator: 4, + total_count: 3 + } = app.scores.micro_g end end diff --git a/test/plexus/ratings_test.exs b/test/plexus/ratings_test.exs index ebb98379..54555332 100644 --- a/test/plexus/ratings_test.exs +++ b/test/plexus/ratings_test.exs @@ -11,7 +11,7 @@ defmodule Plexus.RatingsTest do app_package: nil, app_build_number: nil, app_version: nil, - google_lib: nil, + rating_type: nil, rom_name: nil, rom_build: nil, installation_source: nil, @@ -43,7 +43,7 @@ defmodule Plexus.RatingsTest do android_version: "some android_version", app_build_number: 42, app_version: "some app_version", - google_lib: :none, + rating_type: :native, notes: "some notes", score: 3, installation_source: "fdroid", @@ -56,7 +56,7 @@ defmodule Plexus.RatingsTest do assert rating.app_package == app.package assert rating.app_build_number == 42 assert rating.app_version == "some app_version" - assert rating.google_lib == :none + assert rating.rating_type == :native assert rating.notes == "some notes" assert rating.score == 3 assert rating.installation_source == "fdroid" diff --git a/test/plexus_web/controllers/api/v1/app_controller_test.exs b/test/plexus_web/controllers/api/v1/app_controller_test.exs index 238a4b0a..0501252e 100644 --- a/test/plexus_web/controllers/api/v1/app_controller_test.exs +++ b/test/plexus_web/controllers/api/v1/app_controller_test.exs @@ -18,8 +18,8 @@ defmodule PlexusWeb.API.V1.AppControllerTest do describe "index" do test "lists all apps", %{conn: conn} do conn = get(conn, ~p"/api/v1/apps") - - assert %{"data" => []} = json_response(conn, 200) + assert %{"data" => data} = json_response(conn, 200) + assert is_list(data) end test "renders meta", %{conn: conn} do @@ -29,19 +29,19 @@ defmodule PlexusWeb.API.V1.AppControllerTest do "meta" => %{ "page_number" => 1, "limit" => 25, - "total_count" => 0, - "total_pages" => 1 + "total_count" => _, + "total_pages" => _ } } = json_response(conn, 200) end test "with scores", %{conn: conn} do - app_fixture() opts = %{scores: true} conn = get(conn, ~p"/api/v1/apps?#{opts}") - assert [%{"scores" => scores}] = json_response(conn, 200)["data"] - assert is_list(scores) + for app <- json_response(conn, 200)["data"] do + assert is_map(app["scores"]) + end end end diff --git a/test/plexus_web/controllers/api/v1/rating_controller_test.exs b/test/plexus_web/controllers/api/v1/rating_controller_test.exs index be302add..39f9bf3e 100644 --- a/test/plexus_web/controllers/api/v1/rating_controller_test.exs +++ b/test/plexus_web/controllers/api/v1/rating_controller_test.exs @@ -7,7 +7,7 @@ defmodule PlexusWeb.API.V1.RatingControllerTest do android_version: "some android version", app_build_number: 42, app_version: "some app_version", - google_lib: :none, + rating_type: :native, notes: "some notes", score: 2, installation_source: "fdroid", @@ -19,7 +19,7 @@ defmodule PlexusWeb.API.V1.RatingControllerTest do android_version: nil, app_build_number: nil, app_version: nil, - google_lib: nil, + rating_type: nil, installation_source: nil, rom_name: nil, rom_build: nil, @@ -72,7 +72,7 @@ defmodule PlexusWeb.API.V1.RatingControllerTest do "rom_name" => "some ROM name", "rom_build" => "some ROM build", "installation_source" => "fdroid", - "google_lib" => "none", + "rating_type" => "native", "notes" => "some notes", "score" => %{"numerator" => 2, "denominator" => 4} } = json_response(conn, 200)["data"] @@ -90,7 +90,7 @@ defmodule PlexusWeb.API.V1.RatingControllerTest do "rom_name" => ["can't be blank"], "rom_build" => ["can't be blank"], "installation_source" => ["can't be blank"], - "google_lib" => ["can't be blank"], + "rating_type" => ["can't be blank"], "score" => ["can't be blank"] } } = json_response(conn, 422) diff --git a/test/support/fixtures/ratings_fixtures.ex b/test/support/fixtures/ratings_fixtures.ex index 4fccaf79..4f9d0e03 100644 --- a/test/support/fixtures/ratings_fixtures.ex +++ b/test/support/fixtures/ratings_fixtures.ex @@ -61,7 +61,7 @@ defmodule Plexus.RatingsFixtures do rom_name: "Graphene", rom_build: "ROMv1", installation_source: "fdroid", - google_lib: :none, + rating_type: :native, notes: gnu_linux(), score: 2 })