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

(Improve order flow) Clean up the Order model #4082

Merged
merged 3 commits into from
Jan 14, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion cg/meta/orders/api.py
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ def submit(self, order_type: OrderType, raw_order: dict, user: User) -> dict:
"""
storing_service: StoreOrderService = self.storing_registry.get_storing_service(order_type)
order: Order = self.validation_service.parse_and_validate(
raw_order=raw_order, order_type=order_type
raw_order=raw_order, order_type=order_type, user_id=user.id
)
ticket_number: int = self.ticket_handler.create_ticket(
order=order, user_name=user.name, user_mail=user.email, order_type=order_type
Expand Down
6 changes: 1 addition & 5 deletions cg/server/endpoints/orders.py
Original file line number Diff line number Diff line change
Expand Up @@ -166,8 +166,6 @@ def submit_order(order_type: OrderType):
content=request_json, file_format=FileFormat.JSON
),
)
request_json["project_type"] = order_type
request_json["user_id"] = g.current_user.id

result: dict = api.submit(
raw_order=request_json,
Expand Down Expand Up @@ -257,9 +255,7 @@ def get_options():
@ORDERS_BLUEPRINT.route("/validate_order/<order_type>", methods=["POST"])
def validate_order(order_type: OrderType):
raw_order = request.get_json()
raw_order["project_type"] = order_type
raw_order["user_id"] = g.current_user.id
response = order_validation_service.get_validation_response(
raw_order=raw_order, order_type=order_type
raw_order=raw_order, order_type=order_type, user_id=g.current_user.id
)
return jsonify(response), HTTPStatus.OK
3 changes: 1 addition & 2 deletions cg/services/order_validation_service/models/order.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,14 +6,13 @@

class Order(BaseModel):
comment: str | None = None
connect_to_ticket: bool = False
customer: str = Field(min_length=1)
delivery_type: DataDelivery
order_type: OrderType = Field(alias="project_type")
name: str = Field(min_length=1)
skip_reception_control: bool = False
_generated_ticket_id: int | None = PrivateAttr(default=None)
user_id: int
_user_id: int = PrivateAttr(default=None)

@model_validator(mode="before")
def convert_empty_strings_to_none(cls, data):
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,17 +27,20 @@ class OrderValidationService:
def __init__(self, store: Store):
self.store = store

def get_validation_response(self, raw_order: dict, order_type: OrderType) -> dict:
def get_validation_response(self, raw_order: dict, order_type: OrderType, user_id: int) -> dict:
model = ORDER_TYPE_MODEL_MAP[order_type]
rule_set = ORDER_TYPE_RULE_SET_MAP[order_type]
errors = self._get_errors(raw_order=raw_order, model=model, rule_set=rule_set)
errors: ValidationErrors = self._get_errors(
raw_order=raw_order, model=model, rule_set=rule_set, user_id=user_id
)
return create_order_validation_response(raw_order=raw_order, errors=errors)

def _get_errors(
self, raw_order: dict, model: type[Order], rule_set: RuleSet
self, raw_order: dict, model: type[Order], rule_set: RuleSet, user_id: int
) -> ValidationErrors:
parsed_order, errors = ModelValidator.validate(order=raw_order, model=model)
if parsed_order:
parsed_order._user_id = user_id
errors: ValidationErrors = self._get_rule_validation_errors(
order=parsed_order, rule_set=rule_set
)
Expand Down Expand Up @@ -78,11 +81,12 @@ def _get_rule_validation_errors(self, order: Order, rule_set: RuleSet) -> Valida
sample_errors=sample_errors,
)

def parse_and_validate(self, raw_order: dict, order_type: OrderType) -> Order:
def parse_and_validate(self, raw_order: dict, order_type: OrderType, user_id: int) -> Order:
model = ORDER_TYPE_MODEL_MAP[order_type]
rule_set = ORDER_TYPE_RULE_SET_MAP[order_type]
parsed_order, errors = ModelValidator.validate(order=raw_order, model=model)
if parsed_order:
parsed_order._user_id = user_id
errors: ValidationErrors = self._get_rule_validation_errors(
order=parsed_order,
rule_set=rule_set,
Expand Down
19 changes: 5 additions & 14 deletions cg/services/order_validation_service/rules/order/rules.py
Original file line number Diff line number Diff line change
@@ -1,12 +1,9 @@
from cg.services.order_validation_service.errors.order_errors import (
CustomerCannotSkipReceptionControlError,
CustomerDoesNotExistError,
OrderError,
OrderNameRequiredError,
UserNotAssociatedWithCustomerError,
)
from cg.services.order_validation_service.models.order import Order
from cg.services.order_validation_service.rules.order.utils import is_order_name_missing
from cg.store.store import Store


Expand All @@ -22,27 +19,21 @@ def validate_customer_exists(
return errors


def validate_user_belongs_to_customer(order: Order, store: Store, **kwargs) -> list[OrderError]:
def validate_user_belongs_to_customer(
order: Order, store: Store, **kwargs
) -> list[UserNotAssociatedWithCustomerError]:
has_access: bool = store.is_user_associated_with_customer(
user_id=order.user_id,
user_id=order._user_id,
customer_internal_id=order.customer,
)

errors: list[OrderError] = []
errors: list[UserNotAssociatedWithCustomerError] = []
if not has_access:
error = UserNotAssociatedWithCustomerError()
errors.append(error)
return errors


def validate_name_required_for_new_order(order: Order, **kwargs) -> list[OrderNameRequiredError]:
errors: list[OrderNameRequiredError] = []
if is_order_name_missing(order):
error = OrderNameRequiredError()
errors.append(error)
return errors


def validate_customer_can_skip_reception_control(
order: Order,
store: Store,
Expand Down
9 changes: 0 additions & 9 deletions cg/services/order_validation_service/rules/order/utils.py

This file was deleted.

3 changes: 1 addition & 2 deletions tests/services/order_validation_service/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -44,14 +44,13 @@ def create_case(samples: list[TomteSample]) -> TomteCase:

def create_tomte_order(cases: list[TomteCase]) -> TomteOrder:
order = TomteOrder(
connect_to_ticket=True,
delivery_type=TomteDeliveryType.FASTQ,
name="order_name",
project_type=OrderType.TOMTE,
user_id=1,
customer="cust000",
cases=cases,
)
order._user_id = 1
order._generated_ticket_id = 123456
return order

Expand Down
65 changes: 54 additions & 11 deletions tests/services/order_validation_service/test_order_rules.py
Original file line number Diff line number Diff line change
@@ -1,21 +1,64 @@
from cg.services.order_validation_service.errors.order_errors import OrderNameRequiredError
from cg.services.order_validation_service.models.order import Order
from cg.services.order_validation_service.errors.order_errors import (
CustomerCannotSkipReceptionControlError,
CustomerDoesNotExistError,
UserNotAssociatedWithCustomerError,
)
from cg.services.order_validation_service.rules.order.rules import (
validate_name_required_for_new_order,
validate_customer_can_skip_reception_control,
validate_customer_exists,
validate_user_belongs_to_customer,
)
from cg.services.order_validation_service.workflows.tomte.models.order import TomteOrder
from cg.store.models import Customer
from cg.store.store import Store


def test_validate_customer_can_skip_reception_control(base_store: Store, valid_order: TomteOrder):
# GIVEN an order attempting to skip reception control from a not trusted customer
customer: Customer = base_store.get_customer_by_internal_id(valid_order.customer)
customer.is_trusted = False
valid_order.skip_reception_control = True

# WHEN validating that the customer can skip reception control
errors: list[CustomerCannotSkipReceptionControlError] = (
validate_customer_can_skip_reception_control(order=valid_order, store=base_store)
)

# THEN an error should be returned
assert errors

# THEN the error should concern the customer not being allowed to skip reception control
assert isinstance(errors[0], CustomerCannotSkipReceptionControlError)

def test_order_name_is_required(valid_order: Order):

# GIVEN an order that needs a name but is missing one
valid_order.name = None
valid_order.connect_to_ticket = False
def test_validate_customer_does_not_exist(base_store: Store, valid_order: TomteOrder):
# GIVEN an order from an unknown customer
valid_order.customer = "Unknown customer"

# WHEN validating the order
errors: list[OrderNameRequiredError] = validate_name_required_for_new_order(valid_order)
# WHEN validating that the customer exists
errors: list[CustomerDoesNotExistError] = validate_customer_exists(
order=valid_order, store=base_store
)

# THEN an error should be returned
assert errors

# THEN the error should be about the name
assert isinstance(errors[0], OrderNameRequiredError)
# THEN the error should concern the unknown customer
assert isinstance(errors[0], CustomerDoesNotExistError)


def test_validate_user_belongs_to_customer(base_store: Store, valid_order: TomteOrder):
# GIVEN an order for a customer which the logged-in user does not have access to
customer: Customer = base_store.get_customer_by_internal_id(valid_order.customer)
customer.users = []

# WHEN validating that the user belongs to the customer account
errors: list[UserNotAssociatedWithCustomerError] = validate_user_belongs_to_customer(
order=valid_order, store=base_store
)

# THEN an error should be raised
assert errors

# THEN the error should concern the user not belonging to the customer
assert isinstance(errors[0], UserNotAssociatedWithCustomerError)
Original file line number Diff line number Diff line change
Expand Up @@ -43,16 +43,16 @@ def create_case(samples: list[BalsamicSample]) -> BalsamicCase:


def create_order(cases: list[BalsamicCase]) -> BalsamicOrder:
return BalsamicOrder(
connect_to_ticket=True,
order = BalsamicOrder(
delivery_type=BalsamicDeliveryType.FASTQ_ANALYSIS,
name="order_name",
ticket_number="#12345",
project_type=OrderType.BALSAMIC,
user_id=1,
customer="cust000",
cases=cases,
)
order._user_id = 1
order._generated_ticket_id = 12345
return order


@pytest.fixture
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ def test_valid_order(
raw_order=valid_order.model_dump(by_alias=True),
model=BalsamicOrder,
rule_set=balsamic_rule_set,
user_id=valid_order._user_id,
)

# THEN no errors should be raised
Expand All @@ -35,7 +36,7 @@ def test_valid_order_conversion(

# WHEN validating the order
response = balsamic_validation_service.get_validation_response(
raw_order=order, order_type=OrderType.BALSAMIC
raw_order=order, order_type=OrderType.BALSAMIC, user_id=valid_order._user_id
)

# THEN a response should be given
Expand All @@ -53,7 +54,7 @@ def test_order_error_conversion(

# WHEN validating the order
response: dict = balsamic_validation_service.get_validation_response(
raw_order=order, order_type=OrderType.BALSAMIC
raw_order=order, order_type=OrderType.BALSAMIC, user_id=valid_order._user_id
)

# THEN there should be an error for the missing name
Expand All @@ -71,7 +72,7 @@ def test_case_error_conversion(

# WHEN validating the order
response: dict = balsamic_validation_service.get_validation_response(
raw_order=order, order_type=OrderType.BALSAMIC
raw_order=order, order_type=OrderType.BALSAMIC, user_id=valid_order._user_id
)

# THEN there should be an error for the faulty priority
Expand All @@ -89,7 +90,7 @@ def test_sample_error_conversion(

# WHEN validating the order
response = balsamic_validation_service.get_validation_response(
raw_order=invalid_order, order_type=OrderType.BALSAMIC
raw_order=invalid_order, order_type=OrderType.BALSAMIC, user_id=valid_order._user_id
)

# THEN an error should be returned regarding the invalid volume
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,10 @@ def test_valid_order(

# WHEN validating the order
errors = tomte_validation_service._get_errors(
raw_order=valid_order.model_dump(by_alias=True), model=TomteOrder, rule_set=tomte_rule_set
raw_order=valid_order.model_dump(by_alias=True),
model=TomteOrder,
rule_set=tomte_rule_set,
user_id=valid_order._user_id,
)

# THEN no errors should be raised
Expand All @@ -33,7 +36,7 @@ def test_valid_order_conversion(

# WHEN validating the order
response = tomte_validation_service.get_validation_response(
raw_order=order, order_type=OrderType.TOMTE
raw_order=order, order_type=OrderType.TOMTE, user_id=valid_order._user_id
)

# THEN a response should be given
Expand All @@ -51,7 +54,7 @@ def test_order_error_conversion(

# WHEN validating the order
response: dict = tomte_validation_service.get_validation_response(
raw_order=order, order_type=OrderType.TOMTE
raw_order=order, order_type=OrderType.TOMTE, user_id=valid_order._user_id
)

# THEN there should be an error for the missing name
Expand All @@ -66,7 +69,7 @@ def test_case_error_conversion(valid_order, tomte_validation_service: OrderValid

# WHEN validating the order
response: dict = tomte_validation_service.get_validation_response(
raw_order=order, order_type=OrderType.TOMTE
raw_order=order, order_type=OrderType.TOMTE, user_id=valid_order._user_id
)

# THEN there should be an error for the faulty priority
Expand All @@ -84,7 +87,7 @@ def test_sample_error_conversion(

# WHEN validating the order
response = tomte_validation_service.get_validation_response(
raw_order=invalid_order, order_type=OrderType.TOMTE
raw_order=invalid_order, order_type=OrderType.TOMTE, user_id=valid_order._user_id
)

# THEN an error should be returned regarding the invalid volume
Expand Down
Loading