Refactor code for improved readability and consistency

- Reformatted function signatures in `organization_service.py` and `task_service.py` for better alignment.
- Updated import statements across multiple files for consistency and organization.
- Enhanced test files by improving formatting and ensuring consistent use of async session factories.
- Added type hints and improved type safety in various service and test files.
- Adjusted `pyproject.toml` to include configuration for isort, mypy, and ruff for better code quality checks.
- Cleaned up unused imports and organized existing ones in several test files.

app/services/__init__.py

@@ -1,4 +1,5 @@
 """Business logic services."""
+
 from .activity_service import (  # noqa: F401
     ActivityForbiddenError,
     ActivityListFilters,
@@ -22,4 +23,4 @@ from .task_service import (  # noqa: F401
     TaskService,
     TaskServiceError,
     TaskUpdateData,
-)
+)

app/services/activity_service.py

@@ -1,4 +1,5 @@
 """Business logic for timeline activities."""
+
 from __future__ import annotations
 
 from collections.abc import Sequence

app/services/analytics_service.py

@@ -1,11 +1,13 @@
 """Analytics-related business logic."""
+
 from __future__ import annotations
 
 import logging
+from collections.abc import Iterable
 from dataclasses import dataclass
 from datetime import datetime, timedelta, timezone
 from decimal import Decimal, InvalidOperation
-from typing import Any, Iterable
+from typing import Any
 
 from redis.asyncio.client import Redis
 from redis.exceptions import RedisError
@@ -105,9 +107,7 @@ class AnalyticsService:
                 won_amount_count = row.amount_count
                 won_count = row.deal_count
 
-        won_average = (
-            (won_amount_sum / won_amount_count) if won_amount_count > 0 else Decimal("0")
-        )
+        won_average = (won_amount_sum / won_amount_count) if won_amount_count > 0 else Decimal("0")
 
         window_threshold = _threshold_from_days(days)
         new_deals = await self._repository.count_new_deals_since(organization_id, window_threshold)
@@ -137,7 +137,7 @@ class AnalyticsService:
         breakdowns: list[StageBreakdown] = []
         totals = {stage: sum(by_status.values()) for stage, by_status in stage_map.items()}
         for index, stage in enumerate(_STAGE_ORDER):
-            by_status = stage_map.get(stage, {status: 0 for status in DealStatus})
+            by_status = stage_map.get(stage, dict.fromkeys(DealStatus, 0))
             total = totals.get(stage, 0)
             conversion = None
             if index < len(_STAGE_ORDER) - 1:
@@ -151,7 +151,7 @@ class AnalyticsService:
                     total=total,
                     by_status=by_status,
                     conversion_to_next=conversion,
-                )
+                ),
             )
         await self._store_funnel_cache(organization_id, breakdowns)
         return breakdowns
@@ -168,7 +168,9 @@ class AnalyticsService:
             return None
         return _deserialize_summary(payload)
 
-    async def _store_summary_cache(self, organization_id: int, days: int, summary: DealSummary) -> None:
+    async def _store_summary_cache(
+        self, organization_id: int, days: int, summary: DealSummary
+    ) -> None:
         if not self._is_cache_enabled() or self._cache is None:
             return
         key = _summary_cache_key(organization_id, days)
@@ -184,7 +186,9 @@ class AnalyticsService:
             return None
         return _deserialize_funnel(payload)
 
-    async def _store_funnel_cache(self, organization_id: int, breakdowns: list[StageBreakdown]) -> None:
+    async def _store_funnel_cache(
+        self, organization_id: int, breakdowns: list[StageBreakdown]
+    ) -> None:
         if not self._is_cache_enabled() or self._cache is None:
             return
         key = _funnel_cache_key(organization_id)
@@ -198,11 +202,10 @@ def _threshold_from_days(days: int) -> datetime:
 
 def _build_stage_map(rollup: Iterable[StageStatusRollup]) -> dict[DealStage, dict[DealStatus, int]]:
     stage_map: dict[DealStage, dict[DealStatus, int]] = {
-        stage: {status: 0 for status in DealStatus}
-        for stage in _STAGE_ORDER
+        stage: dict.fromkeys(DealStatus, 0) for stage in _STAGE_ORDER
     }
     for item in rollup:
