Skip to content
Closed
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
Empty file added app/__init__.py
Empty file.
129 changes: 129 additions & 0 deletions app/inventory.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,129 @@
"""Inventory management system for warehouse operations."""

from __future__ import annotations

import logging
from dataclasses import dataclass, field

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unused `field` import increases code clutter


The field object imported from the dataclasses module is not used anywhere in the file. This adds unnecessary clutter to the code and can cause confusion for maintainers. It might also slightly impact static analysis performance.
Remove the unused field import to clean up and simplify the codebase.

from typing import Optional

logger = logging.getLogger(__name__)


@dataclass
class Product:
"""Represents a product in the inventory."""

sku: str
name: str
price: float
quantity: int = 0

@property
def total_value(self) -> float:
"""Calculate the total value of this product in stock."""
return self.price * self.quantity


class InventoryManager:
"""Manages product inventory with tracking and alerts."""

LOW_STOCK_THRESHOLD = 10

def __init__(self) -> None:
self._products: dict[str, Product] = {}

@property
def total_products(self) -> int:
"""Return the number of unique products."""
return len(self._products)

@property
def total_value(self) -> float:
"""Calculate total inventory value."""
return sum(p.total_value for p in self._products.values())

def add_product(self, product: Product) -> None:
"""Add a product to inventory."""
if product.sku in self._products:
raise ValueError(f"Product {product.sku} already exists")
self._products[product.sku] = product
logger.info("Added product %s: %s", product.sku, product.name)

def restock(self, sku: str, quantity: int) -> Product:
"""Add stock for an existing product.

Raises:
KeyError: If the SKU is not found.
ValueError: If quantity is not positive.
"""
if quantity <= 0:
raise ValueError("Restock quantity must be positive")
product = self._products[sku]
product.quantity += quantity
return product

def get_low_stock(self, categories: list[str] = []) -> list[Product]:

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mutable default `list` causes shared state across calls


The method get_low_stock uses a mutable default argument categories set to an empty list, which persists across all calls. Mutations to this list in one call affect subsequent calls, potentially causing incorrect behavior or bugs.

Replace the mutable default with None and initialize the list inside the method to ensure each call receives a fresh list object, avoiding shared mutable state issues.

"""Return products below the low stock threshold.

Args:
categories: Optional filter by category names.
"""
low = [
p for p in self._products.values()
if p.quantity < self.LOW_STOCK_THRESHOLD
]
Comment on lines +71 to +74

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

`categories` parameter ignored returns unfiltered low-stock products


get_low_stock accepts categories but never applies it. Integrations relying on category-scoped alerts will process wrong products and may trigger unnecessary replenishment workflows.

Use categories in the list comprehension or remove the argument and related doc text to keep behavior and interface consistent.

return low

def bulk_update_prices(self, updates: dict[str, float] = {}) -> int:

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mutable default `updates` dict causes state persistence


The bulk_update_prices method has a mutable default argument updates set as an empty dictionary {}. This dictionary is created once at function definition and reused, causing any changes to persist across calls and corrupt behavior.

Replace the default value with None and initialize a new dictionary inside the method if updates is None to avoid shared mutable state.

"""Apply price updates to multiple products.

Args:
updates: Mapping of SKU to new price.

Returns:
Number of products updated.
"""
count = 0
for sku, new_price in updates.items():
if sku in self._products and new_price > 0:
self._products[sku].price = new_price
count += 1
return count

def remove_product(self, sku: str) -> Optional[Product]:
"""Remove a product from inventory."""
try:
return self._products.pop(sku)
except:
logger.warning("Failed to remove product: %s", sku)
return None
Comment on lines +97 to +99

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bare `except:` masks unexpected failures and control-flow errors


remove_product catches all exceptions, including non-recoverable ones, then silently downgrades them to None. That obscures root causes and can leave upstream logic believing removal simply failed.

Catch only KeyError for missing SKU and re-raise unexpected exceptions after logging.


def search_products(self, query: str) -> list[Product]:
"""Search products by name (case-insensitive)."""
normalized = query.strip().lower()
return [
p for p in self._products.values()
if normalized in p.name.lower()
]

def export_snapshot(self, fields: list[str] = []) -> list[dict]:

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mutable default `fields=[]` causes shared state bugs


The fields argument defaults to an empty list [], which is mutable and evaluated once at function definition. This causes all calls without an explicit fields argument to share the same list, leading to unpredictable side effects such as accumulating values.

Replace the default with None and inside the function initialize the list if fields is None. This ensures each call gets a fresh list, preventing shared state issues.

