Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make config raise specific domain specific error #176

Merged
merged 2 commits into from
Oct 27, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions .github/workflows/test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,12 @@ jobs:
otp: "19.3"
- elixir: "1.11"
otp: "20.0"
- elixir: "1.9"
otp: "23.0"
- elixir: "1.7"
otp: "23.0"
- elixir: "1.8"
otp: "23.0"

steps:
- name: Checkout
Expand Down
6 changes: 5 additions & 1 deletion lib/paginator.ex
Original file line number Diff line number Diff line change
Expand Up @@ -207,7 +207,11 @@ defmodule Paginator do
iex> Paginator.cursor_for_record(%Paginator.Customer{id: 1, name: "Alice"}, [id: :asc, name: :desc])
"g3QAAAACZAACaWRhAWQABG5hbWVtAAAABUFsaWNl"
"""
@spec cursor_for_record(any(), [atom() | {atom(), atom()}], (map(), atom() | {atom(), atom()} -> any())) :: binary()
@spec cursor_for_record(
any(),
[atom() | {atom(), atom()}],
(map(), atom() | {atom(), atom()} -> any())
) :: binary()
def cursor_for_record(
record,
cursor_fields,
Expand Down
14 changes: 11 additions & 3 deletions lib/paginator/config.ex
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,10 @@ defmodule Paginator.Config do
:total_count_limit
]

defmodule ArgumentError do
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fix: I don't think this is necessary since the Elixir stdlib for Exceptions already has this [1]? @ulissesalmeida: Is that correct?

[1] https://github.com/elixir-lang/elixir/blob/main/lib/elixir/lib/exception.ex#L807

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ahh yea, we should use def exception indeed 👍🏽

defexception [:message]
end

@default_total_count_primary_key_field :id
@default_limit 50
@minimum_limit 1
Expand Down Expand Up @@ -55,15 +59,19 @@ defmodule Paginator.Config do

def validate!(%__MODULE__{} = config) do
unless config.cursor_fields do
raise(ArgumentError, "expected `:cursor_fields` to be set")
raise(Paginator.Config.ArgumentError, message: "expected `:cursor_fields` to be set")
end

if !cursor_values_match_cursor_fields?(config.after_values, config.cursor_fields) do
raise(ArgumentError, message: "expected `:after` cursor to match `:cursor_fields`")
raise(Paginator.Config.ArgumentError,
message: "expected `:after` cursor to match `:cursor_fields`"
)
end

if !cursor_values_match_cursor_fields?(config.before_values, config.cursor_fields) do
raise(ArgumentError, message: "expected `:before` cursor to match `:cursor_fields`")
raise(Paginator.Config.ArgumentError,
message: "expected `:before` cursor to match `:cursor_fields`"
)
end
end

Expand Down
26 changes: 16 additions & 10 deletions test/paginator/config_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -120,17 +120,19 @@ defmodule Paginator.ConfigTest do
test "raises ArgumentError when cursor_fields are not set" do
config = Config.new([])

assert_raise ArgumentError, "expected `:cursor_fields` to be set", fn ->
assert_raise Config.ArgumentError, "expected `:cursor_fields` to be set", fn ->
Config.validate!(config)
end
end

test "raises ArgumentError when after cursor does not match the cursor_fields" do
config = Config.new(cursor_fields: [:date], after: complex_after())

assert_raise ArgumentError, "expected `:after` cursor to match `:cursor_fields`", fn ->
Config.validate!(config)
end
assert_raise Config.ArgumentError,
"expected `:after` cursor to match `:cursor_fields`",
fn ->
Config.validate!(config)
end
end

test "raises ArgumentError when after cursor does not match cursor_fields with schema" do
Expand All @@ -144,9 +146,11 @@ defmodule Paginator.ConfigTest do
})
)

assert_raise ArgumentError, "expected `:after` cursor to match `:cursor_fields`", fn ->
Config.validate!(config)
end
assert_raise Config.ArgumentError,
"expected `:after` cursor to match `:cursor_fields`",
fn ->
Config.validate!(config)
end
end

test "ok when after cursor matches cursor_fields with schema" do
Expand All @@ -166,9 +170,11 @@ defmodule Paginator.ConfigTest do
test "raises ArgumentError when before cursor does not match the cursor_fields" do
config = Config.new(cursor_fields: [:id], before: complex_before())

assert_raise ArgumentError, "expected `:before` cursor to match `:cursor_fields`", fn ->
Config.validate!(config)
end
assert_raise Config.ArgumentError,
"expected `:before` cursor to match `:cursor_fields`",
fn ->
Config.validate!(config)
end
end
end

Expand Down
4 changes: 1 addition & 3 deletions test/paginator_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -970,9 +970,7 @@ defmodule PaginatorTest do
end

for field0_order <- @available_sorting_order, field1_order <- @available_sorting_order do
test "paginates correctly when pages contains nulls - order by charged_at #{field0_order}, id #{
field1_order
}" do
test "paginates correctly when pages contains nulls - order by charged_at #{field0_order}, id #{field1_order}" do
customer = insert(:customer)

now = NaiveDateTime.utc_now()
Expand Down