Skip to content

Conversation

@radimvaculik
Copy link
Contributor

Closes #276.

@radimvaculik radimvaculik force-pushed the feature/has-many-remove-all branch 2 times, most recently from 607cd60 to cd07a7d Compare December 21, 2025 16:51
@radimvaculik radimvaculik force-pushed the feature/has-many-remove-all branch from cd07a7d to f6a8cb9 Compare December 21, 2025 17:40
Comment on lines +10 to +11
START TRANSACTION;
COMMIT;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is no DELETE. Do you know why? @hrach

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:-O I do not. Hopefully, you have not discovered a bug.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks buggy to me.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh no, this is a real bug that has been hidden for many years.

Expected behavior:

  • throw an exception, that (in this case) the TagFollowe::$author cannot be null, or
  • delete the TagFollower if and only if the relationship is marked with a correct, currently non-existing, cascade marker, e.g., removeOrphan.

The second option is tracked in #205 and #278. Sadly, the current behavior does not rigger exception (either missing unsupported removeOrphan || non-nullable).

Supporting removeOrphan is a bit more difficult - we need to rewrite PersistenceHelper & RemovalHelper -> join the implementation together -> since the wanted behavior is in RemovalHelper, but needs to be triggered from PersistenceHelper.

As we are in the 5.1 RC phase, I would rather incline to trigger the exception. The working workaround is to call the TagFollower repository's remove().

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.

Add HasMany::clear() method.

2 participants