Skip to content

Commit

Permalink
Resolve outstanding TODOs and fix linting #342
Browse files Browse the repository at this point in the history
  • Loading branch information
@joelvdavies
joelvdavies committed Aug 7, 2024
1 parent d17e167 commit 878054b
Show file tree
Hide file tree
Showing 5 changed files with 67 additions and 35 deletions.
27 changes: 25 additions & 2 deletions test/e2e/conftest.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ def check_created_and_modified_times_updated_correctly(post_response: Response,

@staticmethod
def replace_unit_values_with_ids_in_properties(data: dict, unit_value_id_dict: dict[str, str]) -> dict:
"""Inserts unit IDs into some data that may have a 'properties' list within it.
"""Inserts unit IDs into some data that may have a 'properties' list within it while removing the unit value.
:param data: Dictionary of data that could have a 'properties' value within it.
:param unit_value_id_dict: Dictionary of unit value and ID pairs for unit ID lookups.
Expand All @@ -129,9 +129,32 @@ def replace_unit_values_with_ids_in_properties(data: dict, unit_value_id_dict: d
return {**data, "properties": new_properties}
return data

@staticmethod
def add_unit_ids_to_properties(data: dict, unit_value_id_dict: dict[str, str]) -> dict:
"""Inserts unit IDs into some data that may have a 'properties' list within it.
:param data: Dictionary of data that could have a 'properties' value within it.
:param unit_value_id_dict: Dictionary of unit value and ID pairs for unit ID lookups.
:return: The data with any needed unit IDs inserted.
"""

if "properties" in data and data["properties"]:
new_properties = []
for prop in data["properties"]:
new_property = {**prop}
if "unit" in prop:
if prop["unit"] is not None:
new_property["unit_id"] = unit_value_id_dict[prop["unit"]]
else:
new_property["unit_id"] = None
new_properties.append(new_property)
return {**data, "properties": new_properties}
return data

@staticmethod
def replace_property_names_with_ids_in_properties(data: dict, property_name_id_dict: dict[str, str]) -> dict:
"""Inserts property IDs into some data that may have a 'properties' list within it.
"""Inserts property IDs into some data that may have a 'properties' list within it while removing the property
name.
:param data: Dictionary of data that could have a 'properties' value within it.
:param property_name_id_dict: Dictionary of property name and ID pairs for property ID lookups.
Expand Down
23 changes: 16 additions & 7 deletions test/e2e/test_catalogue_category.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -130,12 +130,15 @@ def check_post_catalogue_category_success(self, expected_catalogue_category_get_
Checks that a prior call to `post_catalogue_category` gave a successful response with the expected data
returned.
:param expected_catalogue_category_get_data: Dictionary containing the expected catalogue category data returned
as would be required for a `CatalogueCategorySchema`.
:param expected_catalogue_category_get_data: Dictionary containing the expected system data returned as would
be required for a `CatalogueCategorySchema`. Does not need unit IDs
as they will be added automatically to check they are as expected.
"""

assert self._post_response_catalogue_category.status_code == 201
assert self._post_response_catalogue_category.json() == expected_catalogue_category_get_data
assert self._post_response_catalogue_category.json() == E2ETestHelpers.add_unit_ids_to_properties(
expected_catalogue_category_get_data, self.unit_value_id_dict
)

def check_post_catalogue_category_failed_with_detail(self, status_code: int, detail: str) -> None:
"""
Expand Down Expand Up @@ -403,11 +406,14 @@ def check_get_catalogue_category_success(self, expected_catalogue_category_get_d
Checks that a prior call to `get_catalogue_category` gave a successful response with the expected data returned.
:param expected_catalogue_category_get_data: Dictionary containing the expected system data returned as would
be required for a `CatalogueCategorySchema`.
be required for a `CatalogueCategorySchema`. Does not need unit IDs
as they will be added automatically to check they are as expected.
"""

assert self._get_response_catalogue_category.status_code == 200
assert self._get_response_catalogue_category.json() == expected_catalogue_category_get_data
assert self._get_response_catalogue_category.json() == E2ETestHelpers.add_unit_ids_to_properties(
expected_catalogue_category_get_data, self.unit_value_id_dict
)

def check_get_catalogue_category_failed_with_detail(self, status_code: int, detail: str) -> None:
"""
Expand Down Expand Up @@ -771,11 +777,14 @@ def check_patch_catalogue_category_response_success(self, expected_catalogue_cat
returned.
:param expected_catalogue_category_get_data: Dictionary containing the expected system data returned as would
be required for a `CatalogueCategorySchema`.
be required for a `CatalogueCategorySchema`. Does not need unit IDs
as they will be added automatically to check they are as expected.
"""

assert self._patch_response_catalogue_category.status_code == 200
assert self._patch_response_catalogue_category.json() == expected_catalogue_category_get_data
assert self._patch_response_catalogue_category.json() == E2ETestHelpers.add_unit_ids_to_properties(
expected_catalogue_category_get_data, self.unit_value_id_dict
)

E2ETestHelpers.check_created_and_modified_times_updated_correctly(
self._post_response_catalogue_category, self._patch_response_catalogue_category
Expand Down
28 changes: 16 additions & 12 deletions test/e2e/test_catalogue_item.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,14 +34,14 @@
MANUFACTURER_POST_DATA_ALL_VALUES,
MANUFACTURER_POST_DATA_REQUIRED_VALUES_ONLY,
PROPERTY_DATA_BOOLEAN_MANDATORY_TRUE,
PROPERTY_DATA_NUMBER_NON_MANDATORY_42,
PROPERTY_DATA_NUMBER_NON_MANDATORY_NONE,
PROPERTY_DATA_NUMBER_NON_MANDATORY_WITH_ALLOWED_VALUES_LIST_1,
PROPERTY_DATA_NUMBER_NON_MANDATORY_WITH_MM_UNIT_42,
PROPERTY_DATA_NUMBER_NON_MANDATORY_WITH_MM_UNIT_NONE,
PROPERTY_DATA_STRING_MANDATORY_TEXT,
PROPERTY_DATA_STRING_NON_MANDATORY_WITH_ALLOWED_VALUES_LIST_NONE,
PROPERTY_DATA_STRING_NON_MANDATORY_WITH_ALLOWED_VALUES_LIST_VALUE1,
PROPERTY_GET_DATA_NUMBER_NON_MANDATORY_42,
PROPERTY_GET_DATA_NUMBER_NON_MANDATORY_WITH_ALLOWED_VALUES_LIST_1,
PROPERTY_GET_DATA_NUMBER_NON_MANDATORY_WITH_MM_UNIT_42,
PROPERTY_GET_DATA_STRING_NON_MANDATORY_WITH_ALLOWED_VALUES_LIST_VALUE1,
UNIT_POST_DATA_MM,
)
Expand Down Expand Up @@ -308,7 +308,7 @@ def check_post_catalogue_item_failed_with_validation_message(self, status_code:


class TestCreate(CreateDSL):
"""Tests for creating a catalogue category."""
"""Tests for creating a catalogue item."""

def test_create_with_only_required_values_provided(self):
"""Test creating a catalogue item with only required values provided."""
Expand Down Expand Up @@ -426,7 +426,7 @@ def test_create_with_non_mandatory_properties_given_none(self):
**CATALOGUE_ITEM_DATA_WITH_MANDATORY_PROPERTIES_ONLY,
"properties": [
PROPERTY_DATA_BOOLEAN_MANDATORY_TRUE,
PROPERTY_DATA_NUMBER_NON_MANDATORY_NONE,
PROPERTY_DATA_NUMBER_NON_MANDATORY_WITH_MM_UNIT_NONE,
PROPERTY_DATA_STRING_NON_MANDATORY_WITH_ALLOWED_VALUES_LIST_NONE,
],
}
Expand Down Expand Up @@ -1000,12 +1000,15 @@ def test_partial_update_catalogue_category_id_and_properties_with_different_defi

self.patch_catalogue_item(
catalogue_item_id,
{"catalogue_category_id": new_catalogue_category_id, "properties": [PROPERTY_DATA_NUMBER_NON_MANDATORY_42]},
{
"catalogue_category_id": new_catalogue_category_id,
"properties": [PROPERTY_DATA_NUMBER_NON_MANDATORY_WITH_MM_UNIT_42],
},
)
self.check_patch_catalogue_item_response_success(
{
**CATALOGUE_ITEM_GET_DATA_WITH_ALL_PROPERTIES,
"properties": [PROPERTY_GET_DATA_NUMBER_NON_MANDATORY_42],
"properties": [PROPERTY_GET_DATA_NUMBER_NON_MANDATORY_WITH_MM_UNIT_42],
}
)

Expand Down Expand Up @@ -1219,7 +1222,7 @@ def test_partial_update_properties_with_non_mandatory_properties_given_none(self
{
"properties": [
PROPERTY_DATA_BOOLEAN_MANDATORY_TRUE,
PROPERTY_DATA_NUMBER_NON_MANDATORY_NONE,
PROPERTY_DATA_NUMBER_NON_MANDATORY_WITH_MM_UNIT_NONE,
PROPERTY_DATA_STRING_NON_MANDATORY_WITH_ALLOWED_VALUES_LIST_NONE,
]
},
Expand Down Expand Up @@ -1305,16 +1308,17 @@ def test_partial_update_properties_with_number_property_with_invalid_value_type(

catalogue_item_id = self.post_catalogue_item_and_prerequisites_with_given_properties(
catalogue_category_properties_data=[CATALOGUE_CATEGORY_PROPERTY_DATA_NUMBER_NON_MANDATORY_WITH_MM_UNIT],
catalogue_item_properties_data=[PROPERTY_DATA_NUMBER_NON_MANDATORY_42],
catalogue_item_properties_data=[PROPERTY_DATA_NUMBER_NON_MANDATORY_WITH_MM_UNIT_42],
)

self.patch_catalogue_item(
catalogue_item_id, {"properties": [{**PROPERTY_DATA_NUMBER_NON_MANDATORY_42, "value": "42"}]}
catalogue_item_id, {"properties": [{**PROPERTY_DATA_NUMBER_NON_MANDATORY_WITH_MM_UNIT_42, "value": "42"}]}
)
self.check_patch_catalogue_item_failed_with_detail(
422,
"Invalid value type for property with ID "
f"'{self.property_name_id_dict[PROPERTY_DATA_NUMBER_NON_MANDATORY_42['name']]}'. Expected type: number.",
f"'{self.property_name_id_dict[PROPERTY_DATA_NUMBER_NON_MANDATORY_WITH_MM_UNIT_42['name']]}'. "
"Expected type: number.",
)

def test_partial_update_properties_with_boolean_property_with_invalid_value_type(self):
Expand Down Expand Up @@ -1511,7 +1515,7 @@ def test_delete(self):
self.check_get_catalogue_item_failed_with_detail(404, "Catalogue item not found")

def test_delete_with_child_item(self):
"""Test deleting a catalogue category with a child catalogue item."""
"""Test deleting a catalogue item with a child item."""

self.post_catalogue_category(CATALOGUE_CATEGORY_POST_DATA_LEAF_NO_PARENT_NO_PROPERTIES)
self.post_manufacturer(MANUFACTURER_POST_DATA_REQUIRED_VALUES_ONLY)
Expand Down
20 changes: 8 additions & 12 deletions test/mock_data.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,11 +40,9 @@

CATALOGUE_CATEGORY_PROPERTY_IN_DATA_BOOLEAN_MANDATORY = {**CATALOGUE_CATEGORY_PROPERTY_DATA_BOOLEAN_MANDATORY}

# TODO: Unit ids should be known so ideally should be inserted I think - check for other ANY's like this
CATALOGUE_CATEGORY_PROPERTY_GET_DATA_BOOLEAN_MANDATORY = {
**CATALOGUE_CATEGORY_PROPERTY_DATA_BOOLEAN_MANDATORY,
"id": ANY,
"unit_id": ANY,
"unit": None,
"allowed_values": None,
}
Expand All @@ -66,11 +64,11 @@
CATALOGUE_CATEGORY_PROPERTY_GET_DATA_NUMBER_NON_MANDATORY_WITH_MM_UNIT = {
**CATALOGUE_CATEGORY_PROPERTY_DATA_NUMBER_NON_MANDATORY_WITH_MM_UNIT,
"id": ANY,
"unit_id": ANY,
"allowed_values": None,
}

# Number, Non Mandatory, Allowed values list

CATALOGUE_CATEGORY_PROPERTY_DATA_NUMBER_NON_MANDATORY_WITH_ALLOWED_VALUES_LIST = {
"name": "Non Mandatory Number Property With Allowed Values",
"type": "number",
Expand Down Expand Up @@ -103,7 +101,6 @@
CATALOGUE_CATEGORY_PROPERTY_GET_DATA_STRING_NON_MANDATORY_WITH_ALLOWED_VALUES_LIST = {
**CATALOGUE_CATEGORY_PROPERTY_DATA_STRING_NON_MANDATORY_WITH_ALLOWED_VALUES_LIST,
"id": ANY,
"unit_id": ANY,
"unit": None,
}

Expand Down Expand Up @@ -267,22 +264,21 @@

# Number, Non Mandatory, 42

# TODO: Rename? Specific to the CATALOGUE_CATEGORY_PROPERTY_DATA_NUMBER_NON_MANDATORY_WITH_MM_UNIT
PROPERTY_DATA_NUMBER_NON_MANDATORY_42 = {
PROPERTY_DATA_NUMBER_NON_MANDATORY_WITH_MM_UNIT_42 = {
"name": CATALOGUE_CATEGORY_PROPERTY_DATA_NUMBER_NON_MANDATORY_WITH_MM_UNIT["name"],
"value": 42,
}

PROPERTY_GET_DATA_NUMBER_NON_MANDATORY_42 = {**PROPERTY_DATA_NUMBER_NON_MANDATORY_42}
PROPERTY_GET_DATA_NUMBER_NON_MANDATORY_WITH_MM_UNIT_42 = {**PROPERTY_DATA_NUMBER_NON_MANDATORY_WITH_MM_UNIT_42}

# Number, Non Mandatory, None

PROPERTY_DATA_NUMBER_NON_MANDATORY_NONE = {
PROPERTY_DATA_NUMBER_NON_MANDATORY_WITH_MM_UNIT_NONE = {
"name": CATALOGUE_CATEGORY_PROPERTY_DATA_NUMBER_NON_MANDATORY_WITH_MM_UNIT["name"],
"value": None,
}

PROPERTY_GET_DATA_NUMBER_NON_MANDATORY_NONE = {**PROPERTY_DATA_NUMBER_NON_MANDATORY_NONE}
PROPERTY_GET_DATA_NUMBER_NON_MANDATORY_WITH_MM_UNIT_NONE = {**PROPERTY_DATA_NUMBER_NON_MANDATORY_WITH_MM_UNIT_NONE}

# String, Mandatory, text
PROPERTY_DATA_STRING_MANDATORY_TEXT = {
Expand Down Expand Up @@ -402,7 +398,7 @@
"name": "Catalogue Item With All Properties",
"properties": [
PROPERTY_DATA_BOOLEAN_MANDATORY_TRUE,
PROPERTY_DATA_NUMBER_NON_MANDATORY_42,
PROPERTY_DATA_NUMBER_NON_MANDATORY_WITH_MM_UNIT_42,
PROPERTY_DATA_STRING_NON_MANDATORY_WITH_ALLOWED_VALUES_LIST_VALUE1,
],
}
Expand All @@ -412,7 +408,7 @@
**CATALOGUE_ITEM_DATA_WITH_ALL_PROPERTIES,
"properties": [
PROPERTY_GET_DATA_BOOLEAN_MANDATORY_TRUE,
PROPERTY_GET_DATA_NUMBER_NON_MANDATORY_42,
PROPERTY_GET_DATA_NUMBER_NON_MANDATORY_WITH_MM_UNIT_42,
PROPERTY_GET_DATA_STRING_NON_MANDATORY_WITH_ALLOWED_VALUES_LIST_VALUE1,
],
}
Expand All @@ -432,7 +428,7 @@
**CATALOGUE_ITEM_DATA_WITH_MANDATORY_PROPERTIES_ONLY,
"properties": [
PROPERTY_GET_DATA_BOOLEAN_MANDATORY_TRUE,
PROPERTY_GET_DATA_NUMBER_NON_MANDATORY_NONE,
PROPERTY_GET_DATA_NUMBER_NON_MANDATORY_WITH_MM_UNIT_NONE,
PROPERTY_GET_DATA_STRING_NON_MANDATORY_WITH_ALLOWED_VALUES_LIST_NONE,
],
}
Expand Down
4 changes: 2 additions & 2 deletions test/unit/services/test_catalogue_item.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
CATALOGUE_ITEM_DATA_REQUIRED_VALUES_ONLY,
CATALOGUE_ITEM_DATA_WITH_ALL_PROPERTIES,
MANUFACTURER_IN_DATA_A,
PROPERTY_DATA_NUMBER_NON_MANDATORY_42,
PROPERTY_DATA_NUMBER_NON_MANDATORY_WITH_MM_UNIT_42,
)
from test.unit.services.conftest import ServiceTestHelpers
from typing import Optional
Expand Down Expand Up @@ -969,7 +969,7 @@ def test_update_catalogue_category_id_and_properties_with_different_defined_prop
catalogue_item_id,
catalogue_item_update_data={
"catalogue_category_id": str(ObjectId()),
"properties": [PROPERTY_DATA_NUMBER_NON_MANDATORY_42],
"properties": [PROPERTY_DATA_NUMBER_NON_MANDATORY_WITH_MM_UNIT_42],
},
stored_catalogue_item_data=CATALOGUE_ITEM_DATA_WITH_ALL_PROPERTIES,
stored_catalogue_category_in_data=CATALOGUE_CATEGORY_IN_DATA_LEAF_NO_PARENT_WITH_PROPERTIES_MM,
Expand Down

0 comments on commit 878054b

Please sign in to comment.