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

Allow custom DELETE logic #311

Open
sssilver opened this issue May 3, 2014 · 14 comments
Open

Allow custom DELETE logic #311

sssilver opened this issue May 3, 2014 · 14 comments

Comments

@sssilver
Copy link

sssilver commented May 3, 2014

In many cases, one might not want DELETE calls to actually issue a deletion of the record from the database, instead it would be desirable to set an is_deleted attribute to True or something similar.

Currently flask-restless does not seem to allow this, which makes it too rigid to be used in critical applications like financial software, where persistent deletion of records is very uncommon.

@jfinkels
Copy link
Owner

jfinkels commented May 9, 2014

This is a good idea, I think.

However, this would require the user to write the entire code, thereby making Flask-Restless essentially pointless for this particular HTTP method. If we allowed that, would it make sense to allow the user to specify entirely customized code for every method?

@sssilver
Copy link
Author

sssilver commented May 9, 2014

Well, that would certainly be a good start. We already have the request preprocessors and postprocessors, an optional processor override would be great. I don't think it'd draw Flask-Restless useless for several reasons:

  1. There are still all the other methods
  2. There is still the regular DELETE for the models that don't require soft deletion
  3. Flask-Restless still imposes good RESTful practice

Right now I'm having to write a custom SQLAlchemy session and a custom Base-derived model to inherit my models from, and it's too much effort for what could have been a single simple function.

Another way to approach this would be to have the user put all the custom code in the preprocessor, and then not actually process the default deletion if the custom preprocessor returns False or something.

@jfinkels
Copy link
Owner

jfinkels commented May 9, 2014

I don't like that alternate approach, it seems too...complicating from the
user's perspective. Swapping out processing functionality is, I suppose,
cleaner, both intuitively and from an implementation perspective. I guess
also that this generalizes the current default functionality, which could
then simply be provided as the default "processing" function. However, then
we lose some of the value (the simplicity) of having a single class-based
implementation. Another tradeoff...

@sssilver
Copy link
Author

sssilver commented May 9, 2014

I agree on swapping out processing functionality.

Do you want me to refactor it for myself, then submit a pull request, or would you prefer this to be developed in some other way?

@sssilver
Copy link
Author

Another problem of course is, once you do the custom DELETE, then you have to do the custom GET for the listings, since you don't want entries where is_deleted = True to be fetched in the listing.

How to automate this reasonably? I have no idea at this point.

@Diaoul
Copy link

Diaoul commented May 27, 2014

I am interested in custom implementation of some methods too.
What about decorators within the model?

from flask.ext.restless import override
[...]

class User(Base):
    [...]
    @override('DELETE')
    def my_custom_delete([... something here?]):
        pass

@jfinkels
Copy link
Owner

jfinkels commented Jun 4, 2014

@Diaoul I prefer for the model definition to be independent of (the controller) Flask-Restless.

@sssilver How about for now providing a much smaller level of customization to the user. For example:

manager.create_api(MyModel, methods=['GET', 'DELETE'], on_delete_set_attr='is_deleted')

If the keyword argument on_delete_set_attr is not set, then do what we are doing currently. If it is set to the string 'is_deleted', then do setattr(MyModel, 'is_deleted', True).

@Diaoul
Copy link

Diaoul commented Jun 4, 2014

In my case, I have an Icon model that has a path column. I'd like the file pointed by the path column to be deleted too in case the model gets deleted.
It seemed to me that the "model" was the proper place for this logic in OOP but I'm open to alternative implementations.

@tonnydourado
Copy link

+1 for implementing this functionality. I think that maybe allowing overriding the default behaviour by passing custom functions to the api manager constructor would fit better the current way things are done (pre and post processor, enabled methods, included methods, are all provided that way)

@sschendel
Copy link

+1 for changes to enable soft deletes.

@da4089
Copy link

da4089 commented Mar 1, 2015

+1 for allowing a generic override of a method. I've found myself wanting this several times.

If the default behaviour was exposed, then the current pre- and post- processor customisations could be replaced by the method override, where you can do whatever you like before and/or after calling the default method implementation.

@CaptObvious
Copy link

Is there any plan to implement this?

I've had to work around this for soft DELETEs by doing it in the preprocessor method then doing:

raise ProcessingException(code=200)

to prevent any further processing. This is a really hacky workaround and I'd much prefer to have an officially supported way of overriding methods.

@Dasio
Copy link

Dasio commented Mar 23, 2017

Any info ?

@JBrVJxsc
Copy link

Is there any plan to implement this?

I've had to work around this for soft DELETEs by doing it in the preprocessor method then doing:

raise ProcessingException(code=200)

to prevent any further processing. This is a really hacky workaround and I'd much prefer to have an officially supported way of overriding methods.

I like this idea, but the code should be 204 for a successfully deletion right?

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

No branches or pull requests

9 participants