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

Trigger hook on parent array property #108

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from
Draft

Conversation

undjike
Copy link

@undjike undjike commented Jun 29, 2024

This PR addresses issue #107.

It allows triggering the update hook on the parent array property when a change is made to a key.

Before :
When updating on a key, the hook is only triggered for that key.

updated([
    'profiles' => function () {
        // Not triggered
    },
    /*'profiles.0' => function () {
        // You need to explicitly listen for the key being updated.
        // There can be several keys.
        // You must listen to them all if you want to take action on any changes made to the parent array.
    }*/
]);

With this PR :
When updating on a key, the hook is triggered on the parent array, if not explicitly listening for changes to the key.

updated([
    'profiles' => function ($key) {
        // Triggered, and the modified key is passed as a parameter.
    }
]);

That said... I'm wondering whether we should make it more dynamic to handle deep subkey as well.

updated([
    'profiles.admin' => function () {
        // Triggered on change made to key 'profiles.admin.0'.
    }
]);

This PR will forward the hook to the parent property profiles not to profiles.admin.

@taylorotwell
Copy link
Collaborator

If we are going to merge this it should probably handle your last case of deep subkeys.

@taylorotwell taylorotwell marked this pull request as draft July 2, 2024 15:51
@taylorotwell
Copy link
Collaborator

Mark as ready for review again when you want me to take another look please. Thanks!

@undjike undjike marked this pull request as ready for review July 3, 2024 05:22
@undjike
Copy link
Author

undjike commented Jul 3, 2024

Added support for deep subkeys. Hope the codebase is still in line with the package behaviour.

@undjike
Copy link
Author

undjike commented Jul 6, 2024

Hello @taylorotwell any comment ?

@taylorotwell taylorotwell marked this pull request as draft July 10, 2024 19:30
@undjike
Copy link
Author

undjike commented Jul 20, 2024

@taylorotwell let me know if there is something you want me to adjust.

@alexmanase
Copy link

@undjike @taylorotwell any updates on this PR?

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.

3 participants