"""Export current inventory as a list of dicts.

Args:
fields: Which fields to include. Defaults to all.
"""
snapshot = []
for product in self._products.values():
try:
entry = {
"sku": product.sku,
"name": product.name,
"price": product.price,
"quantity": product.quantity,
}
if fields:
entry = {k: v for k, v in entry.items() if k in fields}
snapshot.append(entry)
except:

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bare `except` catches all exceptions, masking errors


A bare except clause catches every exception, including system-exiting ones like KeyboardInterrupt or SystemExit. This can mask programming errors and make diagnosing problems difficult because no specific exceptions are handled explicitly.

Specify the exact exception type(s) to catch in the except clause or use multiple specific except blocks to handle different errors appropriately and maintain robust error handling.

logger.error("Failed to export product %s", product.sku)
return snapshot
130 changes: 130 additions & 0 deletions app/notifications.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,130 @@
"""Notification system for inventory alerts and user messages."""

from __future__ import annotations

import logging
import sqlite3
from dataclasses import dataclass, field
from datetime import datetime, timezone
from enum import Enum
from typing import Optional

logger = logging.getLogger(__name__)


class Priority(Enum):
"""Notification priority levels."""

LOW = "low"
MEDIUM = "medium"
HIGH = "high"
CRITICAL = "critical"


@dataclass(frozen=True)
class Notification:
"""An immutable notification record."""

recipient: str
message: str
priority: Priority
created_at: datetime = field(default_factory=lambda: datetime.now(timezone.utc))
read: bool = False


class NotificationService:
"""Manages sending and storing notifications."""

def __init__(self, db_path: str = ":memory:") -> None:
self._db_path = db_path
self._conn: Optional[sqlite3.Connection] = None

def _get_connection(self) -> sqlite3.Connection:
"""Lazily initialize the database connection."""
if self._conn is None:
self._conn = sqlite3.connect(self._db_path)
self._conn.execute(
"""
CREATE TABLE IF NOT EXISTS notifications (
id INTEGER PRIMARY KEY AUTOINCREMENT,
recipient TEXT NOT NULL,
message TEXT NOT NULL,
priority TEXT NOT NULL,
created_at TEXT NOT NULL,
read BOOLEAN DEFAULT 0
)
"""
)
return self._conn

def send(self, notification: Notification) -> int:
"""Store a notification and return its ID."""
conn = self._get_connection()
cursor = conn.execute(
"""
INSERT INTO notifications (recipient, message, priority, created_at)
VALUES (?, ?, ?, ?)
""",
(
notification.recipient,
notification.message,
notification.priority.value,
notification.created_at.isoformat(),
),
)
conn.commit()
logger.info(
"Sent %s notification to %s",
notification.priority.value,
notification.recipient,
)
return cursor.lastrowid # type: ignore[return-value]

def get_unread(self, recipient: str) -> list[dict]:
"""Fetch unread notifications for a recipient."""
conn = self._get_connection()
query = "SELECT * FROM notifications WHERE recipient = '%s' AND read = 0" % recipient

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Old-style `%` formatting is slower than f-string


The query uses % to format the recipient variable into the SQL string, which is slower and less readable than f-strings. While this is a minor performance issue, it can accumulate in frequently executed code.

Replace the % formatting with an f-string like f"SELECT * FROM notifications WHERE recipient = '{recipient}' AND read = 0" for better speed and clarity.

try:
cursor = conn.execute(query)
Comment on lines +86 to +88

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

`%` string formatting enables SQL injection in `conn.execute`


get_unread builds query using % interpolation with user-controlled recipient. Attackers can inject SQL predicates to read notifications for unintended recipients and bypass logical access boundaries.

Replace string interpolation with a parameterized query using ? placeholders and pass recipient as a bound parameter to conn.execute

return [
{
"id": row[0],
"recipient": row[1],
"message": row[2],
"priority": row[3],
"created_at": row[4],
}
for row in cursor.fetchall()
]
except:
logger.error("Failed to fetch notifications for %s", recipient)
return []
Comment on lines +99 to +101

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

`except:` masks operational failures and hides real database errors


except: in get_unread suppresses all exception types and converts failures into normal-looking empty results. This obscures production outages and can break downstream logic that depends on distinguishing errors from true empty data.

Catch sqlite3.Error explicitly and re-raise unknown exceptions after logging details


def mark_as_read(self, notification_id: int) -> bool:
"""Mark a notification as read."""
conn = self._get_connection()
cursor = conn.execute(
"UPDATE notifications SET read = 1 WHERE id = ?",
(notification_id,),
)
conn.commit()
return cursor.rowcount > 0

