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

custom change does not work without require_atomic? set to false #1770

Closed
msonawane opened this issue Feb 6, 2025 · 5 comments
Closed

custom change does not work without require_atomic? set to false #1770

msonawane opened this issue Feb 6, 2025 · 5 comments
Labels
bug Something isn't working needs review

Comments

@msonawane
Copy link

Describe the bug
a custom change does not update tags attribute unless require_atomic? is set to false

To Reproduce
Action, attribute and change

attribute :tags, {:array, :string} do
      allow_nil? true
      default []
      public? true
      description "The tags for organization"
    end


update :add_tags do
      description "add tags"
      accept [:*]
      argument :tags_to_add, {:array, :string}, allow_nil?: false
      require_atomic? false
      change {Manwa.Changes.AppendTags, attribute: :tags}
     end

defmodule Manwa.Changes.AppendTags do
  @moduledoc false
  use Ash.Resource.Change

  @impl true
  def init(opts) do
    if is_atom(opts[:attribute]) do
      {:ok, opts}
    else
      {:error, "attribute must be an atom!"}
    end
  end

  @impl true
  def atomic(_changeset, _opts, _context) do
    :ok
  end

  @impl true
  def change(changeset, opts, _context) do
    old_value = Ash.Changeset.get_data(changeset, opts[:attribute])
    {:ok, new_value} = Ash.Changeset.fetch_argument(changeset, :tags_to_add)
    new_value = old_value ++ new_value
    c = Ash.Changeset.force_change_attribute(changeset, opts[:attribute], new_value)
    dbg(c, structs: false)
  end
end


iex(158)> Ash.Changeset.for_update(org, :add_tags, %{tags_to_add: ["munni"]}) |> Ash.update
[lib/manwa/changes/tags.ex:24: Manwa.Changes.AppendTags.change/3]
new_value #=> ["one", "three", "munni"]

[debug] QUERY OK source="organizations" db=11.7ms idle=1332.6ms
UPDATE "organizations" AS o0 SET "updated_at" = $1::timestamp::timestamp WHERE (o0."archived_at"::timestamp IS NULL) AND (o0."id"::uuid = $2::uuid) RETURNING o0."id", o0."name", o0."abbr", o0."is_group", o0."domain", o0."date_of_establishment", o0."logo", o0."monthly_sales_target", o0."description", o0."phone", o0."fax", o0."email", o0."website", o0."registration_number", o0."trade_name", o0."tax_registration_number", o0."account_manager_id", o0."metadata", o0."tags", o0."created_at", o0."updated_at", o0."parent_id", o0."org_type_id", o0."created_by_id", o0."updated_by_id", o0."archived_at" [~U[2025-02-04 10:11:09.814191Z], "0194bd09-a2d6-7863-9e59-6967f75a4dad"]

Expected behavior
it should update the tags attribute. update statement should have tags column

Runtime

  • Elixir version: 1.18
  • Erlang version: 27
  • OS: ubuntu
  • Ash version: 3.4.62
  • :ash_postgres, "2.5.0"

Additional context
Add any other context about the problem here.

@msonawane msonawane added bug Something isn't working needs review labels Feb 6, 2025
@sevenseacat
Copy link
Contributor

This makes sense to me - did you get a warning that the change couldn't be run atomically?

@zachdaniel
Copy link
Contributor

OHHHH. I totally missed this. Your atomic callback should be removed. By defining the callback that way, you're saying "doing this atomically means doing nothing".

