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: use current() instead of approved_up_to_transaction() in codebase #1089

Open
wants to merge 11 commits into
base: master
Choose a base branch
from
Open
4 changes: 1 addition & 3 deletions additional_codes/forms.py
Original file line number Diff line number Diff line change
Expand Up @@ -155,10 +155,8 @@ def clean(self):
def save(self, commit=True):
instance = super().save(commit=False)

tx = WorkBasket.get_current_transaction(self.request)

highest_sid = (
models.AdditionalCode.objects.approved_up_to_transaction(tx).aggregate(
models.AdditionalCode.objects.current().aggregate(
Max("sid"),
)["sid__max"]
) or 0
Expand Down
9 changes: 3 additions & 6 deletions additional_codes/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -45,9 +45,8 @@ class AdditionalCodeViewSet(viewsets.ReadOnlyModelViewSet):
filter_backends = [AdditionalCodeFilterBackend]

def get_queryset(self):
tx = WorkBasket.get_current_transaction(self.request)
return (
AdditionalCode.objects.approved_up_to_transaction(tx)
AdditionalCode.objects.current()
.select_related("type")
.prefetch_related("descriptions")
)
Expand All @@ -65,8 +64,7 @@ class AdditionalCodeMixin:
model: Type[TrackedModel] = AdditionalCode

def get_queryset(self):
tx = WorkBasket.get_current_transaction(self.request)
return AdditionalCode.objects.approved_up_to_transaction(tx).select_related(
return AdditionalCode.objects.current().select_related(
"type",
)

Expand Down Expand Up @@ -208,8 +206,7 @@ class AdditionalCodeDescriptionMixin:
model: Type[TrackedModel] = AdditionalCodeDescription

def get_queryset(self):
tx = WorkBasket.get_current_transaction(self.request)
return AdditionalCodeDescription.objects.approved_up_to_transaction(tx)
return AdditionalCodeDescription.objects.current()


class AdditionalCodeDescriptionCreate(
Expand Down
8 changes: 3 additions & 5 deletions certificates/forms.py
Original file line number Diff line number Diff line change
Expand Up @@ -50,8 +50,7 @@ def __init__(self, *args, **kwargs):

def filter_certificates_for_sid(self, sid):
certificate_type = self.cleaned_data["certificate_type"]
tx = WorkBasket.get_current_transaction(self.request)
return models.Certificate.objects.approved_up_to_transaction(tx).filter(
return models.Certificate.objects.current().filter(
sid=sid,
certificate_type=certificate_type,
)
Expand All @@ -64,14 +63,13 @@ def next_sid(self, instance):
form's save() method (with its commit param set either to True or
False).
"""
current_transaction = WorkBasket.get_current_transaction(self.request)
# Filter certificate by type and find the highest sid, using regex to
# ignore legacy, non-numeric identifiers
return get_next_id(
models.Certificate.objects.filter(
models.Certificate.objects.current().filter(
sid__regex=r"^[0-9]*$",
certificate_type__sid=instance.certificate_type.sid,
).approved_up_to_transaction(current_transaction),
),
instance._meta.get_field("sid"),
max_len=3,
)
Expand Down
9 changes: 3 additions & 6 deletions certificates/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -40,9 +40,8 @@ class CertificatesViewSet(viewsets.ReadOnlyModelViewSet):
permission_classes = [permissions.IsAuthenticated]

def get_queryset(self):
tx = WorkBasket.get_current_transaction(self.request)
return (
models.Certificate.objects.approved_up_to_transaction(tx)
models.Certificate.objects.current()
.select_related("certificate_type")
.prefetch_related("descriptions")
)
Expand All @@ -60,8 +59,7 @@ class CertificateMixin:
model: Type[TrackedModel] = models.Certificate

def get_queryset(self):
tx = WorkBasket.get_current_transaction(self.request)
return models.Certificate.objects.approved_up_to_transaction(tx).select_related(
return models.Certificate.objects.current().select_related(
"certificate_type",
)

Expand Down Expand Up @@ -237,8 +235,7 @@ class CertificateDescriptionMixin:
model: Type[TrackedModel] = models.CertificateDescription

def get_queryset(self):
tx = WorkBasket.get_current_transaction(self.request)
return models.CertificateDescription.objects.approved_up_to_transaction(tx)
return models.CertificateDescription.objects.current()


class CertificateCreateDescriptionMixin:
Expand Down
22 changes: 10 additions & 12 deletions commodities/business_rules.py
Original file line number Diff line number Diff line change
Expand Up @@ -44,10 +44,10 @@ def __init__(self, transaction=None):

def parent_spans_child(self, parent, child) -> bool:
parent_validity = parent.indented_goods_nomenclature.version_at(
self.transaction,
self.transaction
).valid_between
child_validity = child.indented_goods_nomenclature.version_at(
self.transaction,
self.transaction
).valid_between
return validity_range_contains_range(parent_validity, child_validity)

Expand All @@ -60,15 +60,15 @@ def parents_span_childs_future(self, parents, child):
for parent in parents:
parents_validity.append(
parent.indented_goods_nomenclature.version_at(
self.transaction,
self.transaction
).valid_between,
)

# sort by start date so any gaps will be obvious
parents_validity.sort(key=lambda daterange: daterange.lower)

child_validity = child.indented_goods_nomenclature.version_at(
self.transaction,
self.transaction
).valid_between

if (
Expand Down Expand Up @@ -166,10 +166,10 @@ def validate(self, good):

if not (
good.code.is_chapter
or GoodsNomenclatureOrigin.objects.filter(
or GoodsNomenclatureOrigin.objects.current()
.filter(
new_goods_nomenclature__sid=good.sid,
)
.approved_up_to_transaction(good.transaction)
.exists()
):
raise self.violation(
Expand Down Expand Up @@ -252,9 +252,9 @@ class NIG11(ValidityStartDateRules):
def get_objects(self, good):
GoodsNomenclatureIndent = good.indents.model

return GoodsNomenclatureIndent.objects.filter(
return GoodsNomenclatureIndent.objects.current().filter(
indented_goods_nomenclature__sid=good.sid,
).approved_up_to_transaction(self.transaction)
)


class NIG12(DescriptionsRules):
Expand Down Expand Up @@ -305,7 +305,7 @@ def validate(self, association):
goods_nomenclature__sid=association.goods_nomenclature.sid,
valid_between__overlap=association.valid_between,
)
.approved_up_to_transaction(association.transaction)
.current()
.exclude(
id=association.pk,
)
Expand Down Expand Up @@ -351,9 +351,7 @@ def has_violation(self, good):
goods_nomenclature__sid=good.sid,
additional_code__isnull=False,
)
.approved_up_to_transaction(
self.transaction,
)
.current()
.exists()
)

Expand Down
4 changes: 1 addition & 3 deletions commodities/forms.py
Original file line number Diff line number Diff line change
Expand Up @@ -74,9 +74,7 @@ def init_fields(self):
self.fields[
"end_date"
].help_text = "Leave empty if the footnote is needed for an unlimited time"
self.fields[
"associated_footnote"
].queryset = Footnote.objects.approved_up_to_transaction(self.tx).filter(
self.fields["associated_footnote"].queryset = Footnote.objects.current().filter(
footnote_type__application_code__in=[1, 2],
)
self.fields[
Expand Down
6 changes: 3 additions & 3 deletions commodities/models/orm.py
Original file line number Diff line number Diff line change
Expand Up @@ -111,9 +111,9 @@ def get_url(self):
return reverse("commodity-ui-detail", kwargs={"sid": self.sid})

def get_dependent_measures(self, transaction=None):
return self.measures.model.objects.filter(
return self.measures.model.objects.current().filter(
goods_nomenclature__sid=self.sid,
).approved_up_to_transaction(transaction)
)

@property
def is_taric_code(self) -> bool:
Expand Down Expand Up @@ -200,7 +200,7 @@ def get_good_indents(
) -> QuerySet:
"""Return the related goods indents based on approval status."""
good = self.indented_goods_nomenclature
return good.indents.approved_up_to_transaction(
return good.indents.current(
as_of_transaction or self.transaction,
)

Expand Down
9 changes: 2 additions & 7 deletions commodities/tests/test_views.py
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ def test_commodity_list_queryset():
good_1 = factories.SimpleGoodsNomenclatureFactory.create(item_id="1010000000")
good_2 = factories.SimpleGoodsNomenclatureFactory.create(item_id="1000000000")
tx = Transaction.objects.last()
commodity_count = GoodsNomenclature.objects.approved_up_to_transaction(tx).count()
commodity_count = GoodsNomenclature.objects.current().count()
with override_current_transaction(tx):
qs = view.get_queryset()

Expand Down Expand Up @@ -522,12 +522,7 @@ def test_commodity_footnote_update_success(valid_user_client, date_ranges):
"end_date": "",
}
response = valid_user_client.post(url, data)
tx = Transaction.objects.last()
updated_association = (
FootnoteAssociationGoodsNomenclature.objects.approved_up_to_transaction(
tx,
).first()
)
updated_association = FootnoteAssociationGoodsNomenclature.objects.current().first()
assert response.status_code == 302
assert response.url == updated_association.get_url("confirm-update")

Expand Down
10 changes: 2 additions & 8 deletions commodities/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -47,11 +47,8 @@ def get_queryset(self):

Only return valid names that are products (suffix=80)
"""
tx = WorkBasket.get_current_transaction(self.request)
return (
GoodsNomenclature.objects.approved_up_to_transaction(
tx,
)
GoodsNomenclature.objects.current()
.prefetch_related("descriptions")
.as_at_and_beyond(date.today())
.filter(suffix=80)
Expand All @@ -69,10 +66,7 @@ class FootnoteAssociationMixin:
model = FootnoteAssociationGoodsNomenclature

def get_queryset(self):
tx = WorkBasket.get_current_transaction(self.request)
return FootnoteAssociationGoodsNomenclature.objects.approved_up_to_transaction(
tx,
)
return self.model.objects.current()


class CommodityList(CommodityMixin, WithPaginationListView):
Expand Down
4 changes: 3 additions & 1 deletion common/jinja2/layouts/confirm.jinja
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,9 @@
<div class="govuk-grid-column-two-thirds">
{% block panel %}{% endblock%}
<h2 class="govuk-heading-m">Next steps</h2>
<p class="govuk-body">To complete your task you must publish your change. </p>
{% block next_steps %}
<p class="govuk-body">To complete your task you must publish your change. </p>
{% endblock%}
<div>{% block main_button %}{% endblock%}</div>
{{ govukButton({
"text": "Go to your workbasket summary",
Expand Down
3 changes: 2 additions & 1 deletion common/models/tracked_qs.py
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,8 @@ def current(self) -> TrackedModelQuerySet:
)

def approved_up_to_transaction(self, transaction=None) -> TrackedModelQuerySet:
"""Get the approved versions of the model being queried, unless there
"""This function is called using the current() function instead of directly calling it on model queries.
Get the approved versions of the model being queried, unless there
exists a version of the model in a draft state within a transaction
preceding (and including) the given transaction in the workbasket of the
given transaction."""
Expand Down
3 changes: 2 additions & 1 deletion common/tests/factories.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

import factory
from factory.fuzzy import FuzzyChoice
from factory.fuzzy import FuzzyText
from faker import Faker

from common.models import TrackedModel
Expand Down Expand Up @@ -745,7 +746,7 @@ class Meta:
model = "quotas.QuotaOrderNumber"

sid = numeric_sid()
order_number = string_sequence(6, characters=string.digits)
order_number = FuzzyText(length=4, chars=string.digits, prefix="05")
mechanism = 0
category = 1
valid_between = date_ranges("big_no_end")
Expand Down
7 changes: 2 additions & 5 deletions common/tests/test_models.py
Original file line number Diff line number Diff line change
Expand Up @@ -308,11 +308,8 @@ def test_current_as_of(sample_model):
version_group=sample_model.version_group,
)

assert models.TestModel1.objects.latest_approved().get().pk == sample_model.pk
assert (
models.TestModel1.objects.approved_up_to_transaction(transaction).get().pk
== unapproved_version.pk
)
assert models.TestModel1.objects.latest_approved().get() == sample_model
assert models.TestModel1.objects.current().get() == unapproved_version


def test_create_with_description():
Expand Down
2 changes: 1 addition & 1 deletion docs/source/architecture/index.rst
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ that is not draft is considered to be "current".
There are a number of convenience methods for finding "current" models.

.. autoclass:: common.models.trackedmodel.TrackedModelQuerySet
:members: latest_approved, approved_up_to_transaction
:members: latest_approved, current, approved_up_to_transaction


Workbaskets
Expand Down
6 changes: 1 addition & 5 deletions footnotes/forms.py
Original file line number Diff line number Diff line change
Expand Up @@ -120,15 +120,11 @@ def next_sid(self, instance):
form's save() method (with its commit param set either to True or
False).
"""
workbasket = WorkBasket.current(self.request)
tx = None
if workbasket:
tx = workbasket.transactions.order_by("order").last()

return get_next_id(
models.Footnote.objects.filter(
footnote_type__footnote_type_id=instance.footnote_type.footnote_type_id,
).approved_up_to_transaction(tx),
).current(),
instance._meta.get_field("footnote_id"),
max_len=3,
)
Expand Down
9 changes: 3 additions & 6 deletions footnotes/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -44,9 +44,8 @@ class FootnoteViewSet(viewsets.ReadOnlyModelViewSet):
]

def get_queryset(self):
tx = WorkBasket.get_current_transaction(self.request)
return (
models.Footnote.objects.approved_up_to_transaction(tx)
models.Footnote.objects.current()
nboyse marked this conversation as resolved.
Show resolved Hide resolved
.select_related("footnote_type")
.prefetch_related("descriptions")
)
Expand All @@ -67,8 +66,7 @@ class FootnoteMixin:
model: Type[TrackedModel] = models.Footnote

def get_queryset(self):
tx = WorkBasket.get_current_transaction(self.request)
return models.Footnote.objects.approved_up_to_transaction(tx).select_related(
return models.Footnote.objects.current().select_related(
nboyse marked this conversation as resolved.
Show resolved Hide resolved
"footnote_type",
)

Expand All @@ -77,8 +75,7 @@ class FootnoteDescriptionMixin:
model: Type[TrackedModel] = models.FootnoteDescription

def get_queryset(self):
tx = WorkBasket.get_current_transaction(self.request)
return models.FootnoteDescription.objects.approved_up_to_transaction(tx)
return models.FootnoteDescription.objects.current()
nboyse marked this conversation as resolved.
Show resolved Hide resolved


class FootnoteCreateDescriptionMixin:
Expand Down
4 changes: 2 additions & 2 deletions geo_areas/business_rules.py
Original file line number Diff line number Diff line change
Expand Up @@ -172,7 +172,7 @@ def validate(self, membership):
member__version_group=membership.member.version_group,
valid_between__overlap=membership.valid_between,
)
.approved_up_to_transaction(membership.transaction)
.current()
.exclude(
id=membership.id,
)
Expand Down Expand Up @@ -209,7 +209,7 @@ def validate(self, membership):
parent.members.filter(
member__sid=membership.member.sid,
)
.approved_up_to_transaction(membership.transaction)
.current()
.exclude(
valid_between__contains=membership.valid_between,
)
Expand Down
3 changes: 1 addition & 2 deletions geo_areas/forms.py
Original file line number Diff line number Diff line change
Expand Up @@ -328,8 +328,7 @@ def clean(self):
)

if membership and action == GeoMembershipAction.DELETE:
tx = WorkBasket.get_current_transaction(self.request)
if membership.member_used_in_measure_exclusion(transaction=tx):
if membership.member_used_in_measure_exclusion():
nboyse marked this conversation as resolved.
Show resolved Hide resolved
self.add_error(
"membership",
f"{membership.member.structure_description} is referenced as an excluded geographical area in a measure and cannot be deleted as a member of the area group.",
Expand Down
Loading
Loading