Eduardo B. Alexandre:

hey Zack, in the <#712035333432147989> 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 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

Eduardo B. Alexandre:

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

Any idea on how to solve this?

ZachDaniel:

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

Eduardo B. Alexandre:

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)

ZachDaniel:

Try importing Ash.Expr in the containing module

ZachDaniel:

And then saying unquote(expr(…)) ?

ZachDaniel:

Actually that might not work

Eduardo B. Alexandre:

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

ZachDaniel:

I’ll have to take a look tomorrow

Eduardo B. Alexandre:

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

kernel:

mmm, can’t you import it at the start of your macro, then import it again with :only keyword to exclude it again?

kernel:

pretty sure it will be available between those two things then

kernel:

so something like this

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

kernel:

barnabasj:

<@816769011533742100> were you able to make this work?

Eduardo B. Alexandre:

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

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

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:

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:

Lemme take a look at the expr issues

ZachDaniel:

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

ZachDaniel:

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

barnabasj:

You could essentially do this with all sections right?

ZachDaniel:

Yeah, this would work for essentially anything

ZachDaniel:

It will merge sections

ZachDaniel:

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

ZachDaniel:

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

barnabasj:

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:

Hmm…yeah, actually I think so.

ZachDaniel:

could probably even tell you which fragment did it

barnabasj:

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

ZachDaniel:

Yep 🙂

ZachDaniel:

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

barnabasj:

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

ZachDaniel:

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

ZachDaniel:

but confirmed it works

barnabasj:

Amazing 🤩

barnabasj:

can the of option also be a custom extended Ash.Resource?

barnabasj:

Something that configures some extensions by default like a BaseResource

ZachDaniel:

I’m pretty sure that will be impossible, unfortunately

barnabasj:

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

ZachDaniel:

I don’t think it would be necessary

ZachDaniel:

you can say of: Ash.Resource and then use YourApp.Resource, fragments: [Fragment]

barnabasj:

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

ZachDaniel:

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

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

ZachDaniel:

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

barnabasj:

I know what I’m doing tomorrow 😆

ZachDaniel:

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:

Don’t want to make it too powerfull

ZachDaniel:

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

ZachDaniel:

this is just like…a better option for composing a resource than macros, basically

barnabasj:

feels a lot cleaner