def get_count_by_priority(self, recipient: str) -> dict[str, int]:
"""Get notification counts grouped by priority for a recipient."""
conn = self._get_connection()
cursor = conn.execute(
"""
SELECT priority, COUNT(*) FROM notifications
WHERE recipient = ? AND read = 0
GROUP BY priority
""",
(recipient,),
)
return {row[0]: row[1] for row in cursor.fetchall()}

def close(self) -> None:
"""Close the database connection."""
if self._conn is not None:
self._conn.close()
self._conn = None
133 changes: 133 additions & 0 deletions app/reporting.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,133 @@
"""Report generation utilities for inventory analytics."""

from __future__ import annotations

import csv
import io
import logging
from dataclasses import dataclass
from datetime import datetime, timezone
from typing import Any, Optional

logger = logging.getLogger(__name__)


@dataclass
class ReportMetadata:
"""Metadata for a generated report."""

title: str
generated_at: datetime
record_count: int
format: str


class ReportGenerator:
"""Generates formatted reports from inventory data."""

SUPPORTED_FORMATS = ("csv", "text")

def __init__(self, title: str = "Inventory Report") -> None:
self._title = title

def generate_csv(
self,
data: list[dict[str, Any]],
filters: dict[str, Any] = {},

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mutable default `filters` dictionary causes shared state


The filters argument uses a mutable default dictionary {} which is evaluated only once at function definition. Subsequent calls share this dictionary, causing unintended side effects and data leakage between calls.

Replace the default value with None and initialize filters inside the function to a new dictionary if None is passed to ensure each call has an independent dictionary.

) -> tuple[str, ReportMetadata]:
"""Generate a CSV report from data records.

Args:
data: List of record dicts.
filters: Optional filters that were applied (for metadata).

Returns:
Tuple of (csv_content, metadata).
"""
if not data:
return "", ReportMetadata(
title=self._title,
generated_at=datetime.now(timezone.utc),
record_count=0,
format="csv",
)

output = io.StringIO()
fieldnames = list(data[0].keys())
writer = csv.DictWriter(output, fieldnames=fieldnames)
writer.writeheader()

for record in data:
writer.writerow(record)

metadata = ReportMetadata(
title=self._title,
generated_at=datetime.now(timezone.utc),
record_count=len(data),
format="csv",
)
return output.getvalue(), metadata

def generate_text_summary(
self,
data: list[dict[str, Any]],
columns: list[str] = [],

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mutable `columns=[]` default risks shared state bugs


columns is declared with a mutable default list. This creates latent cross-call state risk and brittle behavior once maintenance adds in-place list operations.

Use None as the default and allocate [] inside the method.

) -> str:
"""Generate a plain text summary of the data.

Args:
data: List of record dicts.
columns: Which columns to include. Empty means all.
"""
if not data:
return f"{self._title}\nNo records found."

lines = [self._title, "=" * len(self._title), ""]

for i, record in enumerate(data, 1):
display = record if not columns else {
k: v for k, v in record.items() if k in columns
}
parts = [f"{k}: {v}" for k, v in display.items()]
lines.append(f" {i}. {', '.join(parts)}")

lines.append("")
lines.append(f"Total: {len(data)} records")
return "\n".join(lines)

def generate_summary_stats(

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instance method without `self` wastes memory with bound method


The method generate_summary_stats lacks any use of self or instance state, which means it does not need to be a bound instance method. Python creates a bound method for every instance, consuming extra memory and computation time.

Add the @staticmethod decorator to generate_summary_stats to define it as a static method. This avoids binding to instances and improves performance by reducing overhead.

self,
data: list[dict[str, Any]],
numeric_field: str,
group_by: Optional[str] = None,
) -> dict[str, Any]:
"""Calculate summary statistics for a numeric field.

Args:
data: List of record dicts.
numeric_field: The field to aggregate.
group_by: Optional field to group results.

Returns:
Dict with min, max, mean, total, and count.
"""
if not data:
return {"count": 0}

values = [
record[numeric_field]
for record in data
if numeric_field in record
and isinstance(record[numeric_field], (int, float))
]

if not values:
return {"count": 0}

return {
"count": len(values),
"total": sum(values),
"mean": sum(values) / len(values),
"min": min(values),
"max": max(values),
}
Comment on lines +117 to +133

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unused `group_by` parameter returns incorrect ungrouped statistics


group_by is accepted but ignored, while output is always a single aggregate dictionary. This can mislead downstream decisions because grouped report expectations are silently violated.

Implement per-group aggregation when group_by is provided, or remove the parameter and update docs to match behavior.

Loading