Skip to content

Commit

Permalink
Tp2000 819 importer refactor (#1015)
Browse files Browse the repository at this point in the history
* initial new parser commit

* add new parsers and process for getting info from xml to dictionary for message

* updated to using subclasses to find all parsers, then populate the object from the data provided

* updated / added new parsers for all objects

* updated parsers and linked to handlers (for dependency data, may refactor)

* added validation + working on reporting issues

* TP2000-844  Add suffix and indent columns to find commodities view (#885)

* Add suffix and indent column

* Update test

* Show indent as at today

* Update test

* Refactor

* Use optional type hint

* TP2000-738 Display rule check progress (#887)

* Display rule check progress in UI

* Add test

* Add test docstring

* TP2000-849  Add active state filter to quota search (#886)

* Add active state filter

* Add test

* Test invalid active state filter option

* Rename fixture variable

* TP2000-741 ME32 & ME16 rules checker failing (#874)

* TP2000-477 Prepopulate geo exclusions (#889)

* Pre-populate geo exclusions formset

* Add conditions initial data, update tests

* Fix tests

* Fix tests

* Add test for formset_add_or_delete

* Add erga omnes prefix

* Use regex

* Tp2000 762  virus checker (#888)

* adding docker network

* virus check validator

* skip virus check localy

* removing variable

* validator migration

* PR comments and handler update

* updating to make it obvious creds aren't real

* removing dev file upoad handler

* testing defaults

* updated local config

* TP2000-806 get envelope history (#890)

* TP2000-860 Sqlite dump and upload failing (#892)

* Skip migration during sqlite database creation.

* Run Sqlite export process during out of hours.

* updated exclusion mapping list as country has none (#895)

* TP2000-861 Fix footnote delete button on measure edit view (#894)

* Fix delete button

* Add trailing slash to auto-generated URLs

* Order indents to ensure access to latest version. (#897)

* updating link data (WIP)

* updating link data (WIP)

* updating link data (WIP)

* set parent import model (WIP)

* set parent import model (WIP)

* WIP - adding tests with example envelopes for the new importer

* wip

* wip

* initial new parser commit

* add new parsers and process for getting info from xml to dictionary for message

* updated to using subclasses to find all parsers, then populate the object from the data provided

* updated / added new parsers for all objects

* updated parsers and linked to handlers (for dependency data, may refactor)

* added validation + working on reporting issues

* updating link data (WIP)

* updating link data (WIP)

* updating link data (WIP)

* set parent import model (WIP)

* set parent import model (WIP)

* WIP - adding tests with example envelopes for the new importer

* wip

* wip

* test updates

* test updates

* test updates

* test updates

* test updates

* test updates

* test updates

* test updates

* test updates

* test updates

* test updates

* test updates

* test updates

* test updates

* building out some UI for the new importer

* building out some UI for the new importer

* building out some UI for the new importer

* building out some UI for the new importer

* adding identity fields to all parsers

* fix flaky tests

* testing updates

* testing updates

* testing updates - finished update tests for all TARIC objects we use currently

* testing updates - added add codes delete tests

* testing updates - added add codes delete tests

* testing updates - added certificate, commodity, footnote, geoarea, and measure delete tests

* moved new parser / importer to its own app

* finished adding delete tests and removed unused methods

* added documentation

* added documentation + small fixes

* added documentation + small fixes

* added documentation + small fixes

* update comm code import objects

* added documentation + small fixes

* added documentation + small fixes

* added documentation + small fixes

* minor updates based on PR comments

* minor updates based on PR comments

* minor updates based on PR comments

* minor updates based on PR comments

* minor updates based on PR comments

* minor updates based on PR comments

* test fix for py 3.8

* test fix for py 3.8

* tidy up chunker a little and add tests

* duplicated a bunch of files referenced in the importer app - anything thats not a model now also lives in the taric_parsers app.

* duplicated a bunch of files referenced in the importer app - anything that's not a model now also lives in the taric_parsers app.

* minor changes from PR review

* updated issue_type for import error creation to a text choice

* added tests to UI elements

* altered ot absolute imports

* altered exception raised on TaricXmlSourceBase

* import tidy up - absolute imports

* minor updates

* minor updates

* minor updates

* minor updates

* minor updates

* minor updates

* minor updates

* minor updates

* Updating documentation + PR feedback

* Updating documentation + PR feedback

* updates based on PR feedback

* updates based on PR feedback

* updates based on PR feedback

* updates based on PR feedback

* updates based on PR feedback

* updates based on PR feedback

* updates based on PR feedback

* updates based on PR feedback

* updates based on PR feedback

* updates based on PR feedback

* updates based on PR feedback

* updates based on PR feedback

* updates based on PR feedback

* update pre-commit-hook format, was showing as invalid yaml - no functional change - just formatting

* updates to readme, adding pre-commit info

* updates to readme, adding pre-commit info

* updates to readme, adding pre-commit info

* updates to readme, adding pre-commit info

* updates to readme, adding pre-commit info

* rename to remove New prefix

* rename to remove New prefix

* PR updates

* PR updates

* PR updates

* PR updates - adding in importerv2 to main comm code UI

* PR updates : integrating UI - mid progress

* minor UI update

* fix unit tests mid way

* interim commit

* interim commit

* interim commit

* interim commit

* fixed tests

* fixed tests

* fixed tests

* add in missing migration

* update NIG5 to not trigger when goods nomenclature also being deleted in the same transaction

---------

Co-authored-by: Dale Cannon <[email protected]>
Co-authored-by: Paul Pepper <[email protected]>
Co-authored-by: Edie Pearce <[email protected]>
Co-authored-by: Anthoni Gleeson <[email protected]>
  • Loading branch information
5 people authored and CPrich905 committed Jan 12, 2024
1 parent 8ed0017 commit a38689f
Show file tree
Hide file tree
Showing 377 changed files with 30,711 additions and 193 deletions.
12 changes: 5 additions & 7 deletions .pre-commit-config.yaml
Original file line number Diff line number Diff line change
@@ -1,14 +1,13 @@
repos:
- repo: https://github.com/asottile/add-trailing-comma
rev: v2.4.0
rev: v3.1.0
hooks:
- id: add-trailing-comma
- repo: https://github.com/myint/autoflake.git
rev: v2.1.1
rev: v2.2.1
hooks:
- id: autoflake
args:
[
args: [
"--in-place",
"--remove-all-unused-imports",
"--remove-unused-variable",
Expand All @@ -22,8 +21,7 @@ repos:
rev: v1.7.5
hooks:
- id: docformatter
args:
[
args: [
"--in-place",
"--wrap-summaries=80",
"--wrap-descriptions=80",
Expand All @@ -34,7 +32,7 @@ repos:
hooks:
- id: black
- repo: https://github.com/ikamensh/flynt/
rev: '0.78'
rev: '1.0.1'
hooks:
- id: flynt
- repo: https://github.com/uktrade/pii-secret-check-hooks
Expand Down
89 changes: 89 additions & 0 deletions README.rst
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,86 @@ To run tests use the following command:
For more detailed information on running tests, see :doc:`testing`

Pre-commit hooks
----------------

This project uses pre-commit hooks to update formatting and identify potential sensitive data before
it is committed to the public repo.

note: The python package pre-commit is a requirement within requirements-dev.txt and should be installed
to meet development requirements

Install
~~~~~~~

To initially setup the pre-commit hooks you can run the following command.

.. code:: sh
$ pre-commit install
Once installed, when committing it will first run all the predefined processes to clean up code formatting
and notify about any detected sensitive strings found that are not in pii exclude files.

Note: the first commit or run of the pre-commit hooks after installing may take a few minutes for setup the
dependent packages for the first time. This is normal, and will be faster on subsequent commits.

Update
~~~~~~

The packages used to perform the pre-commit process are regularly updated. Periodically its advised
you run the following command to keep the dependencies updated.

.. code:: sh
$ pre-commit autoupdate
This will verify that the dependencies are updated based on requirements.

Uninstall
~~~~~~~~~

The pre-commit hooks can be uninstalled with the following command

.. code:: sh
$ pre-commit uninstall
Run the hooks without committing
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

You may at times want to run the pre-commit hooks before committing. This can be done with
the following command. This command will run the hooks on all changed files.

.. code:: sh
$ pre-commit run
If you would like to run the hooks over all files you can run the following command

.. code:: sh
$ pre-commit run -a
or

.. code:: sh
$ pre-commit run --all-files
Troubleshooting
~~~~~~~~~~~~~~~

If you encounter issues with the pre-commit hooks there are a number of things you can
clear the cached pre-committed files using this command:

.. code:: sh
$ pre-commit clean
If that fails you can try updating the dependencies for the hooks

If the above fails, uninstall and then install again.

Dockerisation
-------------
Expand Down Expand Up @@ -309,6 +389,15 @@ This command is broken into two stages:
$ python manage.py run_import_batch
Using the TARIC parser (currently referenced importer v2)
-----------------------------------------

There are no command line tools available for this tool.

This tool is available as an importer alternative found within the web front end in the footer menu under "New TARIC parser".

This tool addresses several short falls that the current importer has.

Using the exporter
------------------

Expand Down
11 changes: 11 additions & 0 deletions commodities/business_rules.py
Original file line number Diff line number Diff line change
Expand Up @@ -206,6 +206,7 @@ def validate(self, origin):
If it does not meet these criteria it's a validation fail
"""

from commodities.models.orm import GoodsNomenclature
from commodities.models.orm import GoodsNomenclatureOrigin

if origin.new_goods_nomenclature.valid_between.lower < date(2021, 1, 1):
Expand All @@ -224,6 +225,16 @@ def validate(self, origin):
):
return

# verify the goods nomenclature exists in the same transaction, it will not if it has been deleted
if not (
GoodsNomenclature.objects.approved_up_to_transaction(
origin.transaction,
)
.filter(sid=origin.new_goods_nomenclature.sid)
.exists()
):
return

raise self.violation(
model=origin,
message="Non top-level goods must have an origin specified. None remain if this origin is deleted",
Expand Down
44 changes: 20 additions & 24 deletions commodities/models/dc.py
Original file line number Diff line number Diff line change
Expand Up @@ -425,7 +425,7 @@ def __post_init__(self) -> None:
super().__post_init__()

def get_potential_parents(self, commodity: Commodity) -> List[Commodity]:
"""gets possible parents, for queries where a parent may be end-dated,
"""Gets possible parents, for queries where a parent may be end-dated,
and another parent could potentially be present to inherit the child."""

parent = self.get_parent(commodity)
Expand Down Expand Up @@ -686,10 +686,11 @@ def _get_diff(
"""
Returns a snapshot diff for a given relation on a sinlge commodity.
For detailed overview on snapshot diffs, see the docs for the SnapshotDiff class.
For detailed overview on snapshot diffs, see the docs for the
SnapshotDiff class.
You can get one diff per commodity and relation type
(e.g. diff the children of '9999.20.00.00' or diff the siblings of '9999.30.20.10')
You can get one diff per commodity and relation type (e.g. diff the
children of '9999.20.00.00' or diff the siblings of '9999.30.20.10')
"""
if snapshot.moment.clock_type != self.moment.clock_type:
raise ValueError("Cannot diff snapshots with different clock types.")
Expand Down Expand Up @@ -838,19 +839,17 @@ def _get_snapshot_commodities(
"""
Returns the list of commodities than belong to a snapshot.
This method needs to be very efficient -
In particular, it should require the same number
of database round trips regardless of the level
This method needs to be very efficient - In particular, it should
require the same number of database round trips regardless of the level
of the root commodity in the tree.
The solution is fetch all goods matching the snapshot moment
(incl. potentially multiple versions of each)
and then call a new util method get_latest_version,
which efficiently yields only the latest version
of each good from within the returned queryset.
The solution is fetch all goods matching the snapshot moment (incl.
potentially multiple versions of each) and then call a new util method
get_latest_version, which efficiently yields only the latest version of
each good from within the returned queryset.
We then efficiently find the commodities in our collection
that match the latest_version goods.
We then efficiently find the commodities in our collection that match
the latest_version goods.
"""
item_ids = {c.item_id for c in self.commodities if c.obj}
goods = GoodsNomenclature.objects.approved_up_to_transaction(
Expand Down Expand Up @@ -1338,10 +1337,8 @@ def _handle_hierarchy_side_effect(
"""
Updates or deletes a clashing ME32 measure.
If either measure validity period
is fully contained in the other's,
the only option is to delete one,
favoring the one with lower-level code.
If either measure validity period is fully contained in the other's, the
only option is to delete one, favoring the one with lower-level code.
Otherwise we have an opportunity to cap the earlier measure.
"""
Expand Down Expand Up @@ -1526,13 +1523,12 @@ def as_at_date(self) -> date:
"""
Returns the threshold date for the commodity code change.
Dependent measures (or other dependent models with validity spans)
would be exclude those with effective end date before this date.
Dependent measures (or other dependent models with validity spans) would
be exclude those with effective end date before this date.
In the case of a commodity UPDATE or DELETE,
the threshold is the commodity's validity end date.
In the case of a commodity CREATE,
the threshold is the commodity's validity start date.
In the case of a commodity UPDATE or DELETE, the threshold is the
commodity's validity end date. In the case of a commodity CREATE, the
threshold is the commodity's validity start date.
"""
if self.update_type == UpdateType.UPDATE:
return self.current.valid_between.upper
Expand Down
72 changes: 72 additions & 0 deletions commodities/tests/business_rules/test_NIG5_origin.py
Original file line number Diff line number Diff line change
Expand Up @@ -138,3 +138,75 @@ def test_NIG5_origin_does_not_raise_violation_when_origin_still_exists(workbaske
)

business_rules.NIG5_origin(editable_transaction).validate(origin_delete)


def test_NIG5_origin_does_not_raise_violation_when_both_origin_and_goods_nomenclature_are_deleted(
workbasket,
):
"""
When deleting an origin the violation does not trigger if the goods
nomenclature is also being deleted in the same transaction.
This rule is only applicable when origins are deleted.
"""
# Setup data
published_workbasket = factories.PublishedWorkBasketFactory()

approved_transaction = factories.ApprovedTransactionFactory.create(
workbasket=published_workbasket,
order=1,
)

approved_transaction_2 = factories.ApprovedTransactionFactory.create(
workbasket=published_workbasket,
order=2,
)

approved_transaction_3 = factories.ApprovedTransactionFactory.create(
workbasket=published_workbasket,
order=3,
)

goods_chapter = factories.GoodsNomenclatureFactory.create(
item_id="2000000000",
transaction=approved_transaction,
)

# This is the goods we are going to delete the origin from
goods = factories.GoodsNomenclatureFactory.create(
item_id="2000000010",
origin__derived_from_goods_nomenclature=goods_chapter,
indent__indent=1,
transaction=approved_transaction_2,
)

editable_workbasket = factories.WorkBasketFactory()

editable_transaction = factories.TransactionFactory.create(
workbasket=editable_workbasket,
order=4,
)

origin_delete = (
GoodsNomenclatureOrigin.objects.approved_up_to_transaction(
approved_transaction_2,
)
.filter(
new_goods_nomenclature=goods,
derived_from_goods_nomenclature=goods_chapter,
)
.first()
.new_version(
transaction=editable_transaction,
update_type=UpdateType.DELETE,
workbasket=editable_workbasket,
)
)

goods_delete = goods.new_version(
transaction=editable_transaction,
update_type=UpdateType.DELETE,
workbasket=editable_workbasket,
)

business_rules.NIG5_origin(editable_transaction).validate(origin_delete)
13 changes: 6 additions & 7 deletions commodities/tests/test_migrations.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,15 +8,16 @@


@pytest.mark.django_db()
def test_main_migration_works(migrator):
def test_main_migration_works(migrator, setup_content_types):
"""Ensures that the description date fix for TOPS-745 migration works."""
# migrator.reset()

# before migration
old_state = migrator.apply_initial_migration(
("commodities", "0011_TOPS_745_migration_dependencies"),
)

setup_content_types(old_state.apps)

GoodsNomenclatureDescription = old_state.apps.get_model(
"commodities",
"GoodsNomenclatureDescription",
Expand Down Expand Up @@ -83,16 +84,16 @@ def test_main_migration_works(migrator):
6,
)

migrator.reset()


@pytest.mark.django_db()
def test_main_migration_ignores_if_no_data(migrator):
def test_main_migration_ignores_if_no_data(migrator, setup_content_types):
# before migration
old_state = migrator.apply_initial_migration(
("commodities", "0011_TOPS_745_migration_dependencies"),
)

setup_content_types(old_state.apps)

GoodsNomenclatureDescription = old_state.apps.get_model(
"commodities",
"GoodsNomenclatureDescription",
Expand All @@ -104,5 +105,3 @@ def test_main_migration_ignores_if_no_data(migrator):
migrator.apply_tested_migration(
("commodities", "0012_TOPS_745_description_date_fix"),
)

migrator.reset()
6 changes: 5 additions & 1 deletion common/jinja2/layouts/layout.jinja
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,11 @@
},
{
"href": url("import_batch-ui-list"),
"text": "Master importer"
"text": "Importer V1"
},
{
"href": url("taric_parser_import_ui_list"),
"text": "Importer V2"
},
{
"href": url("reports:index"),
Expand Down
2 changes: 1 addition & 1 deletion common/tests/test_views.py
Original file line number Diff line number Diff line change
Expand Up @@ -189,7 +189,7 @@ def test_index_displays_footer_links(valid_user_client):
page = BeautifulSoup(str(response.content), "html.parser")
a_tags = page.select("footer a")

assert len(a_tags) == 6
assert len(a_tags) == 7
assert "Privacy policy" in a_tags[0].text
assert (
a_tags[0].attrs["href"]
Expand Down
Loading

0 comments on commit a38689f

Please sign in to comment.