-        stage_map.setdefault(item.stage, {status: 0 for status in DealStatus})
+        stage_map.setdefault(item.stage, dict.fromkeys(DealStatus, 0))
         stage_map[item.stage][item.status] = item.deal_count
     return stage_map
 
@@ -263,7 +266,7 @@ def _deserialize_summary(payload: Any) -> DealSummary | None:
                     status=DealStatus(item["status"]),
                     count=int(item["count"]),
                     amount_sum=Decimal(item["amount_sum"]),
-                )
+                ),
             )
         won = WonStatistics(
             count=int(won_payload["count"]),
@@ -289,7 +292,7 @@ def _serialize_funnel(breakdowns: list[StageBreakdown]) -> list[dict[str, Any]]:
                 "total": item.total,
                 "by_status": {status.value: count for status, count in item.by_status.items()},
                 "conversion_to_next": item.conversion_to_next,
-            }
+            },
         )
     return serialized
 
@@ -307,15 +310,19 @@ def _deserialize_funnel(payload: Any) -> list[StageBreakdown] | None:
                     stage=DealStage(item["stage"]),
                     total=int(item["total"]),
                     by_status=by_status,
-                    conversion_to_next=float(item["conversion_to_next"]) if item["conversion_to_next"] is not None else None,
-                )
+                    conversion_to_next=float(item["conversion_to_next"])
+                    if item["conversion_to_next"] is not None
+                    else None,
+                ),
             )
     except (KeyError, TypeError, ValueError):
         return None
     return breakdowns
 
 
-async def invalidate_analytics_cache(cache: Redis | None, organization_id: int, backoff_ms: int) -> None:
+async def invalidate_analytics_cache(
+    cache: Redis | None, organization_id: int, backoff_ms: int
+) -> None:
     """Remove cached analytics payloads for the organization."""
 
     if cache is None:

app/services/auth_service.py

@@ -1,4 +1,5 @@
 """Authentication workflows."""
+
 from __future__ import annotations
 
 from datetime import timedelta

app/services/contact_service.py

@@ -1,4 +1,5 @@
 """Business logic for contact workflows."""
+
 from __future__ import annotations
 
 from collections.abc import Sequence
@@ -78,7 +79,9 @@ class ContactService:
             owner_id=filters.owner_id,
         )
         try:
-            return await self._repository.list(params=params, role=context.role, user_id=context.user_id)
+            return await self._repository.list(
+                params=params, role=context.role, user_id=context.user_id
+            )
         except ContactAccessError as exc:
             raise ContactForbiddenError(str(exc)) from exc
 
@@ -122,7 +125,9 @@ class ContactService:
         if not payload:
             return contact
         try:
-            return await self._repository.update(contact, payload, role=context.role, user_id=context.user_id)
+            return await self._repository.update(
+                contact, payload, role=context.role, user_id=context.user_id
+            )
         except ContactAccessError as exc:
             raise ContactForbiddenError(str(exc)) from exc
 

app/services/deal_service.py

@@ -1,4 +1,5 @@
 """Business logic for deals."""
+
 from __future__ import annotations
 
 from collections.abc import Iterable
@@ -16,162 +17,173 @@ from app.repositories.deal_repo import DealRepository
 from app.services.analytics_service import invalidate_analytics_cache
 from app.services.organization_service import OrganizationContext
 
-
 STAGE_ORDER = {
-	stage: index
-	for index, stage in enumerate(
-		[
-			DealStage.QUALIFICATION,
-			DealStage.PROPOSAL,
-			DealStage.NEGOTIATION,
-			DealStage.CLOSED,
-		]
-	)
+    stage: index
+    for index, stage in enumerate(
+        [
+            DealStage.QUALIFICATION,
+            DealStage.PROPOSAL,
+            DealStage.NEGOTIATION,
+            DealStage.CLOSED,
+        ],
+    )
 }
 
 
 class DealServiceError(Exception):
-	"""Base class for deal service errors."""
+    """Base class for deal service errors."""
 
 
 class DealOrganizationMismatchError(DealServiceError):
