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

TP2000-1660: Add staleness checking to missing measures check #1400

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

eadpearce
Copy link
Contributor

TP2000-1660: Add staleness checking to missing measures check

Why

  • The missing measures check should be invalidated when the contents of the workbasket change

What

  • If a missing measures check has been run, checks the state of commodity and measure objects in the workbasket and compares it to the hash stored on the check. If they are different, the check is invalidated and must be run again

Checklist

  • Requires migrations? ✅
  • Requires dependency updates? ❌
Screenshot 2025-01-29 at 11 32 51

@eadpearce eadpearce requested a review from a team as a code owner January 29, 2025 11:38
Comment on lines 89 to +91
{% else %}


{% if missing_measures_check.hash != workbasket.commodity_measure_changes_hash %}
Copy link
Contributor

Choose a reason for hiding this comment

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

This conditional may get skipped even if the hash has changed if the last check was successful.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh yeah you're right. I'll see if I can rewrite this template to be a bit less confusing

Comment on lines 89 to +91
{% else %}


{% if missing_measures_check.hash != workbasket.commodity_measure_changes_hash %}
Copy link
Contributor

Choose a reason for hiding this comment

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

Might this staleness check be useful as a boolean property?

Comment on lines +473 to +477
changes = [
item
for item in self.tracked_models.all().order_by("pk")
if isinstance(item, GoodsNomenclature) or isinstance(item, Measure)
]
Copy link
Contributor

Choose a reason for hiding this comment

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

The queryset here and in def has_commodities() could be filtered for subclasses:

qs = self.tracked_models.filter(
    Q(instance_of=GoodsNomenclature) | Q(instance_of=Measure)
).order_by("pk")

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's much neater thank you!

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