Code Review – src/presentation/bot/middleware/user_and_locale.py

[itisnotyourenv]

🔧 Review Summary: src/presentation/bot/middleware/user_and_locale.py

• File Purpose: Aiogram middleware that loads user profile from database and injects localized i18n translator into handler data. Provides unified user context (profile and i18n) for callback queries and message handlers.
• Reviewer: Claude Code AI
• Review Date: 2026-02-04

---

📁 File Purpose & System Integration

This middleware is part of the language onboarding feature (branch feat/user-language-onboarding). It:

1. Intercepts Telegram events (messages, callback queries) before they reach handlers
2. Loads user profile via GetUserProfileInteractor from the database
3. Injects i18n translator based on user's saved language preference
4. Handles failures gracefully by responding with localized error messages and stopping handler execution

How it fits in the system:

• Registered in main.py for dp.message and dp.callback_query event types
• Replaces/complements the older I18nMiddleware in src/infrastructure/i18n/middleware.py
• Works with GetUserProfileInteractor from application layer
• Uses dishka DI container for dependency injection

---

✅ Strengths

1. Clean separation of concerns - middleware handles cross-cutting concern (loading user context)
2. Proper error handling - catches ApplicationError and provides user-friendly feedback
3. Event-type-aware responses - different error handling for CallbackQuery vs InlineQuery
4. Logging - appropriate warning logs with context for debugging
5. Type hints - good use of union types and type casting

---

⚠️ Issues & Recommendations

Critical

• Missing translation key error_loading_profile - The middleware calls i18n.error_loading_profile() but this translation key is not defined in any .ftl file (locales/en/common.ftl and locales/ru/common.ftl are empty). This will cause a runtime error or return the key name as-is.

Important

• Potential AttributeError on from_user - Line 50 accesses user_event.from_user.id without null check. While the middleware is only registered for events that should have from_user, the type UserEvent includes ChosenInlineResult which theoretically could have edge cases. Consider adding a guard.

• Duplicate middleware functionality - I18nMiddleware in src/infrastructure/i18n/middleware.py provides similar i18n injection. Having two middlewares that both set data["i18n"] could cause confusion or conflicts. Consider deprecating/removing the older middleware or documenting the relationship clearly.

• No unit tests - No tests exist for this middleware. Bot middleware is testable by mocking the handler, event, and container. Missing test coverage for:

  • Happy path (user found, i18n injected)
  • User not found error handling for CallbackQuery
  • User not found error handling for InlineQuery
  • Error handling for other event types (ChosenInlineResult)

Minor

• Hardcoded fallback behavior - On error, InlineQuery gets an empty result with 1-second cache. Consider making cache time configurable or using a constant.

• Silent handling for ChosenInlineResult - The comment says "just skip silently" but logging would help debugging. Consider adding a debug log.

• profile variable might be referenced before assignment - If ApplicationError is raised, profile is never set, but this is handled by early return. However, this pattern could be clearer with an early assignment or restructuring.

• Container key access data[CONTAINER_NAME] - Could raise KeyError if dishka middleware isn't registered. Consider using .get() with appropriate error handling.

Code Style

• Docstring could be more detailed - The class docstring is good, but individual method docstrings are missing. Consider adding docstring to __call__ explaining parameters.

• Consider extracting error handling into helper methods - The error handling block (lines 61-66) could be cleaner as a private method _handle_error_response(user_event, i18n).

---

🏗️ Architectural Considerations

1. Middleware vs. Dependency Injection: The middleware pattern works well here, but consider whether the profile could instead be provided via dishka dependency injection for handlers that need it, allowing optional usage.

2. Coupling to GetUserProfileInteractor: The middleware is tightly coupled to this specific interactor. If profile requirements change, this middleware must change too.

3. Error recovery: Currently, any ApplicationError stops handler execution entirely. Consider whether some errors (e.g., database timeout) should allow retry or fallback behavior.

---

🧪 Test Coverage Needed

# Suggested test file: tests/unit/presentation/bot/middleware/test_user_and_locale.py

class TestUserAndLocaleMiddleware:
    async def test_happy_path_injects_profile_and_i18n(self): ...
    async def test_callback_query_error_shows_alert(self): ...
    async def test_inline_query_error_returns_empty(self): ...
    async def test_chosen_inline_result_error_returns_none(self): ...
    async def test_uses_telegram_language_on_error(self): ...

---

📝 Action Items Summary

Priority	Issue	Effort
🔴 Critical	Add missing error_loading_profile translation key	Low
🟡 Important	Add unit tests	Medium
🟡 Important	Clarify relationship with I18nMiddleware	Low
🟡 Important	Add null check for from_user	Low
🟢 Minor	Add debug logging for silent error cases	Low
🟢 Minor	Extract error handling to helper method	Low