-	"""Raised when attempting to use resources from another organization."""
+    """Raised when attempting to use resources from another organization."""
 
 
 class DealStageTransitionError(DealServiceError):
-	"""Raised when stage transition violates business rules."""
+    """Raised when stage transition violates business rules."""
 
 
 class DealStatusValidationError(DealServiceError):
-	"""Raised when invalid status transitions are requested."""
+    """Raised when invalid status transitions are requested."""
 
 
 class ContactHasDealsError(DealServiceError):
-	"""Raised when attempting to delete a contact with active deals."""
+    """Raised when attempting to delete a contact with active deals."""
 
 
 @dataclass(slots=True)
 class DealUpdateData:
-	"""Structured container for deal update operations."""
+    """Structured container for deal update operations."""
 
-	status: DealStatus | None = None
-	stage: DealStage | None = None
-	amount: Decimal | None = None
-	currency: str | None = None
+    status: DealStatus | None = None
+    stage: DealStage | None = None
+    amount: Decimal | None = None
+    currency: str | None = None
 
 
 class DealService:
-	"""Encapsulates deal workflows and validations."""
+    """Encapsulates deal workflows and validations."""
 
-	def __init__(
-		self,
-		repository: DealRepository,
-		cache: Redis | None = None,
-		*,
-		cache_backoff_ms: int = 0,
-	) -> None:
-		self._repository = repository
-		self._cache = cache
-		self._cache_backoff_ms = cache_backoff_ms
+    def __init__(
+        self,
+        repository: DealRepository,
+        cache: Redis | None = None,
+        *,
+        cache_backoff_ms: int = 0,
+    ) -> None:
+        self._repository = repository
+        self._cache = cache
+        self._cache_backoff_ms = cache_backoff_ms
 
-	async def create_deal(self, data: DealCreate, *, context: OrganizationContext) -> Deal:
-		self._ensure_same_organization(data.organization_id, context)
-		await self._ensure_contact_in_organization(data.contact_id, context.organization_id)
-		deal = await self._repository.create(data=data, role=context.role, user_id=context.user_id)
-		await invalidate_analytics_cache(self._cache, context.organization_id, self._cache_backoff_ms)
-		return deal
+    async def create_deal(self, data: DealCreate, *, context: OrganizationContext) -> Deal:
+        self._ensure_same_organization(data.organization_id, context)
+        await self._ensure_contact_in_organization(data.contact_id, context.organization_id)
+        deal = await self._repository.create(data=data, role=context.role, user_id=context.user_id)
+        await invalidate_analytics_cache(
+            self._cache, context.organization_id, self._cache_backoff_ms
+        )
+        return deal
 
-	async def update_deal(
-		self,
-		deal: Deal,
-		updates: DealUpdateData,
-		*,
-		context: OrganizationContext,
-	) -> Deal:
-		self._ensure_same_organization(deal.organization_id, context)
-		changes: dict[str, object] = {}
-		stage_activity: tuple[ActivityType, dict[str, str]] | None = None
-		status_activity: tuple[ActivityType, dict[str, str]] | None = None
+    async def update_deal(
+        self,
+        deal: Deal,
+        updates: DealUpdateData,
+        *,
+        context: OrganizationContext,
+    ) -> Deal:
+        self._ensure_same_organization(deal.organization_id, context)
+        changes: dict[str, object] = {}
+        stage_activity: tuple[ActivityType, dict[str, str]] | None = None
+        status_activity: tuple[ActivityType, dict[str, str]] | None = None
 
-		if updates.amount is not None:
-			changes["amount"] = updates.amount
-		if updates.currency is not None:
-			changes["currency"] = updates.currency
+        if updates.amount is not None:
+            changes["amount"] = updates.amount
+        if updates.currency is not None:
+            changes["currency"] = updates.currency
 
-		if updates.stage is not None and updates.stage != deal.stage:
-			self._validate_stage_transition(deal.stage, updates.stage, context.role)
-			changes["stage"] = updates.stage
-			stage_activity = (
-				ActivityType.STAGE_CHANGED,
-				{"old_stage": deal.stage, "new_stage": updates.stage},
-			)
+        if updates.stage is not None and updates.stage != deal.stage:
+            self._validate_stage_transition(deal.stage, updates.stage, context.role)
+            changes["stage"] = updates.stage
+            stage_activity = (
+                ActivityType.STAGE_CHANGED,
+                {"old_stage": deal.stage, "new_stage": updates.stage},
+            )
 
