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) Make ticket id a private attribute #4032

Merged
merged 5 commits into from
Dec 19, 2024
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
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

islean marked this conversation as resolved.
Show resolved Hide resolved
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
Loading