Creating forms with change manage_relationship not loading values

I have a resource TodoTask that has a many-to-many with a Context like so

# in todo_task
    relationships do
      has_many :context_relationships, TodoTaskContext do
        destination_attribute :task_id
      end

      many_to_many :contexts, Context do
        join_relationship :context_relationships
        source_attribute_on_join_resource :task_id
        destination_attribute_on_join_resource :context_id
      end
    end

I’m trying to set up the form and change so that all existing Contexts are listed as checkboxes, and when the form is submitted, there will be a TodoTaskContext created for this TodoTask and Context. I have this working correctly in a unit test with a manage_relationship that looks like this:

  # in todo_task
     update :update do
        accept ...
        argument :contexts, {:array, :string}, allow_nil?: true, default: []
        require_atomic? false

        change manage_relationship(:contexts, :context_relationships,
                 value_is_key: :context_id,
                 type: :direct_control
               )
      end

and I’m creating a form in my LV like so

AshPhoenix.Form.for_update(task, :update, as: "task")

But the related contexts aren’t being loaded. When I log out my @form[:contexts], the value is always an empty list.

%{
  id: "task_contexts",
  name: "task[contexts]",
  value: [],
  __struct__: Phoenix.HTML.FormField,
  field: :contexts,
  errors: []
}

I’ve made sure my relationships are loaded just before creating AshPhoenix.Form.for_update, but that doesn’t seem to change anything:

    task = task |> Ash.load!([:contexts, :context_relationships])

Is there something wrong with how I’ve got my manage_relationship set up? Or is there something else I need to add?

Thanks!

:thinking: that should work. Have you confirmed that there are contexts loaded on the task after you do that? Do you have policies that could be filtering the reads on contexts etc.?

Nope, no policies here. The contexts and context_relationships are indeed being loaded.

This is where I’m at right now:

    task = Ash.load!(task, [:contexts, :context_relationships])

    # contexts are being loaded and are not empty
    assert is_list(task.contexts)
    assert is_list(task.context_relationships)
    refute task.contexts == []
    refute task.context_relationships == []

    # create ash form, check the data
    ash_form = task |> AshPhoenix.Form.for_update(:update, as: "task")

    assert ash_form.source.data == task
    assert ash_form.data == task
    assert ash_form.original_data == task
    
    # create the phoenix form 
    phx_form = ash_form |> Phoenix.Component.to_form()

    # fetch the contexts field as I would in the template
    field = phx_form[:contexts]

    assert is_list(field.value)
    refute field.value == [] # this is where the test fails

The only thing I’m seeing that doesn’t seem quite right is the relationships and arguments field of the ash changeset. I’m not sure if arguments should have an empty list:

# IO.inspect(ash_form.source)
#Ash.Changeset<
  domain: TaskApp.TodoTasks,
  action_type: :update,
  action: :update,
  attributes: %{},
  relationships: %{
    context_relationships: [
      {[],
       [
         debug?: false,
         ignore?: false,
         on_missing: :destroy,
         on_match: :update,
         on_lookup: :ignore,
         on_no_match: :create,
         eager_validate_with: false,
         authorize?: true,
         meta: [id: :contexts],
         value_is_key: :context_id,
         type: :direct_control
       ]}
    ]
  },
  arguments: %{contexts: []},
  errors: [],
  data: %TaskApp.TodoTasks.TodoTask{
    ...
    context_relationships: [
      %TaskApp.TodoTasks.TodoTaskContext{ ... },
      %TaskApp.TodoTasks.TodoTaskContext{ ... }
    ],
    contexts: [
      %TaskApp.TodoTasks.Context{ ... },
      %TaskApp.TodoTasks.Context{ ... }
    ],
  },
  valid?: true
>

Ah, I think it might be the value_is_key setting? Can you try removing that?

I think that is what’s behind it. I’ve updated my action, removing value_is_key. I also had to update the type:

       update :update do
         accept [:title, :status, :body]