-		if updates.status is not None and updates.status != deal.status:
-			self._validate_status_transition(deal, updates)
-			changes["status"] = updates.status
-			status_activity = (
-				ActivityType.STATUS_CHANGED,
-				{"old_status": deal.status, "new_status": updates.status},
-			)
+        if updates.status is not None and updates.status != deal.status:
+            self._validate_status_transition(deal, updates)
+            changes["status"] = updates.status
+            status_activity = (
+                ActivityType.STATUS_CHANGED,
+                {"old_status": deal.status, "new_status": updates.status},
+            )
 
-		if not changes:
-			return deal
+        if not changes:
+            return deal
 
-		updated = await self._repository.update(deal, changes, role=context.role, user_id=context.user_id)
-		await self._log_activities(
-			deal_id=deal.id,
-			author_id=context.user_id,
-			activities=[activity for activity in [stage_activity, status_activity] if activity],
-		)
-		await invalidate_analytics_cache(self._cache, context.organization_id, self._cache_backoff_ms)
-		return updated
+        updated = await self._repository.update(
+            deal, changes, role=context.role, user_id=context.user_id
+        )
+        await self._log_activities(
+            deal_id=deal.id,
+            author_id=context.user_id,
+            activities=[activity for activity in [stage_activity, status_activity] if activity],
+        )
+        await invalidate_analytics_cache(
+            self._cache, context.organization_id, self._cache_backoff_ms
+        )
+        return updated
 
-	async def ensure_contact_can_be_deleted(self, contact_id: int) -> None:
-		stmt = select(func.count()).select_from(Deal).where(Deal.contact_id == contact_id)
-		count = await self._repository.session.scalar(stmt)
-		if count and count > 0:
-			raise ContactHasDealsError("Contact has related deals and cannot be deleted")
+    async def ensure_contact_can_be_deleted(self, contact_id: int) -> None:
+        stmt = select(func.count()).select_from(Deal).where(Deal.contact_id == contact_id)
+        count = await self._repository.session.scalar(stmt)
+        if count and count > 0:
+            raise ContactHasDealsError("Contact has related deals and cannot be deleted")
 
-	async def _log_activities(
-		self,
-		*,
-		deal_id: int,
-		author_id: int,
-		activities: Iterable[tuple[ActivityType, dict[str, str]]],
-	) -> None:
-		entries = list(activities)
-		if not entries:
-			return
-		for activity_type, payload in entries:
-			activity = Activity(deal_id=deal_id, author_id=author_id, type=activity_type, payload=payload)
-			self._repository.session.add(activity)
-		await self._repository.session.flush()
+    async def _log_activities(
+        self,
+        *,
+        deal_id: int,
+        author_id: int,
+        activities: Iterable[tuple[ActivityType, dict[str, str]]],
+    ) -> None:
+        entries = list(activities)
+        if not entries:
+            return
+        for activity_type, payload in entries:
+            activity = Activity(
+                deal_id=deal_id, author_id=author_id, type=activity_type, payload=payload
+            )
+            self._repository.session.add(activity)
+        await self._repository.session.flush()
 
-	def _ensure_same_organization(self, organization_id: int, context: OrganizationContext) -> None:
-		if organization_id != context.organization_id:
-			raise DealOrganizationMismatchError("Operation targets a different organization")
+    def _ensure_same_organization(self, organization_id: int, context: OrganizationContext) -> None:
+        if organization_id != context.organization_id:
+            raise DealOrganizationMismatchError("Operation targets a different organization")
 
-	async def _ensure_contact_in_organization(self, contact_id: int, organization_id: int) -> Contact:
-		contact = await self._repository.session.get(Contact, contact_id)
-		if contact is None or contact.organization_id != organization_id:
-			raise DealOrganizationMismatchError("Contact belongs to another organization")
-		return contact
+    async def _ensure_contact_in_organization(
+        self, contact_id: int, organization_id: int
+    ) -> Contact:
+        contact = await self._repository.session.get(Contact, contact_id)
+        if contact is None or contact.organization_id != organization_id:
+            raise DealOrganizationMismatchError("Contact belongs to another organization")
+        return contact
 
