Skip to content

Commit

Permalink
Remove app_version Requirement for App Rating (#368)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
tomkonidas authored Sep 18, 2024
1 parent 7d2c15c commit 5d977e6
Show file tree
Hide file tree
Showing 7 changed files with 50 additions and 7 deletions.
2 changes: 1 addition & 1 deletion lib/plexus/ratings.ex
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
Expand Down
3 changes: 1 addition & 2 deletions lib/plexus/schemas/rating.ex
Original file line number Diff line number Diff line change
Expand Up @@ -27,15 +27,14 @@ defmodule Plexus.Schemas.Rating do
@required [
:android_version,
:app_package,
:app_version,
:app_build_number,
:rom_name,
:rom_build,
:rating_type,
:installation_source,
:score
]
@optional [:notes]
@optional [:app_version, :notes]
@doc false
def changeset(%Rating{} = rating, attrs) do
rating
Expand Down
2 changes: 1 addition & 1 deletion lib/plexus_web/controllers/api/v1/rating_controller.ex
Original file line number Diff line number Diff line change
Expand Up @@ -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]},
Expand Down
2 changes: 1 addition & 1 deletion lib/plexus_web/controllers/api/v1/rating_json.ex
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
Original file line number Diff line number Diff line change
@@ -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
19 changes: 18 additions & 1 deletion test/plexus/ratings_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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
Expand Down
20 changes: 19 additions & 1 deletion test/plexus_web/controllers/api/v1/rating_controller_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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"],
Expand Down

0 comments on commit 5d977e6

Please sign in to comment.