Skip to content

189 enhancement/polymorphic on delete option needs refining and potentially moving - #920

Open
arstamm wants to merge 49 commits into
ash-project:mainfrom
arstamm:189-enhancement/polymorphic-on-delete-option-needs-refining-and-potentially-moving
Open

arstamm wants to merge 49 commits into
ash-project:mainfrom
arstamm:189-enhancement/polymorphic-on-delete-option-needs-refining-and-potentially-moving

Conversation

@arstamm

@arstamm arstamm commented Nov 10, 2026 •

Copy link
Copy Markdown
Contributor

Contributor checklist

Leave anything that you believe does not apply unchecked.

  • I accept the AI Policy, or AI was not used in the creation of this PR.
  • Bug fixes include regression tests
  • Chores
  • Documentation changes
  • Features include unit/acceptance tests
  • Refactoring
  • Update dependencies

The Problem

The options polymorphic_on_update, polymorphic_delete, and polymorphic_name are currently configured in the reference macro. The issue is that these options actually configure the polymorphic resources attached to this resource and not the resource itself. This also allows us to configure this option without attaching any polymorphic resources. This works but also makes little sense.

# Example from the reported issue

defmodule  MyApp.Location do

  use Ash.Resource, data_layer: AshPostgres.DataLayer

  postgres do
    repo MyApp.Repo
    polymorphic? true

    # HAS NO EFFECT
    references do
      polymorphic_on_delete :delete
    end
  end

  attributes do
    uuid_primary_key :id
    attribute :resource_id, :uuid do
      allow_nil? false
      private? true
    end
    attribute :x, :float
    attribute :y, :float
  end
end

Solution

A better location for these settings is within the relationship_context itself. This way, we can see more explicitly what resource these settings are being configured for.

relationships do
  has_many :location, Location do
    relationship_context %{
      data_layer: %{
        table: "event_locations",

## New References Map added to data_layer
        references: %{
          on_delete: :delete,
          on_update: :update,
          name: "event_locations_id_fkey"
        }

      }
    }   
    destination_attribute :resource_id
  end
end

This PR implements this new method while keeping the old method intact. A deprecation warning is now emitted when the old method is used. Guards and warnings have also been set in place to make sure the old and new method cannot be used at the same time for the same resource.

Tests

This PR introduces 24 new tests (8 per option). Each of the three options in question, polymorphic_on_update, polymorphic_delete, and polymorphic_name, are tested with these 8 scenarios:

  1. Using a legacy option works as expected
  2. A legacy option emits a deprecation warning when used
  3. Defining the legacy option multiple times for the same table throws an error
  4. The new option works as expected
  5. The new option used multiple times for the same table is rejected
  6. Using the new option and legacy option in the SAME source resource for the same polymorphic resource is rejected
  7. Using the new option and legacy option in DIFFERENT source resources for the same polymorphic resource is rejected
  8. Configuring nothing results in the default behavior

NOTES:

  • Before, there were no tests covering the old options. Because of this, 7 of the new tests would have already passed before changing any code. These tests are in place to ensure that the old options and new options work before and after changes.
  • Because of the amount of repetition introduced by these tests, these test are implemented with reusable macros. This way, the same code can be reused multiple times while ensuring that the tests remain separate without isolating any variables.
  • The tests above revealed that polymorphic_name was disabled in the codebase. I cannot tell if this was on purpose or not. The function was still in the codebase for it despite being used, and the regular name option in references is still operational. With that said, I decided to bring this back both for the old and new versions.

Side Effect in label.ex

In test/support/resources/label.ex, the option polymorphic_on_delete was configured, but no polymorphic tables were attached in the implementation, so this option was basically dead code. Because of the deprecation warning that is now implemented, this line of code caused errors to come up during the automated checks. This option has been removed from this file because of this.

Feel free to let me know if this file is good as is or if this should be revisited.

Summary of Changes

  • 24 New Tests
  • Deprecation warnings added to the old options
  • Making the new options operational
  • Bringing back polymorphic_name (previously disabled without explanation)
  • Updating the documentation to reflect the changes above

…lymorphic delete, update, and name configurations.
…cement/polymorphic-on-delete-option-needs-refining-and-potentially-moving
…aise errors for conflicting configurations across relationship context and legacy options.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant