Ash Elixir•3y ago

Action macro with Ash queries

hey Zack, in the #ideas channel, you gave the idea of creating a macro in case I want to move an action logic to another module https://discord.com/channels/711271361523351632/712035333432147989/1075932613061116036 I did that and it seems to work great, but when I add some Ash.Query macros to it, then it stops working. The obvious reason should be that the macro is not finding require Ash.Query but I can't find a way to add it to make it work. This is what I have right now:

defmodule Marketplace.Markets.Property.Offer.Actions.MakeOwnOffersOld do
  @moduledoc false

  defmacro read(name: name) do
    quote location: :keep do
      read unquote(name) do
        argument :property_id, :uuid, allow_nil?: false

        filter expr(
                 status == :submitted and offeror_id == ^actor(:id) and
                   property_id == ^arg(:property_id)
               )

        manual fn _, query, _ ->
          {_, updated_offers} = Marketplace.Repo.update_all(query, set: [status: :old])

          {:ok, updated_offers}
        end
      end
    end
  end
end

defmodule Marketplace.Markets.Property.Offer.Actions.MakeOwnOffersOld do
  @moduledoc false

  defmacro read(name: name) do
    quote location: :keep do
      read unquote(name) do
        argument :property_id, :uuid, allow_nil?: false

        filter expr(
                 status == :submitted and offeror_id == ^actor(:id) and
                   property_id == ^arg(:property_id)
               )

        manual fn _, query, _ ->
          {_, updated_offers} = Marketplace.Repo.update_all(query, set: [status: :old])

          {:ok, updated_offers}
        end
      end
    end
  end
end

defmodule Marketplace.Markets.Property.Offer.Actions do
  alias Marketplace.Markets.Property.Offer.Actions

  defmacro __using__(_opts) do
    quote location: :keep do
      require Actions.MakeOwnOffersOld
    end
  end
end

defmodule Marketplace.Markets.Property.Offer.Actions do
  alias Marketplace.Markets.Property.Offer.Actions

  defmacro __using__(_opts) do
    quote location: :keep do
      require Actions.MakeOwnOffersOld
    end
  end
end

defmodule Marketplace.Markets.Property.Offer do
  alias Marketplace.Markets.Property.Offer.Actions

  use Actions

  use Ash.Resource,
    ...

  actions do
    Actions.MakeOwnOffersOld.read(name: :make_own_offers_old)
  end

  ...
end

defmodule Marketplace.Markets.Property.Offer do
  alias Marketplace.Markets.Property.Offer.Actions

  use Actions

  use Ash.Resource,
    ...

  actions do
    Actions.MakeOwnOffersOld.read(name: :make_own_offers_old)
  end

  ...
end

32 Replies

BlibsOP•3y ago

This will fail to compile with this error:

==> marketplace
Compiling 14 files (.ex)
warning: variable "status" does not exist and is being expanded to "status()", please use parentheses to remove the ambiguity or change the variable name
  lib/marketplace/markets/property/offer/actions/make_own_offer_old.ex:10: Marketplace.Markets.Property.Offer


== Compilation error in file lib/marketplace/markets/property/offer.ex ==
** (CompileError) lib/marketplace/markets/property/offer/actions/make_own_offer_old.ex:10: undefined function status/0 (there is no such import)
    (ash 2.6.11) expanding macro: Ash.Expr.expr/1

==> marketplace
Compiling 14 files (.ex)
warning: variable "status" does not exist and is being expanded to "status()", please use parentheses to remove the ambiguity or change the variable name
  lib/marketplace/markets/property/offer/actions/make_own_offer_old.ex:10: Marketplace.Markets.Property.Offer


== Compilation error in file lib/marketplace/markets/property/offer.ex ==
** (CompileError) lib/marketplace/markets/property/offer/actions/make_own_offer_old.ex:10: undefined function status/0 (there is no such import)
    (ash 2.6.11) expanding macro: Ash.Expr.expr/1

Any idea on how to solve this?

ZachDaniel•3y ago

You may need to add import Ash.Expr inside of your macro

BlibsOP•3y ago

Unfortunately that will not work because expr will conflict with another one already imported:

==> marketplace
Compiling 6 files (.ex)