-        argument :contexts, {:array, :string}, allow_nil?: true, default: []
+        argument :contexts, {:array, :map}, allow_nil?: true, default: []
         require_atomic? false
 
         change manage_relationship(:contexts, :context_relationships,
-                 value_is_key: :context_id,
                  type: :direct_control
                )
       end

Now the field.value is a list of Phoenix.HTML.Form structs with data for each of the existing TodoTaskContexts (the resource we’re joining through).

Is this a bug in creating the form? Or is it just that using value_is_key means that it’s expecting context_relationships to be a list of those ids insteat of the actual structs?

I’m using value_is_key because the form is just iterating over the existing Contexts that are available and creating a checkbox group, so the data that’s being submitted is a list of the ids. If there’s a way to tweak that so it plays nice with the way Ash handles forms like this, I’m all ears.

Yeah, this seems like a bug of some kind to me. Could you create a reproduction and open an issue on ash_phoenix?

Will do.

Edit: done. Added here with a link to a GH repo reproducing it.

Looking at your response in GH, it does seem like adding that option auto?: [include_non_map_types?: true does at least handle the specific issue of field.value always being empty.

That being said, even with those values present, I’m still having an issue getting this wired up.

      #  task actions
      update :update do
        accept [:title, :status, :body]
        argument :contexts, {:array, :string}, allow_nil?: true, default: []
        require_atomic? false

        change manage_relationship(:contexts, :context_relationships,
                 value_is_key: :context_id,
                 type: :direct_control
               )
      end
          # in my form component 
          AshPhoenix.Form.for_update(task, :update,
            as: "task",
            auto?: [include_non_map_types?: true]
          ) 

And in the markup itself:

        # inside a simple_form :
        <.input
          field={@form[:contexts]}
          type="select"
          multiple
          prompt="select "
          label="Contexts"
          options={@contexts |> Enum.map(fn opt -> {opt.name, opt.id} end)}
        />

This is submitting correctly, I can see in my logs the contexts with the ids.

Parameters: %{"task" => %{ ..., "contexts" => ["1", "2"] }}

The issue I’m still having is when the form is rendered, the relationships that currently exist in the values aren’t showing up as selected.

I’m starting to think I might just be about this in the wrong way. I feel like this should be a fairly common form pattern with a many-to-many relationship like this. Like updating a Team to have multiple Users. Is there an obvious approach here that I’m missing?

I’m still fairly new to Ash so the help is much appreciated.

That doesn’t look like enough to make those things show up as selected to me.

 options={@contexts |> Enum.map(fn opt -> {opt.name, opt.id} end)}

ahhh, okay. So what is happening here is that the argument itself doesn’t contain a value that corresponds to what is currently selected. The value of @form[:contexts] is now a list of nested forms not a list value? Honestly its been a while since I looked at this part of the code, sorry :sweat_smile:

Try doing something like this:

        <.input
          field={@form[:contexts]}
          value={Enum.map(...)} # the selected values
          type="select"
          multiple
          prompt="select "
          label="Contexts"
          options={@contexts |> Enum.map(fn opt -> {opt.name, opt.id} end)}
        />

Yup, the value of @form[:contexts] is a list of Phoenix.HTML.Form:

    [
      %Phoenix.HTML.Form{
        data: %TodoTaskContext{task_id: ^task_id, context_id: ^context_1_id}
      },
      %Phoenix.HTML.Form{
        data: %TodoTaskContext{task_id: ^task_id, context_id: ^context_2_id}
      }
    ]

So I can map the input value like this

        <.input
          field={@form[:contexts]}
          value={transform_value(@form[:contexts])}
          options={@contexts |> Enum.map(fn opt -> {opt.name, opt.id} end)}
          ...
        />
  # elsewhere       
  defp transform_value(field) do
    for %Phoenix.HTML.Form{data: %TodoTaskContext{context_id: context_id}} <- field.value do
      context_id
    end
  end

or even hard-code it like so value={[1, 2]}, but when it comes time to submit the form, it triggers this warning

warning: Got non-map params at path: task[contexts][0]. Form and nested form params must be maps.

So it seems like there’s two things that might not be working together correctly.

  1. If I leave out the nested form config, I can properly submit my form, BUT the intial field value is always an empty list:
# form
          form = AshPhoenix.Form.for_update(task, :update, as: "task")
# render
        <.input
          field={@form[:contexts]}
          type="select"
          multiple
          label="Contexts"
          options={@contexts |> Enum.map(fn opt -> {opt.name, opt.id} end)}
        />
  1. If I add the nested form config, I can have the values present, (although I do need to map them) BUT I can’t actually submit my form.
#form
          AshPhoenix.Form.for_update(task, :update,
            as: "task",
            forms: [
              auto?: [include_non_map_types?: true]
            ]
          )
# render
        <.input
          field={@form[:contexts]}
          value={transform_value(@form[:contexts])}
          type="select"
          multiple
          label="Contexts"
          options={@contexts |> Enum.map(fn opt -> {opt.name, opt.id} end)}
        />
# elsewhere       
  defp transform_value(field) do
    for %Phoenix.HTML.Form{data: %TodoTaskContext{context_id: context_id}} <- field.value do
      context_id
    end
  end

What’s interesting is that I can hard code the value like value={[1,2]} and submit it fine, and all I change is adding the nested form config, and then I get the Got non-map params warning again.

@matt-savvy I think the best thing from this point might actually be to make a small reproduction project that we can look at, and/or add some tests for this behavior to AshPhoenix.Form. Then we can just get it all fixed in one fell swoop, and maybe even write a guide on it :smiley:

I think the best thing from this point might actually be to make a small reproduction project that we can look at

Yup, I can extend the project I linked yesterday.

and/or add some tests for this behavior to AshPhoenix.Form

I did try to do that, but since the test resources in AshPhoenix are all using the ETS data layer, I was running into some differences right away. Maybe I can give that another shot.

We can start w/ the project and worry about the tests later :smiley:

Too late, I think I got the tests showing the issue

LMK if this is clear.

Will look shortly, thanks for the test!

Okay, so, I’ve modified the tests you provided a bit. Ultimately I think what you want to do here is not use the include_non_map_types? option. It is a bit of a red herring actually.

Ultimately, the argument is disconnected from the underlying value in this kind of update, so I think you want to fill out the options yourself.

Something like this:

# form
          form = AshPhoenix.Form.for_update(task, :update, as: "task")
# render
        <.input
          field={@form[:contexts]}
          value={Enum.map(@form.source.source.data.contexts, &(&1.id))}
          type="select"
          multiple
          label="Contexts"
          options={@contexts |> Enum.map(fn opt -> {opt.name, opt.id} end)}
        />

That seems to do the trick!

  # render
        <.input
          field={@form[:contexts]}
          value={form_context_value(@form)}
          type="select"
          multiple
          label="Contexts"
          options={@contexts |> Enum.map(fn opt -> {opt.name, opt.id} end)}
        />

  # elsewhere
  defp form_context_value(form) do
    case form.source.source.data.contexts do
      %Ash.NotLoaded{} -> [] # needed for the for_create form
      contexts -> Enum.map(contexts, & &1.id)
    end
  end

Oh snap, just saw that. Happy to help, wasn’t really expecting that to land on main. So I’m assuming the related code change actually just prevents the BadMapError.

Thanks for all the help digging into this issue. At this point, I think I at least have a little better understanding, so I’ll probably tinker with my code and see if there seems to be an easy way to set this up so that we don’t need to manually transform the changeset data to get the values.

1 Like

Actually, just found one issue with this approach …

Changing any other field triggers the validation. So if I’ve updated the context, and then I make any other change, the contexts get reset.

For example, load a task that belongs to context 1 and 2 → update the context so it’s only 2 → update the status field → validation triggered, resetting the context to 1 and 2 (the original values).