improvement: remove relationship writability, as it all happens through arguments now

improvement: repurpose `writable?` on `belongs_to` to make the attribute writable
This commit is contained in:
Zach Daniel 2022-07-05 08:16:38 -04:00
parent 15cd3fb7bc
commit ac4590a0ca
14 changed files with 91 additions and 106 deletions

View file

@ -351,30 +351,6 @@ defmodule Ash.Changeset do
end end
@for_create_opts [ @for_create_opts [
relationships: [
type: :any,
doc: """
customize relationship behavior.
By default, any relationships are ignored. There are three ways to change relationships with this function:
### Action Arguments (preferred)
Create an argument on the action and add a `Ash.Resource.Change.Builtins.manage_relationship/3` change to the action.
### Overrides
You can pass the `relationships` option to specify the behavior. It is a keyword list of relationship and either
* one of the preset manage types: #{inspect(@manage_types)}
* explicit options, in the form of `{:manage, [...opts]}`
```elixir
Ash.Changeset.for_create(MyResource, :create, params, relationships: [relationship: :append, other_relationship: {:manage, [...opts]}])
```
You can also use explicit calls to `manage_relationship/4`.
"""
],
require?: [ require?: [
type: :boolean, type: :boolean,
default: false, default: false,
@ -532,7 +508,7 @@ defmodule Ash.Changeset do
|> set_tenant(opts[:tenant] || changeset.tenant) |> set_tenant(opts[:tenant] || changeset.tenant)
|> Map.put(:__validated_for_action__, action.name) |> Map.put(:__validated_for_action__, action.name)
|> Map.put(:action, action) |> Map.put(:action, action)
|> cast_params(action, params, opts) |> cast_params(action, params)
|> set_argument_defaults(action) |> set_argument_defaults(action)
|> run_action_changes(action, opts[:actor]) |> run_action_changes(action, opts[:actor])
|> add_validations() |> add_validations()
@ -560,7 +536,7 @@ defmodule Ash.Changeset do
|> set_tenant(opts[:tenant] || changeset.tenant || changeset.data.__metadata__[:tenant]) |> set_tenant(opts[:tenant] || changeset.tenant || changeset.data.__metadata__[:tenant])
|> Map.put(:action, action) |> Map.put(:action, action)
|> Map.put(:__validated_for_action__, action.name) |> Map.put(:__validated_for_action__, action.name)
|> cast_params(action, params || %{}, opts) |> cast_params(action, params || %{})
|> set_argument_defaults(action) |> set_argument_defaults(action)
|> validate_attributes_accepted(action) |> validate_attributes_accepted(action)
|> require_values(action.type, false, action.require_attributes) |> require_values(action.type, false, action.require_attributes)
@ -767,7 +743,7 @@ defmodule Ash.Changeset do
end end
end end
defp cast_params(changeset, action, params, opts) do defp cast_params(changeset, action, params) do
changeset = %{ changeset = %{
changeset changeset
| params: Map.merge(changeset.params || %{}, Enum.into(params || %{}, %{})) | params: Map.merge(changeset.params || %{}, Enum.into(params || %{}, %{}))
@ -785,24 +761,6 @@ defmodule Ash.Changeset do
changeset changeset
end end
rel = Ash.Resource.Info.public_relationship(changeset.resource, name) ->
if rel.writable? do
behaviour = opts[:relationships][rel.name]
case behaviour do
nil ->
changeset
type when is_atom(type) ->
manage_relationship(changeset, rel.name, value, type: type)
{:manage, manage_opts} ->
manage_relationship(changeset, rel.name, value, manage_opts)
end
else
changeset
end
true -> true ->
changeset changeset
end end
@ -1568,11 +1526,6 @@ defmodule Ash.Changeset do
* belongs_to - an update action on the source resource * belongs_to - an update action on the source resource
""" """
], ],
relationships: [
type: :any,
default: [],
doc: "A keyword list of instructions for nested relationships."
],
error_path: [ error_path: [
type: :any, type: :any,
doc: """ doc: """
@ -1778,15 +1731,6 @@ defmodule Ash.Changeset do
add_error(changeset, error) add_error(changeset, error)
%{writable?: false} = relationship ->
error =
InvalidRelationship.exception(
relationship: relationship.name,
message: "Relationship is not editable"
)
add_error(changeset, error)
relationship -> relationship ->
if relationship.cardinality == :many && is_map(input) && !is_struct(input) do if relationship.cardinality == :many && is_map(input) && !is_struct(input) do
case map_input_to_list(input) do case map_input_to_list(input) do

View file

@ -159,7 +159,18 @@ defmodule Ash.Dsl do
defmacro __after_compile__(_, _) do defmacro __after_compile__(_, _) do
quote do quote do
Ash.Dsl.Extension.run_after_compile() transformers_to_run =
@extensions
|> Enum.flat_map(& &1.transformers())
|> Ash.Dsl.Transformer.sort()
|> Enum.filter(& &1.after_compile?())
__MODULE__
|> Ash.Dsl.Extension.run_transformers(
transformers_to_run,
Module.get_attribute(__MODULE__, :ash_dsl_config),
false
)
end end
end end

View file

@ -484,23 +484,6 @@ defmodule Ash.Dsl.Extension do
end end
end end
defmacro run_after_compile do
quote do
transformers_to_run =
@extensions
|> Enum.flat_map(& &1.transformers())
|> Ash.Dsl.Transformer.sort()
|> Enum.filter(& &1.after_compile?())
__MODULE__
|> Ash.Dsl.Extension.run_transformers(
transformers_to_run,
Module.get_attribute(__MODULE__, :ash_dsl_config),
false
)
end
end
def run_transformers(mod, transformers, ash_dsl_config, store?) do def run_transformers(mod, transformers, ash_dsl_config, store?) do
Enum.reduce_while(transformers, ash_dsl_config, fn transformer, dsl -> Enum.reduce_while(transformers, ash_dsl_config, fn transformer, dsl ->
result = result =

View file

@ -10,6 +10,7 @@ if Code.ensure_loaded?(ElixirSense.Plugin) do
alias ElixirSense.Providers.Suggestion.Matcher alias ElixirSense.Providers.Suggestion.Matcher
def suggestions(hint, {_, function_call, arg_index, info}, _chain, opts) do def suggestions(hint, {_, function_call, arg_index, info}, _chain, opts) do
opts = add_module_store(opts)
option = info.option || get_option(opts.cursor_context.text_before) option = info.option || get_option(opts.cursor_context.text_before)
if option do if option do
@ -20,6 +21,7 @@ if Code.ensure_loaded?(ElixirSense.Plugin) do
end end
def suggestions(hint, opts) do def suggestions(hint, opts) do
opts = add_module_store(opts)
option = get_section_option(opts.cursor_context.text_before) option = get_section_option(opts.cursor_context.text_before)
if option do if option do
@ -29,6 +31,22 @@ if Code.ensure_loaded?(ElixirSense.Plugin) do
end end
end end
# For some reason, the module store does not change when modules are defined
# so we are building our own fresh copy here. This is definitely a performance
# hit
defp add_module_store(opts) do
Map.put(opts, :module_store, ElixirSense.Core.ModuleStore.build(all_loaded()))
end
defp all_loaded do
:code.all_loaded()
|> Enum.filter(fn
{mod, _} when is_atom(mod) -> true
_ -> false
end)
|> Enum.map(&elem(&1, 0))
end
def get_suggestions(hint, opts, opt_path \\ [], type \\ nil) do def get_suggestions(hint, opts, opt_path \\ [], type \\ nil) do
with true <- Enum.any?(opts.env.attributes, &(&1.name == :ash_is)), with true <- Enum.any?(opts.env.attributes, &(&1.name == :ash_is)),
dsl_mod when not is_nil(dsl_mod) <- dsl_mod when not is_nil(dsl_mod) <-
@ -587,7 +605,14 @@ if Code.ensure_loaded?(ElixirSense.Plugin) do
end) end)
|> Enum.uniq() |> Enum.uniq()
|> Enum.map(&Module.concat([&1])) |> Enum.map(&Module.concat([&1]))
|> Enum.filter(&Code.ensure_loaded?/1) |> Enum.filter(fn module ->
try do
Code.ensure_loaded?(module)
rescue
_ ->
false
end
end)
end end
end end
end end

View file

@ -30,6 +30,41 @@ if Code.ensure_loaded?(ElixirSense.Plugin) do
else else
[] []
end end
|> Enum.reject(fn
%{name: name} when name in ["__info__", "module_info", "module_info"] ->
true
_ ->
false
end)
|> Enum.map(fn
%{type: :function, origin: origin, name: name, arity: arity} = completion ->
try do
{:docs_v1, _, _, _, _, _, functions} = Code.fetch_docs(Module.concat([origin]))
new_summary =
Enum.find_value(functions, fn
{{:function, func_name, func_arity}, _, _,
%{
"en" => docs
}, _} ->
if to_string(func_name) == name && func_arity == arity do
docs
end
_other ->
false
end)
%{completion | summary: new_summary || completion.summary}
rescue
_e ->
completion
end
other ->
other
end)
custom = custom =
for module <- module_store.by_behaviour[behaviour] || [], for module <- module_store.by_behaviour[behaviour] || [],
@ -51,6 +86,8 @@ if Code.ensure_loaded?(ElixirSense.Plugin) do
builtins ++ custom builtins ++ custom
end end
defp lowercase_string?(""), do: true
defp lowercase_string?(string) do defp lowercase_string?(string) do
first = String.first(string) first = String.first(string)
String.downcase(first) == first String.downcase(first) == first

View file

@ -24,8 +24,8 @@ defmodule Ash.Resource.Change.Builtins do
If a zero argument function is provided, it is called to determine the value. If a zero argument function is provided, it is called to determine the value.
If a tuple of `{:arg, :argument_name}` is provided, the value will be read from the argument if supplied. If a `arg(:arg_name)` is provided, the value will be read from the argument if supplied.
If the argument is not supplied then nothing happens. If the argument specified is not given to the action, then nothing happens.
""" """
def set_attribute(attribute, value) do def set_attribute(attribute, value) do
{Ash.Resource.Change.SetAttribute, attribute: attribute, value: value} {Ash.Resource.Change.SetAttribute, attribute: attribute, value: value}

View file

@ -56,6 +56,12 @@ defmodule Ash.Resource.Relationships.BelongsTo do
@opt_schema Ash.OptionsHelpers.merge_schemas( @opt_schema Ash.OptionsHelpers.merge_schemas(
[ [
writable?: [
type: :boolean,
doc:
"Whether or not the attribute created by this relationship will be marked with `writable?: true`.",
default: false
],
primary_key?: [ primary_key?: [
type: :boolean, type: :boolean,
default: false, default: false,

View file

@ -8,7 +8,6 @@ defmodule Ash.Resource.Relationships.HasMany do
:source_field, :source_field,
:source, :source,
:context, :context,
:writable?,
:description, :description,
:filter, :filter,
:sort, :sort,
@ -28,7 +27,6 @@ defmodule Ash.Resource.Relationships.HasMany do
type: :has_many, type: :has_many,
cardinality: :many, cardinality: :many,
source: Ash.Resource.t(), source: Ash.Resource.t(),
writable?: boolean,
read_action: atom, read_action: atom,
filter: Ash.Filter.t() | nil, filter: Ash.Filter.t() | nil,
no_fields?: boolean, no_fields?: boolean,

View file

@ -9,7 +9,6 @@ defmodule Ash.Resource.Relationships.HasOne do
:private?, :private?,
:source_field, :source_field,
:allow_orphans?, :allow_orphans?,
:writable?,
:context, :context,
:description, :description,
:filter, :filter,
@ -31,7 +30,6 @@ defmodule Ash.Resource.Relationships.HasOne do
type: :has_one, type: :has_one,
cardinality: :one, cardinality: :one,
source: Ash.Resource.t(), source: Ash.Resource.t(),
writable?: boolean,
name: atom, name: atom,
read_action: atom, read_action: atom,
no_fields?: boolean, no_fields?: boolean,

View file

@ -13,7 +13,6 @@ defmodule Ash.Resource.Relationships.ManyToMany do
:not_found_message, :not_found_message,
:violation_message, :violation_message,
:api, :api,
:writable?,
:private?, :private?,
:sort, :sort,
:read_action, :read_action,
@ -30,7 +29,6 @@ defmodule Ash.Resource.Relationships.ManyToMany do
type: :many_to_many, type: :many_to_many,
cardinality: :many, cardinality: :many,
source: Ash.Resource.t(), source: Ash.Resource.t(),
writable?: boolean,
private?: boolean, private?: boolean,
filter: Ash.Filter.t() | nil, filter: Ash.Filter.t() | nil,
read_action: atom, read_action: atom,

View file

@ -26,11 +26,6 @@ defmodule Ash.Resource.Relationships.SharedOptions do
doc: doc:
"The field on this resource that should match the `destination_field` on the related resource." "The field on this resource that should match the `destination_field` on the related resource."
], ],
writable?: [
type: :boolean,
doc: "Whether or not the relationship may be edited.",
default: true
],
description: [ description: [
type: :string, type: :string,
doc: "An optional description for the relationship" doc: "An optional description for the relationship"

View file

@ -29,7 +29,7 @@ defmodule Ash.Resource.Transformers.BelongsToAttribute do
else else
not relationship.required? not relationship.required?
end, end,
writable?: false, writable?: relationship.writable?,
private?: true, private?: true,
primary_key?: relationship.primary_key? primary_key?: relationship.primary_key?
) )

View file

@ -692,18 +692,6 @@ defmodule Ash.Test.Actions.CreateTest do
end end
describe "creating with required belongs_to relationships" do describe "creating with required belongs_to relationships" do
test "allows creating with belongs_to relationship" do
author =
Author
|> new()
|> change_attribute(:bio, "best dude")
|> Api.create!()
ProfileWithBelongsTo
|> Ash.Changeset.for_create(:create, [author: author], relationships: [author: :replace])
|> Api.create!()
end
test "does not allow creating without the required belongs_to relationship" do test "does not allow creating without the required belongs_to relationship" do
assert_raise Ash.Error.Invalid, ~r/relationship author is required/, fn -> assert_raise Ash.Error.Invalid, ~r/relationship author is required/, fn ->
ProfileWithBelongsTo ProfileWithBelongsTo

View file

@ -543,7 +543,8 @@ defmodule Ash.ElixirSense.PluginTest do
snippet: nil, snippet: nil,
spec: "", spec: "",
summary: summary:
"Calls `Ash.Changeset.manage_relationship/4` with the changeset and relationship provided, using the value provided for the named argument", "Calls `Ash.Changeset.manage_relationship/4` with the changeset and relationship provided, using the value provided for the named argument" <>
_,
type: :function, type: :function,
visibility: :public visibility: :public
}, },
@ -558,7 +559,8 @@ defmodule Ash.ElixirSense.PluginTest do
snippet: nil, snippet: nil,
spec: "", spec: "",
summary: summary:
"Calls `Ash.Changeset.manage_relationship/4` with the changeset and relationship provided, using the value provided for the named argument", "Calls `Ash.Changeset.manage_relationship/4` with the changeset and relationship provided, using the value provided for the named argument" <>
_,
type: :function, type: :function,
visibility: :public visibility: :public
} }