== Compilation error in file lib/marketplace/markets/property/offer.ex ==
** (CompileError) lib/marketplace/markets/property/offer/actions/make_own_offer_old.ex:15: function expr/1 imported from both Ash.Filter.TemplateHelpers and Ash.Expr, call is ambiguous
    (spark 0.4.5) expanding macro: Spark.Dsl.Extension.set_entity_opt/4
    lib/marketplace/markets/property/offer.ex:35: Marketplace.Markets.Property.Offer (module)
    (ash 2.6.11) expanding macro: Ash.Resource.Dsl.Actions.Read.Options.filter/1
    lib/marketplace/markets/property/offer.ex:35: Marketplace.Markets.Property.Offer (module)
    (ash 2.6.11) expanding macro: Ash.Resource.Dsl.Actions.Read.read/2
    lib/marketplace/markets/property/offer.ex:35: Marketplace.Markets.Property.Offer (module)
    expanding macro: Marketplace.Markets.Property.Offer.Actions.MakeOwnOffersOld.read/1
    lib/marketplace/markets/property/offer.ex:35: Marketplace.Markets.Property.Offer (module)

==> marketplace
Compiling 6 files (.ex)

== Compilation error in file lib/marketplace/markets/property/offer.ex ==
** (CompileError) lib/marketplace/markets/property/offer/actions/make_own_offer_old.ex:15: function expr/1 imported from both Ash.Filter.TemplateHelpers and Ash.Expr, call is ambiguous
    (spark 0.4.5) expanding macro: Spark.Dsl.Extension.set_entity_opt/4
    lib/marketplace/markets/property/offer.ex:35: Marketplace.Markets.Property.Offer (module)
    (ash 2.6.11) expanding macro: Ash.Resource.Dsl.Actions.Read.Options.filter/1
    lib/marketplace/markets/property/offer.ex:35: Marketplace.Markets.Property.Offer (module)
    (ash 2.6.11) expanding macro: Ash.Resource.Dsl.Actions.Read.read/2
    lib/marketplace/markets/property/offer.ex:35: Marketplace.Markets.Property.Offer (module)
    expanding macro: Marketplace.Markets.Property.Offer.Actions.MakeOwnOffersOld.read/1
    lib/marketplace/markets/property/offer.ex:35: Marketplace.Markets.Property.Offer (module)

ZachDaniel•3y ago

Try importing Ash.Expr in the containing module And then saying unquote(expr(…))? Actually that might not work

BlibsOP•3y ago

Yeah, I was trying to play around with that but can't make that work 😢

ZachDaniel•3y ago

I’ll have to take a look tomorrow

BlibsOP•3y ago

No problem, enjoy the weekend, we can look into this on Monday 🙂

kernel•3y ago

mmm, can't you import it at the start of your macro, then import it again with :only keyword to exclude it again? pretty sure it will be available between those two things then so something like this

import Ash.Expr
my_awesome_macro_code
import Ash.Expr, only: []

import Ash.Expr
my_awesome_macro_code
import Ash.Expr, only: []

kernel•3y ago

barnabasj•3y ago

@Blibs were you able to make this work?

BlibsOP•3y ago

Not really. In the end, what I did was keep the definitions of the actions in the resource file but move all changes from it as separated modules to make the action smaller and simpler. Something like this:

alias Marketplace.Accounts.User.Actions

...

  update :confirm_phone_number do
    alias Actions.ConfirmPhoneNumber.{Changes, Validations}

    accept []

    argument :code, :string, allow_nil?: false

    validate Validations.UserDontHaveAnOrganization

    change Changes.LoadPhoneNumber
    change Changes.VerifyCode

    change set_attribute(:confirmed_at, DateTime.utc_now())
  end

alias Marketplace.Accounts.User.Actions

...

  update :confirm_phone_number do
    alias Actions.ConfirmPhoneNumber.{Changes, Validations}

    accept []

    argument :code, :string, allow_nil?: false

    validate Validations.UserDontHaveAnOrganization

    change Changes.LoadPhoneNumber
    change Changes.VerifyCode

    change set_attribute(:confirmed_at, DateTime.utc_now())
  end

and the actions modules are like this:

defmodule Marketplace.Accounts.User.Actions.ConfirmPhoneNumber.Changes.VerifyCode do
  @moduledoc """
  Load the user phone number
  """

  alias Marketplace.Services

  alias Ash.Error.Changes.InvalidAttribute

  use Ash.Resource.Change

  def change(changeset, _opts, _context) do
    Ash.Changeset.before_action(
      changeset,
      fn changeset ->
        ...
      end,
      append?: true
    )
  end
end

