Skip to content

Commit

Permalink
(Improve order flow) Make ticket id a private attribute (#4032) (patch)
Browse files Browse the repository at this point in the history
### Fixed

- _generated_ticket_id is a private attribute on the Order model
  • Loading branch information
islean authored Dec 19, 2024
1 parent bb78b45 commit 42221b0
Show file tree
Hide file tree
Showing 9 changed files with 11 additions and 56 deletions.
5 changes: 0 additions & 5 deletions cg/services/order_validation_service/errors/order_errors.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,11 +11,6 @@ class UserNotAssociatedWithCustomerError(OrderError):
message: str = "User does not belong to customer"


class TicketNumberRequiredError(OrderError):
field: str = "ticket_number"
message: str = "Ticket number is required"


class CustomerCannotSkipReceptionControlError(OrderError):
field: str = "skip_reception_control"
message: str = "Customer cannot skip reception control"
Expand Down
6 changes: 2 additions & 4 deletions cg/services/order_validation_service/models/order.py
Original file line number Diff line number Diff line change
@@ -1,10 +1,8 @@
from pydantic import BaseModel, Field, model_validator
from pydantic import BaseModel, Field, PrivateAttr, model_validator

from cg.constants import DataDelivery
from cg.models.orders.constants import OrderType

TICKET_PATTERN = r"^#\d{4,}"


class Order(BaseModel):
comment: str | None = None
Expand All @@ -14,7 +12,7 @@ class Order(BaseModel):
order_type: OrderType = Field(alias="project_type")
name: str = Field(min_length=1)
skip_reception_control: bool = False
ticket_number: str | None = Field(None, pattern=TICKET_PATTERN)
_generated_ticket_id: int | None = PrivateAttr(default=None)
user_id: int

@model_validator(mode="before")
Expand Down
17 changes: 1 addition & 16 deletions cg/services/order_validation_service/rules/order/rules.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,14 +3,10 @@
CustomerDoesNotExistError,
OrderError,
OrderNameRequiredError,
TicketNumberRequiredError,
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,
is_ticket_number_missing,
)
from cg.services.order_validation_service.rules.order.utils import is_order_name_missing
from cg.store.store import Store


Expand Down Expand Up @@ -39,17 +35,6 @@ def validate_user_belongs_to_customer(order: Order, store: Store, **kwargs) -> l
return errors


def validate_ticket_number_required_if_connected(
order: Order,
**kwargs,
) -> list[TicketNumberRequiredError]:
errors: list[TicketNumberRequiredError] = []
if is_ticket_number_missing(order):
error = TicketNumberRequiredError()
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):
Expand Down
Original file line number Diff line number Diff line change
@@ -1,14 +1,11 @@
from cg.services.order_validation_service.rules.order.rules import (
validate_customer_can_skip_reception_control,
validate_customer_exists,
validate_ticket_number_required_if_connected,
validate_user_belongs_to_customer,
)


ORDER_RULES: list[callable] = [
validate_customer_can_skip_reception_control,
validate_customer_exists,
validate_ticket_number_required_if_connected,
validate_user_belongs_to_customer,
]
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ def create_maf_case(self, sample_obj: Sample, order: Order) -> None:

def store_items_in_status(self, order: FastqOrder) -> list[Sample]:
"""Store fastq samples in the status database including family connection and delivery"""
ticket_id: str | None = order.ticket_number
ticket_id: str | None = order._generated_ticket_id
customer: Customer = self.status_db.get_customer_by_internal_id(
customer_internal_id=order.customer
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ def _create_db_case_for_sample(
return case

def _create_db_order(self, order: MicrobialFastqOrder) -> Order:
ticket_id: str = order.ticket_number
ticket_id: str = order._generated_ticket_id
customer: Customer = self.status_db.get_customer_by_internal_id(
customer_internal_id=order.customer
)
Expand Down
4 changes: 2 additions & 2 deletions tests/fixture_plugins/orders_fixtures/status_data_fixtures.py
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ def valid_microbial_fastq_order(ticket_id: str) -> MicrobialFastqOrder:
samples=[sample_1, sample_2, sample_3],
name="MicrobialFastqOrder",
)
order.ticket_number = ticket_id
order._generated_ticket_id = ticket_id
return order


Expand Down Expand Up @@ -141,5 +141,5 @@ def tomte_status_data(
@pytest.fixture
def fastq_order(fastq_order_to_submit: dict) -> FastqOrder:
fastq_order = FastqOrder.model_validate(fastq_order_to_submit)
fastq_order.ticket_number = 123456
fastq_order._generated_ticket_id = 123456
return fastq_order
5 changes: 3 additions & 2 deletions tests/services/order_validation_service/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -42,16 +42,17 @@ def create_case(samples: list[TomteSample]) -> TomteCase:


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


@pytest.fixture
Expand Down
23 changes: 1 addition & 22 deletions tests/services/order_validation_service/test_order_rules.py
Original file line number Diff line number Diff line change
@@ -1,31 +1,10 @@
from cg.services.order_validation_service.errors.order_errors import (
OrderNameRequiredError,
TicketNumberRequiredError,
)
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.rules.order.rules import (
validate_name_required_for_new_order,
validate_ticket_number_required_if_connected,
)


def test_ticket_is_required(valid_order: Order):
# GIVEN an order that should be connected to a ticket but is missing a ticket number
valid_order.connect_to_ticket = True
valid_order.ticket_number = None

# WHEN validating the order
errors: list[TicketNumberRequiredError] = validate_ticket_number_required_if_connected(
valid_order
)

# THEN an error should be returned
assert errors

# THEN the error should be about the ticket number
assert isinstance(errors[0], TicketNumberRequiredError)


def test_order_name_is_required(valid_order: Order):

# GIVEN an order that needs a name but is missing one
Expand Down

0 comments on commit 42221b0

Please sign in to comment.