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-1092 reference documents v1 1 #1290

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

Conversation

dougmills-DIT
Copy link
Contributor

@dougmills-DIT dougmills-DIT commented Sep 12, 2024

TP2000-1092 reference documents v1 1

  • This work facilitates the digital creation of the data contained within reference documents, the tables of order numbers, quota definition, suspensions and preferential rates into a version of a reference document associated with a country.
  • Allows the versioning of a reference document as new versions are released into the public domain, for example 1.1, 1.2, 1.3 of a reference document as they are reviewed and adjusted on a yearly or ad-hoc basis.
  • Allows the bulk adding of preferencial quotas and quota definitions (with some limitations)
  • Allows the adding of a quota definition range, covering multiple quota definition periods with and a linear increment on volume
  • Allows the adding of a quota suspension range, covering multiple quota suspension periods
  • Allows a report to be ran against the reference document data and comparison with existing published TAP data, recording inconsistencies for later review.

Why

  • This is the first step to digitising the reference document creation process
  • having the facility to query and compare reference document data to live daat on TAP improves our ability to check and correct mistakes in live data.
  • The facility to populate and check new reference document data against existing TAP data provides a tool to improve the quality of the current process which is checked by human eyes.

What

  • Create and manage reference documents (associated with a geo area)
  • Create and manage reference document versions, with a process to review and publish data to prevent further changes.
  • Create and manage order numbers, quota definition, suspensions and preferential rates
  • Run a check against TAP data and review where differences are.
  • lots of testing, 96% ov lines are covered by at least one unit test

Checklist

  • Requires migrations? yes
  • Requires dependency updates? no

Links to relevant material
See: Description

LaurenMullally and others added 30 commits November 22, 2023 10:28
…into TP2000-1186_ref_doc_data_model

# Conflicts:
#	reference_documents/models.py
#	reference_documents/urls.py
#	reference_documents/views.py
…sion views, refactored the checks and ran the checks several times against reference document versions.
… review tabs (#1133)

* Add sub-quotas nested review tab

* Add quota blocking periods nested review tab

* Add quota suspension periods nested review tab

* Use tab title instead of model verbose name

* Add blocking period and suspension period SID to table
* Add MAINTENANCE_MODE setting and middleware

* Fix middleware removal and recursive redirect

* Add template view and url

* Add tests

* Update contact us form link for other pages

* Update text wording

* Remove database route during maintenance

* Update maintenance page template/url name

---------

Co-authored-by: Dale Cannon <[email protected]>
* record seq number & message id fix

* fix taricXMLRenderer, pass in value of counter
@dougmills-DIT dougmills-DIT marked this pull request as ready for review September 17, 2024 07:50
@dougmills-DIT dougmills-DIT requested a review from a team as a code owner September 17, 2024 07:50
…ce-documents_v1-1

# Conflicts:
#	reference_documents/tests/test_ref_order_number_forms.py
@@ -190,6 +190,12 @@ def overlaps(self, compared_date_range: TaricDateRange):

return False

def contains(self, compared_date_range: TaricDateRange):
Copy link
Contributor

Choose a reason for hiding this comment

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

Could contained_date_range be used here?

QuotaSuspensionChecks,
)

# import additional checks
Copy link
Contributor

Choose a reason for hiding this comment

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

Commented code

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh wait I misread. Do we need this comment?

from reference_documents.check.base import BaseQuotaSuspensionCheck
from reference_documents.check.base import BaseRateCheck
from reference_documents.check.ref_order_numbers import OrderNumberChecks # noqa
from reference_documents.check.ref_quota_definitions import ( # noqa
Copy link
Contributor

Choose a reason for hiding this comment

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

These could be on one line if it's only one import in the brackets

from reference_documents.views.rate_views import RefRateCreate
from reference_documents.views.rate_views import RefRateDelete
from reference_documents.views.rate_views import RefRateEdit
from reference_documents.views.reference_document_version_views import (
Copy link
Contributor

Choose a reason for hiding this comment

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

The formatting of these imports is kinda weird. Usually we don't use brackets like this

@@ -1,3 +1,34 @@
.check-passing {
color: #1d640f;
Copy link
Contributor

Choose a reason for hiding this comment

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

Should be using the govuk design system colours here. govuk-colour("green")

}

.check-failing {
color: #671111;
Copy link
Contributor

Choose a reason for hiding this comment

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

govuk-colour("red")

}

input[type=number].govuk-input--width-20 {
font-family: "GDS Transport", arial, sans-serif;
Copy link
Contributor

Choose a reason for hiding this comment

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

All this could probably be done with a class instead. What is this styling overriding?

@@ -3,44 +3,175 @@

<div class="quota__core-data govuk-grid-row">
<div class="quota__core-data__content govuk-grid-column-three-quarters">
{% for key, value in reference_document_version_quotas.items() %}
{% if value["quota_order_number"] != None %}
{% for on_ctx in order_numbers %}
Copy link
Contributor

Choose a reason for hiding this comment

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

What is on_ctx here?

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh it's an OrderNumberContext instance. This abbreviation had me a bit confused

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.

8 participants