This should not be possible, please report a detailed bug at error when error is very clear
Bug Report
Describe the Bug
When attempting to use Ash.Sort.expr_sort with a resource or relationship that does not exist, the error message suggests reporting a bug instead of providing a clear and direct error message about the invalid resource/relationship.
To Reproduce
Add the following code to a resource:
prepare build(
sort: [
Ash.Sort.expr_sort(not_existing_resource_or_relationship.attribute, :string)
]
)
And you get the error
** (Ash.Error.Framework.AssumptionFailed) Assumption failed: Listing refs in not_existing_resource_or_relationship.attribute:
Found reference :attribute at path [:not_existing_resource_or_relationship]
No related resource at path [:not_existing_resource_or_relationship] from Cvs.MatchingTable.Entry
This should not be possible, please report a detailed bug at:
https://github.com/ash-project/ash/issues/new?assignees=&labels=bug%2C+needs+review&template=bug_report.md&title=
Expected Behavior
A more specific error message pointing directly to the Ash.Sort.expr_sort call and that it is a human error. Not asying that you should report a bug here.
Runtime
[ - Elixir version](elixir: "> 1.16",) {:ash, "> 3.4.37"}, {:ash_postgres, "> 2.4.12"}, {:ash_phoenix, "> 2.0"}, {:ash_admin, "> 0.11.9"}, {:ash_authentication, "> 4.2.7"}, {:ash_authentication_phoenix, "~> 2.1.2"},
@zachdaniel Is this fixed? Seems like it is. When I try this out compile doesn't pass:
error: undefined variable "not_existing_resource_or_relationship"
│
29 │ Ash.Sort.expr_sort(not_existing_resource_or_relationship.attribute, :string)
│ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
│
== Compilation error in file lib/nutrea/food.ex ==
** (CompileError) cannot compile module (errors have been logged)
(ash 3.4.64) expanding macro: Ash.Resource.Dsl.Actions.Read.Actions.Preparations.Prepare.prepare/1
You have to first require Ash.Sort
I didn't reproduce the same error but something similar:
Unknown Error
* ** (MatchError) no match of right hand side value: {:error, "Invalid reference not_existing_resource_or_relationship.attribute"}
(ash 3.4.64) lib/ash/actions/read/read.ex:1833: anonymous fn/8 in Ash.Actions.Read.add_calc_context_to_sort/8
which happens on:
{:ok, expr} = # <- L1833
Ash.Filter.hydrate_refs(
expr,
%{
resource: resource,
public?: false,
parent_stack: opts[:parent_stack]
}
)
This happens in add_calc_context_to_sort which is called in do_read
query <- %{
query
| ...,
sort:
add_calc_context_to_sort(
query,
opts[:actor],
opts[:authorize?],
query.tenant,
opts[:tracer],
query.resource,
query.domain,
parent_stack: parent_stack_from_context(query.context)
)
}
My idea is adding some kind of validation in the with chain before: :ok <- validate_expr_path(query).
Not sure if right approach but I can't figure out where else I could plug in the error.
Yeah, that would make sense to me 😄
Okay cool. One more technical question:
What's in expression is:
type(not_existing_resource_or_relationship.attribute, Ash.Type.String, [])
And I'm not sure how to break that down. 🤔
I was thinking of using Info module for checks but i need to get from the above resource NotExistingResourceOrRelationship and attribute :attribute right? What would be the clean way of getting that (if there is a clean way 😬)?
Oh, I would not try to an analyze the expression yourself. just make sure that the error from hydrate_refs gets returned instead of a pattern match error.
Yea, that does make more sense. ⭐ Which would mean that instead of
query <- %{
query
| filter:
add_calc_context_to_filter(
query.filter,
opts[:actor],
opts[:authorize?],
query.tenant,
opts[:tracer],
query.domain,
query.resource,
parent_stack: parent_stack_from_context(query.context)
),
sort:
add_calc_context_to_sort(
query,
opts[:actor],
opts[:authorize?],
query.tenant,
opts[:tracer],
query.resource,
query.domain,
parent_stack: parent_stack_from_context(query.context)
)
I will put add_calc_context_to_filter and add_calc_context_to_sort into the with chain above and they should return {:ok filter} and {:ok, sort} and will also have their with chains.
Yea, that's juicy. 🐛 Would that be the right approach? I'll assume it is.
Yeah, that code is pretty convoluted as well, so make sure that any other usages of those functions are updated. Are they public? I forget.