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

Cascade Delete #423

Open
ghost opened this issue Jan 9, 2020 · 5 comments
Open

Cascade Delete #423

ghost opened this issue Jan 9, 2020 · 5 comments
Labels
layer: Model type: Enhancement This intends to enhance the project.

Comments

@ghost
Copy link

ghost commented Jan 9, 2020

Is there a way to setup Redux-ORM such that related models cascade delete by default?

@haveyaseen
Copy link
Member

haveyaseen commented Jan 16, 2020

Right now the delete method mimics SQL's ON DELETE SET NULL instead of ON DELETE CASCADE.

If you take a look at this block of code it becomes clear what you need to do.

redux-orm/src/Model.js

Lines 758 to 779 in 3eccce7

_onDelete() {
const { virtualFields } = this.getClass();
// eslint-disable-next-line guard-for-in, no-restricted-syntax
for (const key in virtualFields) {
const field = virtualFields[key];
if (field instanceof ManyToMany) {
// Delete any many-to-many rows the entity is included in.
this[key].clear();
} else if (field instanceof ForeignKey) {
const relatedQs = this[key];
if (relatedQs.exists()) {
relatedQs.update({ [field.relatedName]: null });
}
} else if (field instanceof OneToOne) {
// Set null to any foreign keys or one to ones pointed to
// this instance.
if (this[key] !== null) {
this[key][field.relatedName] = null;
}
}
}
}

I haven't tested this but in theory you could extend the Model class and overwrite the delete method (or just copy it right over and prepend this in your own custom Model class).

import { Model } from "redux-orm";
import ForeignKey from "redux-orm/fields/ForeignKey";
import ManyToMany from "redux-orm/fields/ManyToMany";
import OneToOne from "redux-orm/fields/OneToOne";

class CascadingDeleteModel extends Model {
    delete() {
        const { virtualFields } = this.getClass();
        // eslint-disable-next-line guard-for-in, no-restricted-syntax
        for (const key in virtualFields) {
            const field = virtualFields[key];
            if (field instanceof ManyToMany) {
                this[key].delete();
            } else if (field instanceof ForeignKey) {
                this[key].delete();
            } else if (field instanceof OneToOne) {
                const other = this[key];
                if (other !== null) {
                    other.delete();
                }
            }
        }
        super.delete();
    }
}

It's possible that the many-to-many deletion does not work because it then can't delete the through model instances anymore (as it doesn't find any). _onDelete would be a better place for this anyways but you'll have to see.

@ghost
Copy link
Author

ghost commented Jan 28, 2020

It seems incredibly verbose and low level, and requires a lot of deciphering the source. If the object of the library is to simulate a SQL database on the frontend as a Redux store, then it should have some of these standard DELETE options built-in in my opinion.

@haveyaseen
Copy link
Member

You're right, this could be part of Redux-ORM. However I don't know what a good API would look like. Adding to the API surface area is always a dangerous move and it should have some foresight. Open to suggestions.

@haveyaseen haveyaseen added type: Enhancement This intends to enhance the project. layer: Model and removed type: Question labels Jan 28, 2020
@ghost
Copy link
Author

ghost commented Jan 28, 2020

I would probably accept another argument in the relationship where you define your relationship fields just as you would in any other ORM, backend or frontend. Personally, I come from a Django background in which the ForeignKey field accepts another model to which it refers. It optionally accepts the name of the reverse relationship and an on_delete constant such as CASCADE, SET_NULL, etc.

https://stackoverflow.com/questions/38388423/what-does-on-delete-do-on-django-models

As it now stands, the API already supports the first two arguments. Obviously easier said than done, because I certainly can't decipher the source and submit a PR, but you'd have to add that last argument for delete events. Not a related issue, but you should also decouple models from ORM instances entirely such that they can be handpicked and reused in different instances as mentioned in #417 and #422. And support a bulk fetch where you can set the model's entire QuerySet on a GET list request instead of having to manually clear if need be, followed by a forEach and create tag-team.

@ghost
Copy link
Author

ghost commented Feb 11, 2020

Any updates on this?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
layer: Model type: Enhancement This intends to enhance the project.
Projects
None yet
Development

No branches or pull requests

1 participant