What you would do instead is something like:

  @impl true
  def atomic(_changeset, _opts, _context) do
    %{tags: expr(some_postgres_fragment_that_adds_to_your_tags_list}
  end

@msonawane
Copy link
Author

This

  def atomic(changeset, opts, _context) do
    {:ok, new_value} = Ash.Changeset.fetch_argument(changeset, :tags_to_add)
   
    {:atomic,
     %{
       opts[:attribute] => expr(fragment("array_cat(tags, ?)", ^new_value))
     }}
  end

errors out

** (Ash.Error.Invalid)
Invalid Error

* Change Manwa.Changes.AppendTags must be atomic, but could not be done atomically: Cannot atomically update Manwa.Orgs.Organization.tags: Cannot cast a non-literal list atomically
  (ash 3.4.62) lib/ash/error/changes/invalid_changes.ex:4: Ash.Error.Changes.InvalidChanges.exception/1
  (ash 3.4.62) lib/ash/changeset/changeset.ex:5985: Ash.Changeset.add_error/3
  (ash 3.4.62) lib/ash/changeset/changeset.ex:2487: anonymous fn/6 in Ash.Changeset.run_action_changes/6
  (elixir 1.18.1) lib/enum.ex:2546: Enum."-reduce/3-lists^foldl/2-0-"/3
  (ash 3.4.62) lib/ash/changeset/changeset.ex:2485: Ash.Changeset.run_action_changes/6
  (ash 3.4.62) lib/ash/changeset/changeset.ex:1958: Ash.Changeset.do_for_action/4
  (manwa 0.1.0) deps/ash/lib/ash/code_interface.ex:886: Manwa.Orgs.add_tags!/3
  (elixir 1.18.1) src/elixir.erl:386: :elixir.eval_external_handler/3
  (stdlib 6.2) erl_eval.erl:919: :erl_eval.do_apply/7
  (stdlib 6.2) erl_eval.erl:663: :erl_eval.expr/6
  (elixir 1.18.1) src/elixir.erl:364: :elixir.eval_forms/4
  (elixir 1.18.1) lib/module/parallel_checker.ex:120: Module.ParallelChecker.verify/1
  (iex 1.18.1) lib/iex/evaluator.ex:336: IEx.Evaluator.eval_and_inspect/3
  (iex 1.18.1) lib/iex/evaluator.ex:310: IEx.Evaluator.eval_and_inspect_parsed/3
  (iex 1.18.1) lib/iex/evaluator.ex:299: IEx.Evaluator.parse_eval_inspect/4
  (iex 1.18.1) lib/iex/evaluator.ex:189: IEx.Evaluator.loop/1
  (iex 1.18.1) lib/iex/evaluator.ex:34: IEx.Evaluator.init/5
  (stdlib 6.2) proc_lib.erl:329: :proc_lib.init_p_do_apply/3
    (ash 3.4.62) lib/ash/error/invalid.ex:3: Ash.Error.Invalid.exception/1
    (ash 3.4.62) /home/m/work/manwa/backend/manwa/deps/splode/lib/splode.ex:264: Ash.Error.to_class/2
    (ash 3.4.62) lib/ash/error/error.ex:108: Ash.Error.to_error_class/2
    (ash 3.4.62) lib/ash/actions/update/update.ex:20: Ash.Actions.Update.run/4
    (ash 3.4.62) lib/ash.ex:2699: Ash.update/3
    (ash 3.4.62) lib/ash.ex:2639: Ash.update!/3
    iex:28: (file)

at

def cast_atomic_array(new_value, _constraints) do

  def cast_atomic_array(new_value, _constraints) do
        {:not_atomic, "Cannot cast a non-literal list atomically"}
      end

@zachdaniel
Copy link
Contributor

Two things:

  • don't use a literal reference, will cause query composition problems
  • try {:atomic, ...} and see how you do
  def atomic(changeset, opts, _context) do
    {:ok, new_value} = Ash.Changeset.fetch_argument(changeset, :tags_to_add)
   
    {:atomic,
     %{
       opts[:attribute] => {:atomic, expr(fragment("array_cat(?, ?)", tags, ^new_value))}
     }}
  end

@msonawane
Copy link
Author

that worked thanks
this is the change which worked

defmodule Manwa.Changes.AppendTags do
  @moduledoc false
  use Ash.Resource.Change

  @impl true
  def init(opts) do
    if is_atom(opts[:attribute]) do
      {:ok, opts}
    else
      {:error, "attribute must be an atom!"}
    end
  end

  @impl true
  def atomic(changeset, opts, _context) do
    {:ok, new_value} = Ash.Changeset.fetch_argument(changeset, :tags_to_add)
    {:atomic,
     %{
       opts[:attribute] => {:atomic, expr(fragment("array_cat(?, ?)", tags, ^new_value))}
     }}
  end

end


Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working needs review
Projects
None yet
Development

No branches or pull requests

3 participants