Repository navigation
189 enhancement/polymorphic on delete option needs refining and potentially moving - #920
Open
arstamm wants to merge 49 commits into
Conversation
…lymorphic delete, update, and name configurations.
…pected foreign-key delete behavior."
…cement/polymorphic-on-delete-option-needs-refining-and-potentially-moving
…reign-key delete behavior.
… both old and new versions.
…result in rejection."
…ationship_context result in rejection."
…phic_on_delete results in rejection.
…es result in rejection.
…aise errors for conflicting configurations across relationship context and legacy options.
…ip_context retains default behavior.
…cement/polymorphic-on-delete-option-needs-refining-and-potentially-moving
…to .formatter.exs.
…topics/resources/polymorphic-resources.md.
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Contributor checklist
Leave anything that you believe does not apply unchecked.
The Problem
The options
polymorphic_on_update,polymorphic_delete, andpolymorphic_nameare 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.Solution
A better location for these settings is within the
relationship_contextitself. This way, we can see more explicitly what resource these settings are being configured for.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, andpolymorphic_name, are tested with these 8 scenarios:NOTES:
polymorphic_namewas 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 regularnameoption 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.exIn test/support/resources/label.ex, the option
polymorphic_on_deletewas 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