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

Duplicate error messages on embedded form #104

Closed
rgraff opened this issue Oct 10, 2023 · 2 comments
Closed

Duplicate error messages on embedded form #104

rgraff opened this issue Oct 10, 2023 · 2 comments
Labels
bug Something isn't working

Comments

@rgraff
Copy link
Contributor

rgraff commented Oct 10, 2023

Describe the bug
I have an action that given invalid input will generate 2 errors. But when given the same input in an AshPhoenix.Form, the same invalid input will generate each error twice.

I wrote the following test to duplicate the issue in my app.

    test "should not duplicate error messages", %{
      environment: environment,
      feature: feature,
      owner: owner
    } do
      # This assertation passes. There are 2 errors: key is required, value is required
      assert {:error,
              %Ash.Error.Invalid{
                errors: [
                  %Ash.Error.Changes.Required{
                    field: :key,
                    type: :attribute,
                    resource: MyApp.Environments.FeatureVariant.String,
                    changeset: nil,
                    query: nil,
                    error_context: [],
                    vars: [],
                    path: [:variant],
                    class: :invalid
                  },
                  %Ash.Error.Changes.Required{
                    field: :value,
                    type: :attribute,
                    resource: MyApp.Environments.FeatureVariant.String,
                    changeset: nil,
                    query: nil,
                    error_context: [],
                    vars: [],
                    path: [:variant],
                    class: :invalid
                  }
                ]
              }} =
               MyApp.Environments.Feature.add_variant(
                 feature,
                 %{
                   variant: %{
                     description: "",
                     enabled: "true",
                     key: "",
                     metadata: "{}",
                     type: "string",
                     value: ""
                   }
                 },
                 actor: owner,
                 tenant: environment.organization_id
               )

      # This should be the same as above, except going AshPhoenix.Form, but there are now 4 errors.
      assert {:error,
              %{
                source: %{
                  forms: %{
                    variant: %{
                      # submit_errors does not match.
                      # I expect 2 errors, but there are 4. Each of the 2 errors is duplicated.
                      submit_errors: [value: {"is required", []}, key: {"is required", []}]
                    }
                  }
                }
              }} =
               AshPhoenix.Form.for_update(feature, :add_variant,
                 api: MyApp.Environments,
                 as: "feature",
                 forms: [
                   auto?: true
                 ],
                 actor: owner,
                 tenant: environment.organization_id
               )
               |> Phoenix.Component.to_form()
               |> AshPhoenix.Form.add_form(:variant, params: %{"type" => "string"})
               |> AshPhoenix.Form.submit(
                 params: %{
                   "variant" => %{
                     "_form_type" => "create",
                     "_persistent_id" => "0",
                     "_touched" => "_form_type,_touched,type",
                     "description" => "",
                     "enabled" => "true",
                     "key" => "",
                     "metadata" => "{}",
                     "type" => "string",
                     "value" => ""
                   }
                 }
               )
    end

To Reproduce
I can reliably reproduce this in my app. I don't have an easy way to replicate it outside of my app though.

Expected behavior
The errors should not duplicate

** Runtime
I'm on the latest Ash Versions.

Additional context
This form has an embedded form for an argument that is an embedded resource (but not a relationship). That resource is also a union of models.

@rgraff rgraff added the bug Something isn't working label Oct 10, 2023
@zachdaniel
Copy link
Contributor

Interestinggggggg....It's hard to say what the cause is without a reproduction, but I think it's safe for us to use an Enum.uniq when generating these errors anyway. The main reason being that two error messages could theoretically be added with the same error message for the same field and we'd want to hide those anyway.

@zachdaniel
Copy link
Contributor

Should be "fixed" in main. The production of duplicate errors somewhere in the process is not ideal, but this should at least fix the UX issue that you're seeing.

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

No branches or pull requests

2 participants