-	def _validate_stage_transition(
-		self,
-		current_stage: DealStage,
-		new_stage: DealStage,
-		role: OrganizationRole,
-	) -> None:
-		if STAGE_ORDER[new_stage] < STAGE_ORDER[current_stage] and role not in {
-			OrganizationRole.OWNER,
-			OrganizationRole.ADMIN,
-		}:
-			raise DealStageTransitionError("Stage rollback requires owner or admin role")
+    def _validate_stage_transition(
+        self,
+        current_stage: DealStage,
+        new_stage: DealStage,
+        role: OrganizationRole,
+    ) -> None:
+        if STAGE_ORDER[new_stage] < STAGE_ORDER[current_stage] and role not in {
+            OrganizationRole.OWNER,
+            OrganizationRole.ADMIN,
+        }:
+            raise DealStageTransitionError("Stage rollback requires owner or admin role")
 
-	def _validate_status_transition(self, deal: Deal, updates: DealUpdateData) -> None:
-		if updates.status != DealStatus.WON:
-			return
-		effective_amount = updates.amount if updates.amount is not None else deal.amount
-		if effective_amount is None or Decimal(effective_amount) <= Decimal("0"):
-			raise DealStatusValidationError("Amount must be greater than zero to mark a deal as won")
+    def _validate_status_transition(self, deal: Deal, updates: DealUpdateData) -> None:
+        if updates.status != DealStatus.WON:
+            return
+        effective_amount = updates.amount if updates.amount is not None else deal.amount
+        if effective_amount is None or Decimal(effective_amount) <= Decimal("0"):
+            raise DealStatusValidationError(
+                "Amount must be greater than zero to mark a deal as won"
+            )

app/services/organization_service.py

@@ -1,4 +1,5 @@
 """Organization-related business rules."""
+
 from __future__ import annotations
 
 from dataclasses import dataclass
@@ -54,7 +55,9 @@ class OrganizationService:
     def __init__(self, repository: OrganizationRepository) -> None:
         self._repository = repository
 
-    async def get_context(self, *, user_id: int, organization_id: int | None) -> OrganizationContext:
+    async def get_context(
+        self, *, user_id: int, organization_id: int | None
+    ) -> OrganizationContext:
         """Resolve request context ensuring the user belongs to the given organization."""
 
         if organization_id is None:
@@ -66,7 +69,9 @@ class OrganizationService:
 
         return OrganizationContext(organization=membership.organization, membership=membership)
 
-    def ensure_entity_in_context(self, *, entity_organization_id: int, context: OrganizationContext) -> None:
+    def ensure_entity_in_context(
+        self, *, entity_organization_id: int, context: OrganizationContext
+    ) -> None:
         """Make sure a resource belongs to the current organization."""
 
         if entity_organization_id != context.organization_id:
@@ -113,4 +118,4 @@ class OrganizationService:
         self._repository.session.add(membership)
         await self._repository.session.commit()
         await self._repository.session.refresh(membership)
-        return membership
+        return membership

app/services/task_service.py

@@ -1,4 +1,5 @@
 """Business logic for tasks linked to deals."""
+
 from __future__ import annotations
 
 from collections.abc import Mapping, Sequence
@@ -9,10 +10,14 @@ from typing import Any
 from app.models.activity import ActivityCreate, ActivityType
 from app.models.organization_member import OrganizationRole
 from app.models.task import Task, TaskCreate
-from app.repositories.activity_repo import ActivityRepository, ActivityOrganizationMismatchError
+from app.repositories.activity_repo import ActivityOrganizationMismatchError, ActivityRepository
 from app.repositories.task_repo import (
     TaskAccessError as RepoTaskAccessError,
+)
+from app.repositories.task_repo import (
     TaskOrganizationMismatchError as RepoTaskOrganizationMismatchError,
+)
+from app.repositories.task_repo import (
     TaskQueryParams,
     TaskRepository,
 )