defmodule Marketplace.Accounts.User.Actions.ConfirmPhoneNumber.Changes.VerifyCode do
  @moduledoc """
  Load the user phone number
  """

  alias Marketplace.Services

  alias Ash.Error.Changes.InvalidAttribute

  use Ash.Resource.Change

  def change(changeset, _opts, _context) do
    Ash.Changeset.before_action(
      changeset,
      fn changeset ->
        ...
      end,
      append?: true
    )
  end
end

It is not perfect, but at least is better than having everything inside the action definition and having a bigger and more polluted (IMO) resource.

barnabasj•3y ago

I like this for changes that are reusable. If its something resource specific or a manual action I'm not happy with splitting up the definition of the arguments and the logic itself. I would really prefer to have it all in one place in that case. Nothing major in the grand scheme of things though

ZachDaniel•3y ago

Lemme take a look at the expr issues Alright, here is what I'm looking at:

defmodule MyApp.Resource.Actions.SomeAction do
  use Spark.Dsl.Fragment, of: Ash.Resource

  actions do
    action :some_action do
      argument ....
      manual ....
    end
  end
end

defmodule MyApp.Resource do
  use Ash.Resource, fragments: [
    MyApp.Resource.Actions.SomeAction
  ]

  ...
end

defmodule MyApp.Resource.Actions.SomeAction do
  use Spark.Dsl.Fragment, of: Ash.Resource

  actions do
    action :some_action do
      argument ....
      manual ....
    end
  end
end

defmodule MyApp.Resource do
  use Ash.Resource, fragments: [
    MyApp.Resource.Actions.SomeAction
  ]

  ...
end

How do we feel about using fragments in cases where we want to split a resource up?

barnabasj•3y ago

You could essentially do this with all sections right?

ZachDaniel•3y ago

Yeah, this would work for essentially anything It will merge sections but "options" would overwrite eachother (which I know isn't always clear what is what on the tin) but like

resource do
  description "description"
end

resource do
  description "description"
end

that in a fragment would end up with one description and the others being overwritten

barnabasj•3y ago

Feels very powerful. I probably have to restrain myself from overusing it 😉. Would it be possible to emit a warning if something gets overwritten, without a big compile time hit?

ZachDaniel•3y ago

Hmm...yeah, actually I think so. could probably even tell you which fragment did it

barnabasj•3y ago

Should make it pretty safe, all other compile time checks still apply too, right?

ZachDaniel•3y ago

Yep 🙂 It should be equivalent in all other ways as if you had just put all the fragments at the top of the resource

barnabasj•3y ago

Do we lose any language server features by splitting things up like that?

ZachDaniel•3y ago

The autocomplete extension works in fragments, had to make a small adjustment for it to work but confirmed it works

barnabasj•3y ago

Amazing 🤩 can the of option also be a custom extended Ash.Resource? Something that configures some extensions by default like a BaseResource

ZachDaniel•3y ago

I'm pretty sure that will be impossible, unfortunately

barnabasj•3y ago

But if I have a custom extension could i declare it as of or can i not use this feature with custom extensions?

ZachDaniel•3y ago

I don't think it would be necessary you can say of: Ash.Resource and then use YourApp.Resource, fragments: [Fragment]

barnabasj•3y ago

Sorry, I think I didn't communicate that clearly, can i make a fragment for AshGraphql.Resource for example?

ZachDaniel•3y ago

Ah, okay yeah I see. This is how it works currently

use Spark.Dsl.Fragment, of: Ash.Resource, extensions: [AshGraphql.Resource]

use Spark.Dsl.Fragment, of: Ash.Resource, extensions: [AshGraphql.Resource]

and then a resource has the extensions that the sum of all of its fragments have

barnabasj•3y ago

I know what I'm doing tomorrow 😆

ZachDaniel•3y ago

I'll need to run it by some people and make sure this this is the right way to go about it, but this isn't the first time people have wanted to split up their resources 😆 The main thing that I don't like about this pattern is that it doesn't support any kind of dynamic thing. Which may actually just be better.

barnabasj•3y ago

Don't want to make it too powerfull

ZachDaniel•3y ago

Yeah, agreed. I think static is better, and if you want to do this to organize a resource then thats fine, but more complex things and you can write an extension this is just like...a better option for composing a resource than macros, basically

barnabasj•3y ago

feels a lot cleaner

Gaming

Programming

Action macro with Ash queries

Did you find this page helpful?