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

For hooks, "was" should track change since last save, not initial value #141

Open
swehba opened this issue Nov 3, 2023 · 5 comments
Open
Labels
bug Something isn't working

Comments

@swehba
Copy link

swehba commented Nov 3, 2023

As it now stands, a django-lifecycle hook such as AFTER_UPDATE with was and changed_to parameters, compares the current value of a field to the initial value, not the last saved value. I'm not sure if this changed, but in its current form, it is not very useful. It would be much better to compare to the last saved value.

  • Did this feature change?
  • Is there a reason it doesn't compare to the last saved value?
  • Is there a different mechanism for tracking since the last save instead of the initial value?
@EnriqueSoria
Copy link
Collaborator

Hi, could you provide an example of how would you expect this to work and how it actually behaves? That would help a lot.

@EnriqueSoria EnriqueSoria added the question Further information is requested label Nov 4, 2023
@swehba
Copy link
Author

swehba commented Nov 4, 2023

"""
Here is an example. I didn't actually run the code, but I am confident it behaves as illustrated.
"""
from django.db import models
from django_lifecycle import hook, AFTER_UPDATE, LifecycleModel

class Color(models.IntegerChoices):
    GREEN = 1
    YELLOW = 2
    RED = 3

class TrafficLight(LifecycleModel):
    color = models.IntegerField(choices=Color, default=Color.RED)

    def slow_down(self) -> None:
        if self.color != Color.GREEN:
            return
        self.color = Color.YELLOW
        self.save()

    def stop(self) -> None:
        if self.color != Color.YELLOW:
            return
        self.color = Color.YELLOW
        self.save()

    def go(self) -> None:
        if self.color != Color.RED:
            return
        self.color = Color.RED
        self.save()

    @hook(AFTER_UPDATE, was=Color.RED, is_now=Color.GREEN)
    def on_red_to_green(self):
        print('Go.')

    @hook(AFTER_UPDATE, was=Color.GREEN, is_now=Color.YELLOW)
    def on_green_to_yellow(self):
        print('Slow down.')

    @hook(AFTER_UPDATE, was=Color.YELLOW, is_now=Color.RED)
    def on_yellow_to_red(self):
        print('Stop!')


def test_traffic_light() -> None:
    traffic_light = TrafficLight()
    traffic_light.go()
    traffic_light.slow_down()
    traffic_light.stop()

    """
    I expect to see the following output:
    
        Go.
        Slow down.
        Stop!
        
    But I think I will see the following:
    
        Go.
        
    That is because LifecycleModel is only tracking changes in the color property between what
    it was when the model was first loaded into memory and what it now is.
    
    However, if I do the following...
    """

    traffic_light = TrafficLight()
    traffic_light.go()
    traffic_light = TrafficLight.objects.last()
    traffic_light.slow_down()
    traffic_light = TrafficLight.objects.last()
    traffic_light.stop()

    """
    ...I will see what I expected, namely:
    
        Go.
        Slow down.
        Stop!
    
    That's because the model is being loaded/initialized before each time the color property is
    modified.
    """


if __name__ == '__main__':
    test_traffic_light()

@EnriqueSoria
Copy link
Collaborator

Thanks for the example. I will test it when I have time, but it should work as you expected.

We've recently made a change that involved refreshing initial state on a transaction.on_commit. Maybe if you do all of this inside a transaction it doesn't work properly.
Have to check this.

@EnriqueSoria EnriqueSoria added bug Something isn't working and removed question Further information is requested labels Nov 4, 2023
@swehba
Copy link
Author

swehba commented Nov 6, 2023

We have unit tests which all used to pass, but recently they started failing. I am guessing the behavior changed as some point so that the previous field values were retained only when the in-memory model was instantiated, not after each save.

Also, these models are not using transactions. So, I don't think that applies.

@chrisimcevoy
Copy link

Hitting the same problem after upgrading django-lifecycle from 1.0.0 to 1.2.4.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants