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

SoftDeletesMixin doesn't work as expected when trying to change the column name. #613

Closed
JRubics opened this issue Mar 29, 2022 · 4 comments · Fixed by #616
Closed

SoftDeletesMixin doesn't work as expected when trying to change the column name. #613

JRubics opened this issue Mar 29, 2022 · 4 comments · Fixed by #616
Labels
bug An existing feature is not working as intended

Comments

@JRubics
Copy link
Contributor

JRubics commented Mar 29, 2022

SoftDeletesMixin doesn't work as expected when trying to change the column name.

When we try to add __deleted_at__ = 'archived_at' to the Model class, it changes the value of deleted_at field on the class but it is not used later when creating a query. I think the reason is this in masoniteorm.models.Model, method boot, this line:

if class_name.endswith("Mixin"):
       getattr(base_class(), "boot_" + class_name)(self.builder)

because it doesn't use previously changed mixin, but creates a new one.

I think it can be solved like this:

if class_name.endswith("Mixin"):
       getattr(self, "boot_" + class_name)(self.builder)

but i am not sure if it will break some other mixin.

Can you please tell me if this change is safe to add, and I will create a PR if it is. If not, do you have some other solution to my problem?

Desktop:

OS: Linux
Version Ubuntu 21.10

What database are you using?

Type: [MySQL]
Version [14.14 Distrib 5.7.30, for Linux (x86_64) using EditLine wrapper]
Masonite ORM [2.4.2]
@JRubics JRubics added the bug An existing feature is not working as intended label Mar 29, 2022
@Marlysson
Copy link
Contributor

Marlysson commented Mar 29, 2022

@JRubics Do you inherit your model class from SoftDeletesMixin like this:

class User(Model, SoftDeletesMixin):

Maybe the error is here:

__deleted_at__ = "deleted_at"

With delete_at column defined hardcoded instead get from model located in the builder.

You can create a PR with some tests and run them safely that we will analyze and just merged with everything ok.

@JRubics
Copy link
Contributor Author

JRubics commented Mar 29, 2022

@Marlysson I did and exactly this is the problem because boot method in Model class creates a new instance of the SoftDeletesMixin that doesn't contain my change but only hardcoded value. Okay, I'll try tomorrow :)

@josephmancuso
Copy link
Member

This is fixed in latest 2.5.0 version

@JRubics
Copy link
Contributor Author

JRubics commented Mar 30, 2022

Thank you! 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug An existing feature is not working as intended
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants