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

feat: add hooks on_item_added_and_edited and on_item_added_and_edited_and_deleted #972

Open
wants to merge 2 commits into
base: develop
Choose a base branch
from

Conversation

sveneberth
Copy link
Member

I've often methods / actions that should be called after and skeleton has changes and exist (on_item_added_and_edited) or a skeleton has changes in any way (on_item_added_and_edited_and_deleted).
With these methods it's very easy to call these actions.


I know the name is long, but expressive. And to be honest, I don't have a better one. on_changed would be okay for all three cases, but how should the other one be called?

`on_item_added_and_edited_and_deleted`

I've often methods / actions  which should be called after and skeleton has
chnanges and exist (`on_item_added_and_edited`) or an skeletons has
changes in any way (`on_item_added_and_edited_and_deleted`).
With these methods it's very easy to call these actions.
@sveneberth sveneberth added feature New feature or request Priority: Medium This issue may be useful, and needs some attention. labels Dec 7, 2023
@phorward
Copy link
Member

phorward commented Dec 7, 2023

Hello @sveneberth,

Thanks for creating this pull request, and pointing out the need for such hooks. Nevertheless, I am not convinced by this implementation, as I find that this concept collides with the envisaged "ActionSkels"-paradigm we already talked about, but also with ideas regarding an improvement of the model (Skeleton-)level, see #630.

You could also think about an event bus that you can hook into for certain events. However, this all needs to be planned properly.

I will block this pull request for now, as I don't see any more hooks for actions in the VIUR system and especially the prototypes, but would rather reduce them and replace them with a more general, leaner and universal solution that fits all demans.

Please respect this for the time being. We can discuss about this in the viur-meeting tomorrow, but I see much more general work required first, to achieve this issue.

@phorward phorward added this to the ViUR-core v4.0 milestone Dec 12, 2023
phorward added a commit that referenced this pull request Jan 25, 2024
…1036)

- Provides `clone`-actions for `List` and `Tree` prototypes
- Provides `cloneSkel`, `onClone` and `onCloned` hooks; Yes, this is in
contrast to [the comment
here](#972 (comment)),
which should not allow any more hooks in ViUR-core. However, this pull
request should be seen as an intermediate step to make the feature and
its possibilities available and usable in the new admin as well as in
ViUR itself, without changing the overall concept of the system now.
This will be done in a later step (Action-Skels)
- Added and fixed some docstrings.
- Tested agains a project, using the [Vi 3.0.37 in the
`new-clone-attempt`
branch](viur-framework/viur-vi@v3.0.37...new-clone-attempt)

---------

Co-authored-by: Sven Eberth <mail@sveneberth.de>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New feature or request Priority: Medium This issue may be useful, and needs some attention.
Projects
Status: Todo
Development

Successfully merging this pull request may close these issues.

None yet

4 participants