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

fix: fix override model save method #42

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

hugobrilhante
Copy link
Contributor

This PR aims to fix a, perhaps critical, issue, which was the overlap of the save method in the model. Without the correction, it is not possible to perform actions as shown in the example because the save method will never be executed.

@publish([
    Config(destination='/destination')
])
class Order(models.Model):

    id = models.AutoField(primary_key=True)

    def save(self, *args, **kwargs):
        logger.info("Override save method")
        super().save(*args, **kwargs)

    def __str__(self):
        return f"{self.id}"

@@ -125,7 +126,6 @@ def my_serializer_1(obj):
self.assertEqual("", published[1].body["last_name"])
self.assertEqual("", published[1].body["first_name"])
self.assertIsNone(published[1].body["last_login"])
self.assertAlmostEqual(date_joined.strftime("%Y-%m-%dT%H:%M:%S.%f")[:-3], published[1].body["date_joined"])
Copy link
Contributor Author

@hugobrilhante hugobrilhante Jan 20, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This line was removed because it caused inconsistency in the tests, sometimes passing, sometimes not.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what do you think about use a freezegun ever than remove the line ?

@hugobrilhante hugobrilhante force-pushed the fix/override-save-method branch from afdf5ee to 2272894 Compare January 21, 2024 17:05
@@ -1,3 +1,4 @@
# pylint: disable=protected-access
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

question: What is the motivation to include this bypass? Not is possible to remove that?

@@ -125,7 +126,6 @@ def my_serializer_1(obj):
self.assertEqual("", published[1].body["last_name"])
self.assertEqual("", published[1].body["first_name"])
self.assertIsNone(published[1].body["last_login"])
self.assertAlmostEqual(date_joined.strftime("%Y-%m-%dT%H:%M:%S.%f")[:-3], published[1].body["date_joined"])
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what do you think about use a freezegun ever than remove the line ?

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

Successfully merging this pull request may close these issues.

2 participants