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 hook before and after delete a file #577

Closed
wants to merge 7 commits into from
Closed

Add hook before and after delete a file #577

wants to merge 7 commits into from

Conversation

SamyLegal
Copy link

Add possibility, for the localfile field, of specify a pre and/or post hook when we delete a file.

When i upload an image on my website with the localfile field, i generate multiple images of differents size. For that, i use post move hook of localfile. I have add a pre/post delete hook to delete images i have generate, when i delete the original file.

@grabbou
Copy link
Contributor

grabbou commented Aug 26, 2014

Hi @SamyLegal , thanks for your PR!

It's a nice feature to add to our 'before-React' releases as hooks will come out of the box with our next major release. I left my private comments on lines you've made (especially due to code-style guide we have here.

I am a huge fun of being consistent so that's why I'd feel more enthusiastic about that if all of our fields have the same list of hooks available but we are going to change that mechanism anyway in the nearest future.

Let's wait for the others (especially for @JedWatson) and see what we can do.

@SamyLegal
Copy link
Author

Hi,

Thank you for the answer.

I'm agree with you for use dot notation instead bracket notation.
I have used bracket notation because 'delete' is a reserved word in javascript (jshint give me an error) and i have used this word because you use it in your code.
I can use another word like 'remove' or if you prefer i can remove quotes too, tell-me.

Otherwise, i have add comments.

Thanks.

@morenoh149
Copy link
Contributor

this could be useful

@creynders
Copy link
Contributor

Hi @SamyLegal sorry for the very long wait. Just a quick update: I've rewritten the entire hooking mechanism for keystone, which will land in v0.4. This means that your PR would need a number of changes to be up-to-date. We're starting assembly of v0.4 changes today or tomorrow, so I'm going to take this PR along partially and modify it to use the new hooking system.

@creynders creynders added the v0.4 label Mar 26, 2015
@creynders creynders mentioned this pull request Mar 26, 2015
@JedWatson JedWatson modified the milestone: 0.4.0 Apr 28, 2015
@JedWatson JedWatson removed the v0.4 label Apr 28, 2015
@JedWatson
Copy link
Member

Closing in line with @creynders note - this has been superseded. Thanks for the original PR @SamyLegal!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants