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

Add more relation events #537

Closed
wants to merge 29 commits into from
Closed

Add more relation events #537

wants to merge 29 commits into from

Conversation

mjauvin
Copy link
Contributor

@mjauvin mjauvin commented Oct 27, 2020

@mjauvin mjauvin marked this pull request as draft October 27, 2020 19:32
@mjauvin mjauvin changed the title [wip] Add more relation events Add more relation events Oct 27, 2020
@Rike-cz
Copy link
Contributor

Rike-cz commented Nov 1, 2020

One thing to attach/detach event. I thought that it was written previously like you can halt whole procedure through false value returned on before*** event. It's not possible now?

And do you plan to implement HasOneOrMany, AttachOneOrMany, MorphOneOrMany?

@mjauvin
Copy link
Contributor Author

mjauvin commented Nov 1, 2020

One thing to attach/detach event. I thought that it was written previously like you can halt whole procedure through false value returned on before*** event. It's not possible now?

@LukeTowers requested this to be changed. The way to halt these is to throw an exception now.

And do you plan to implement HasOneOrMany, AttachOneOrMany, MorphOneOrMany?

For which events? I thought you said you were now thinking this was not required, did I read that wrong?

ref.

And here is the question. To have inversional events for these relationships or to call inversional counterparts above when needed? When needed means add/remove methods where relationship arises.
If is it good idea to have them here too (I don't think so now)...

@mjauvin mjauvin marked this pull request as ready for review November 1, 2020 14:34
@Rike-cz
Copy link
Contributor

Rike-cz commented Nov 1, 2020

@mjauvin I was thinking, but last word has @LukeTowers. Can you tell us, what do you think about HasOneOrMany, AttachOneOrMany, MorphOneOrMany events, @LukeTowers? Useless?

Edit: I thought about two approaches to HasOneOrMany etc. and definitelly yes, add/remove-events is good and more clean idea.

src/Database/Relations/MorphTo.php Outdated Show resolved Hide resolved
@mjauvin
Copy link
Contributor Author

mjauvin commented Nov 17, 2020

This is ready guys.

@github-actions
Copy link

This pull request will be closed and archived in 3 days, as there has been no activity in the last 60 days.
If this is still being worked on, please respond and we will re-open this pull request.
If this pull request is critical to your business, consider joining the Premium Support Program where a Service Level Agreement is offered.

@mjauvin
Copy link
Contributor Author

mjauvin commented Feb 7, 2021

Can this be added to Milestone 1.1.2?

@LukeTowers LukeTowers added this to the v1.1.3 milestone Feb 7, 2021
@LukeTowers
Copy link
Contributor

Will add it to 1.1.3 instead

Co-authored-by: Ben Thomson <git@alfreido.com>
@daftspunk
Copy link
Member

Still some work needed on this, in particular, adherence to the developer guide and before events should halt to meet continuity with the rest of the platform

Overall looks good though, will be available in next release. Thanks @mjauvin 🙏

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