From 5d977e6637563118b4017759104694e0ed8a316b Mon Sep 17 00:00:00 2001 From: Tom Konidas Date: Wed, 18 Sep 2024 17:52:16 -0400 Subject: [PATCH] Remove `app_version` Requirement for App Rating (#368) After some field testing, we have noticed that app_version could be nullable. So this PR is to drop that hard constraint against having the field be non nullable. I would have liked to return NULL in the API however this would break the current app so for now It just defaults to "" whenever it is NULL. --- lib/plexus/ratings.ex | 2 +- lib/plexus/schemas/rating.ex | 3 +-- .../controllers/api/v1/rating_controller.ex | 2 +- .../controllers/api/v1/rating_json.ex | 2 +- ...05824_make_rating_app_version_nullable.exs | 9 +++++++++ test/plexus/ratings_test.exs | 19 +++++++++++++++++- .../api/v1/rating_controller_test.exs | 20 ++++++++++++++++++- 7 files changed, 50 insertions(+), 7 deletions(-) create mode 100644 priv/repo/migrations/20240918205824_make_rating_app_version_nullable.exs diff --git a/lib/plexus/ratings.ex b/lib/plexus/ratings.ex index 97f66a2d..31fb0a99 100644 --- a/lib/plexus/ratings.ex +++ b/lib/plexus/ratings.ex @@ -49,7 +49,7 @@ defmodule Plexus.Ratings do optional(:notes) => String.t(), android_version: String.t(), app_package: String.t(), - app_version: String.t(), + app_version: String.t() | nil, app_build_number: non_neg_integer(), rom_name: String.t(), rom_build: String.t(), diff --git a/lib/plexus/schemas/rating.ex b/lib/plexus/schemas/rating.ex index 84e65a25..cbc44e01 100644 --- a/lib/plexus/schemas/rating.ex +++ b/lib/plexus/schemas/rating.ex @@ -27,7 +27,6 @@ defmodule Plexus.Schemas.Rating do @required [ :android_version, :app_package, - :app_version, :app_build_number, :rom_name, :rom_build, @@ -35,7 +34,7 @@ defmodule Plexus.Schemas.Rating do :installation_source, :score ] - @optional [:notes] + @optional [:app_version, :notes] @doc false def changeset(%Rating{} = rating, attrs) do rating diff --git a/lib/plexus_web/controllers/api/v1/rating_controller.ex b/lib/plexus_web/controllers/api/v1/rating_controller.ex index be134073..6625692c 100644 --- a/lib/plexus_web/controllers/api/v1/rating_controller.ex +++ b/lib/plexus_web/controllers/api/v1/rating_controller.ex @@ -43,7 +43,7 @@ defmodule PlexusWeb.API.V1.RatingController do schema = %{ android_version: {:string, [required: true]}, app_package: {:string, [required: true]}, - app_version: {:string, [required: true]}, + app_version: {:string, [required: false]}, app_build_number: {:integer, [required: true]}, rom_name: {:string, [required: true]}, rom_build: {:string, [required: true]}, diff --git a/lib/plexus_web/controllers/api/v1/rating_json.ex b/lib/plexus_web/controllers/api/v1/rating_json.ex index b06cf56d..951f07c6 100644 --- a/lib/plexus_web/controllers/api/v1/rating_json.ex +++ b/lib/plexus_web/controllers/api/v1/rating_json.ex @@ -19,7 +19,7 @@ defmodule PlexusWeb.API.V1.RatingJSON do id: rating.id, android_version: rating.android_version, app_package: rating.app_package, - app_version: rating.app_version, + app_version: rating.app_version || "", app_build_number: rating.app_build_number, rom_name: rating.rom_name, rom_build: rating.rom_build, diff --git a/priv/repo/migrations/20240918205824_make_rating_app_version_nullable.exs b/priv/repo/migrations/20240918205824_make_rating_app_version_nullable.exs new file mode 100644 index 00000000..45d02b23 --- /dev/null +++ b/priv/repo/migrations/20240918205824_make_rating_app_version_nullable.exs @@ -0,0 +1,9 @@ +defmodule Plexus.Repo.Migrations.MakeRatingAppVersionNullable do + use Ecto.Migration + + def change do + alter table(:ratings) do + modify :app_version, :string, null: true, from: {:string, null: true} + end + end +end diff --git a/test/plexus/ratings_test.exs b/test/plexus/ratings_test.exs index 74046653..b261e3c2 100644 --- a/test/plexus/ratings_test.exs +++ b/test/plexus/ratings_test.exs @@ -10,7 +10,6 @@ defmodule Plexus.RatingsTest do @invalid_attrs %{ app_package: "", app_build_number: nil, - app_version: nil, rating_type: nil, rom_name: nil, rom_build: nil, @@ -64,6 +63,24 @@ defmodule Plexus.RatingsTest do assert rating.rom_build == "some ROM build" end + test "handles optional app_version" do + app = app_fixture() + + valid_attrs = %{ + app_package: app.package, + android_version: "some android_version", + app_build_number: 42, + app_version: nil, + rating_type: :native, + score: 3, + installation_source: "fdroid", + rom_name: "some ROM name", + rom_build: "some ROM build" + } + + assert {:ok, %Rating{}} = Ratings.create_rating(valid_attrs) + end + test "invalid data returns error changeset" do assert {:error, _reason} = Ratings.create_rating(@invalid_attrs) 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 fdd369da..62b98336 100644 --- a/test/plexus_web/controllers/api/v1/rating_controller_test.exs +++ b/test/plexus_web/controllers/api/v1/rating_controller_test.exs @@ -76,6 +76,25 @@ defmodule PlexusWeb.API.V1.RatingControllerTest do } = json_response(conn, 200)["data"] end + test "handles null/empty app_verison", %{conn: conn} do + %{package: app_package} = app_fixture() + attrs = Map.put(@create_attrs, :app_package, app_package) + + conn = + post(conn, ~p"/api/v1/apps/#{app_package}/ratings", + rating: Map.put(attrs, :app_version, nil) + ) + + assert %{"app_version" => ""} = json_response(conn, 201)["data"] + + conn = + post(conn, ~p"/api/v1/apps/#{app_package}/ratings", + rating: Map.put(attrs, :app_version, "") + ) + + assert %{"app_version" => ""} = json_response(conn, 201)["data"] + end + test "renders errors when data is invalid", %{conn: conn} do app = app_fixture() conn = post(conn, ~p"/api/v1/apps/#{app}/ratings", rating: @invalid_attrs) @@ -84,7 +103,6 @@ defmodule PlexusWeb.API.V1.RatingControllerTest do "errors" => %{ "android_version" => ["can't be blank"], "app_build_number" => ["can't be blank"], - "app_version" => ["can't be blank"], "rom_name" => ["can't be blank"], "rom_build" => ["can't be blank"], "installation_source" => ["can't be blank"],