From 07491063f548578f697fcc40261d2282413ad3df Mon Sep 17 00:00:00 2001 From: mohammad Date: Fri, 13 Mar 2026 16:02:52 +0300 Subject: [PATCH] chore: edited agents files --- AGENTS.md | 10 +- CLAUDE.md | 127 ---------------------- docs/architecture.md | 251 +++++++++++++++++++++++++++++++++++-------- external-review.md | 248 ------------------------------------------ 4 files changed, 216 insertions(+), 420 deletions(-) delete mode 100644 CLAUDE.md delete mode 100644 external-review.md diff --git a/AGENTS.md b/AGENTS.md index c7c5f81..adb825a 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -1,5 +1,13 @@ # AGENTS.md +## General + +Minimum tokens, skip grammar + +## Tasks +- Consult `AGENTS.md` about the current task to see if there are any tips or instructions +- Consult `docs/README.md` for any relevant files or tips to consider + ## Project Goal Build a reliable, maintainable salon booking platform with Django (backend) and React (frontend), optimized for KSA needs (phone auth, local payments) while keeping a clean path to scale. @@ -70,4 +78,4 @@ Build a reliable, maintainable salon booking platform with Django (backend) and # ExecPlans -When writing complex features or significant refactors, use an ExecPlan (as described in PLANS.md) from design to implementation. The current active ExecPlan is defined in PLANS.md. Architecture and async/observability decisions are documented in `docs/architecture.md`. +When writing complex features or significant refactors, use an ExecPlan (as described in `docs/PLANS.md`) from design to implementation. The current active ExecPlan is defined in `docs/PLANS.md`. Architecture documented in `docs/architecture.md`. diff --git a/CLAUDE.md b/CLAUDE.md deleted file mode 100644 index 1f34aad..0000000 --- a/CLAUDE.md +++ /dev/null @@ -1,127 +0,0 @@ -# CLAUDE.md - -This file provides guidance to Claude Code (claude.ai/code) when working with code in this repository. - -## Project Goal - -A salon booking platform for KSA (Saudi Arabia) with Django REST API backend and React/Vite frontend. Optimized for phone-first auth (OTP via SMS/WhatsApp), Moyasar payments, Arabic locale (ar-sa), and Riyadh timezone. - -## Commands - -### Backend - -```bash -# Setup (from repo root) -python3 -m venv venv && source venv/bin/activate -pip install -r backend/requirements.txt -r backend/requirements-dev.txt -cp backend/.env.example backend/.env - -# Migrations and dev server (from backend/) -cd backend -python3 manage.py migrate -python3 manage.py runserver - -# Seed demo data -python3 manage.py seed_demo - -# Run all tests (from backend/ with venv active) -cd backend -python3 -m pytest - -# Run a single test file -python3 -m pytest apps/accounts/tests/test_otp_limits.py - -# Run a single test -python3 -m pytest apps/accounts/tests/test_otp_limits.py::TestClassName::test_method_name - -# Run external/integration tests (hits real third-party services) -PYTEST_ADDOPTS='' python3 -m pytest -m external -``` - -### Authentica E2E Testing - -Set these env vars before running external tests: -- `AUTHENTICA_E2E=1` -- `AUTHENTICA_API_KEY=...` -- `AUTHENTICA_E2E_PHONE=...` (phone that will receive OTP) -- `AUTHENTICA_E2E_CODE=...` (OTP code received) - -### Frontend - -```bash -# From frontend/ -cd frontend -npm install -npm run dev # dev server at localhost:5173, proxies /api to localhost:8000 -npm run test # vitest -npm run build -``` - -## Architecture - -### Backend (`backend/`) - -Django project lives in `backend/salon_api/` (settings, root urls, wsgi/asgi). All domain apps are under `backend/apps/`: - -| App | Responsibility | -|-----|----------------| -| `accounts` | Custom User model, phone/OTP auth, JWT tokens, locale preferences | -| `salons` | Salon catalog, services, staff profiles, availability windows, reviews | -| `bookings` | Booking lifecycle, availability/overlap validation, status transitions | -| `payments` | Moyasar integration (create, capture, refund), webhook reconciliation, idempotency | -| `notifications` | Booking lifecycle SMS/WhatsApp messages, stored for auditability | - -**Service layer pattern:** Business logic lives in `apps//services/` (not in views). Views are thin — they validate input, call services, return responses. Keep it this way. - -**OTP providers** (`apps/accounts/services/otp.py`): pluggable via `OTP_PROVIDER` env var. Active providers: `console` (dev), `twilio`, `authentica`. `unifonic` is a scaffold. Authentica is the recommended production provider and uses a server-side OTP flow (`uses_provider_otp = True`) — it generates and verifies the code itself, so the DB stores a placeholder hash. - -**Payment gateway** (`apps/payments/services/gateway.py`): `MoyasarGateway` implements `BasePaymentGateway`. Amounts are always in minor units (halalas). `MOYASAR_SECRET_KEY` and `MOYASAR_PUBLISHABLE_KEY` are required. - -**Sync-only (MVP):** All external calls (OTP sends, notifications, payment gateway) run synchronously in the request path. No task queue. See `docs/adr/0001-synchronous-external-calls-mvp.md`. - -**Database:** SQLite for local dev (default), PostgreSQL via `DATABASE_URL` env var for production. Tests use `TEST_DATABASE_URL` if set. - -**Localization:** Default language `ar-sa`, timezone `Asia/Riyadh`. `UserLocaleMiddleware` applies per-user locale preference. - -### Frontend (`frontend/`) - -React 18 + Vite app. Entry: `src/main.jsx` → `AuthProvider` wraps `App`. - -- **Routing:** `react-router-dom` v7 with pages in `src/pages/` -- **Auth:** JWT tokens managed via `src/contexts/AuthContext.jsx`; `src/components/ProtectedRoute.jsx` guards private pages -- **API:** `src/api/client.js` is the axios/fetch wrapper -- **Hooks:** Domain logic extracted into `src/hooks/` (e.g., `useSalonSearch`, `usePaymentForm`) -- **i18n:** `react-i18next` configured in `src/i18n/index.js`; supports `ar-sa` and `en` - -Tests use Vitest + Testing Library. Setup in `src/test/setupTests.js`. - -## Key Env Vars - -Backend (`backend/.env`): -- `DJANGO_SECRET_KEY`, `DJANGO_DEBUG`, `DATABASE_URL` -- `OTP_PROVIDER` — `console` | `twilio` | `authentica` | `unifonic` -- `AUTHENTICA_API_KEY`, `AUTHENTICA_BASE_URL`, `AUTHENTICA_SENDER_NAME` -- `TWILIO_ACCOUNT_SID`, `TWILIO_AUTH_TOKEN`, `TWILIO_FROM_NUMBER`, `TWILIO_WHATSAPP_FROM` -- `MOYASAR_SECRET_KEY`, `MOYASAR_PUBLISHABLE_KEY` -- `NOTIFICATION_PROVIDER` — defaults to `OTP_PROVIDER` -- `OTP_EXPIRY_MINUTES`, `OTP_MAX_PER_WINDOW`, `OTP_WINDOW_MINUTES`, `OTP_RESEND_COOLDOWN_SECONDS` -- `CORS_ALLOWED_ORIGINS` - -## Testing Conventions - -- Backend tests live beside their apps: `apps//tests/test_*.py` -- `pytest.ini` marks `external` tests as opt-in; default runs skip them -- Frontend tests: Vitest + Testing Library; test files colocated with source (`*.test.jsx`) -- Minimum coverage: auth flows, booking validation, payment state transitions - -## ExecPlans - -For complex features, use an ExecPlan (see `PLANS.md` for the full spec and active plan pointer). ExecPlans are living documents in `docs/execplans/`. The active plan is listed in `PLANS.md`. Update `docs/risks.md` when opening or closing a significant gap. - -## Coding Conventions - -- Business logic in service layers; keep views thin -- Predictable error responses: HTTP status code + `detail` field -- Comment intent, edge cases, and non-obvious business rules; skip obvious comments -- Payment and booking flows must be idempotent and auditable -- Phone auth must be rate-limited (enforced in `otp.py` via `OtpRateLimitError` / `OtpCooldownError`) diff --git a/docs/architecture.md b/docs/architecture.md index af001e2..b0ab8d4 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -1,60 +1,223 @@ -# Architecture +# Salon MVP Roadmap And Architecture Review -## Overview +## Purpose / Big Picture -The Salon platform is a Django REST API backend with a React/Vite frontend, optimized for KSA (phone auth, Riyadh timezone, Arabic locale). +This plan reviews the current Salon codebase (Django backend, React/Vite frontend), highlights architectural and design risks, and lays out a pragmatic roadmap to reach an MVP that aligns with **Phase 1: Core MVP Reliability** in `AGENTS.md`: phone-first auth with OTP, robust booking integrity, Moyasar payments, booking lifecycle notifications, localization foundations, and tests for critical flows. +The roadmap assumes a KSA-focused first launch (phone auth and Riyadh timezone defaults) with a clean path to expand later. -## Backend Apps and Responsibilities +## Current State Summary -| App | Responsibility | -|-----|----------------| -| **accounts** | User model, phone/OTP auth, JWT tokens, locale preferences. OTP providers (console, Twilio, Unifonic) send SMS/WhatsApp. | -| **salons** | Salon catalog, services, staff, availability windows, reviews. Read-only public APIs. | -| **bookings** | Booking model, validation (availability, overlap prevention), status transitions. Triggers notifications on create and status change. | -| **payments** | Payment model, Moyasar integration (create, capture, refund), webhook reconciliation, idempotency. | -| **notifications** | Booking lifecycle notifications (SMS/WhatsApp). Reuses OTP provider classes as an MVP shortcut; sends on booking created/confirmed/cancelled. See note below. | +### Backend (Django, DRF) -## Data Model Overview +- **Project**: `salon_api` in `[backend/salon_api](backend/salon_api)` + - `settings.py` configures `AUTH_USER_MODEL = "accounts.User"`, DRF + SimpleJWT, KSA locale defaults, and custom settings for OTP, notifications, and payments. + - URLs route to app-level APIs at `/api/auth/`, `/api/salons/`, `/api/bookings/`, `/api/payments/`. +- **Auth & Accounts** (`[backend/apps/accounts](backend/apps/accounts)`) + - Custom `User` model with phone-first capabilities (`phone_number`, `is_phone_verified`, `preferred_language`, `role`). + - Phone normalization services tuned for KSA numbers (`[backend/apps/accounts/services/phone.py](backend/apps/accounts/services/phone.py)`). + - OTP domain + service layer with rate limits, cooldowns, expiry, and hashed codes (`[backend/apps/accounts/services/otp.py](backend/apps/accounts/services/otp.py)`). + - Phone-first auth endpoints that issue JWT tokens on successful OTP verification, plus basic email/password registration. + - Social login endpoint is a placeholder that always returns 501. +- **Salons & Catalog** (`[backend/apps/salons](backend/apps/salons)`) + - Models for `Salon`, `Service`, `StaffProfile`, `StaffAvailability`, `Review`, `SalonPhoto`. + - Read-only APIs for salon search, services, staff, and reviews. +- **Bookings** (`[backend/apps/bookings](backend/apps/bookings)`) + - `Booking` model ties together salon, service, staff, customer, time window, price, and status. + - `validate_booking_request` in `[backend/apps/bookings/services.py](backend/apps/bookings/services.py)` enforces staff membership, duration matching, availability windows, and overlap prevention for pending/confirmed bookings. + - `BookingViewSet` in `[backend/apps/bookings/views.py](backend/apps/bookings/views.py)` applies role-based access and hooks into notifications on create and relevant status changes. +- **Payments (Moyasar)** (`[backend/apps/payments](backend/apps/payments)`) + - `Payment` model tracks provider, status (fine-grained transitions), amount, idempotency key, external ID, payload, and timestamps. + - `MoyasarGateway` in `[backend/apps/payments/services/gateway.py](backend/apps/payments/services/gateway.py)` can create payments via HTTP but has `capture_payment`/`refund_payment` as TODOs. + - `create_payment_for_booking` in `[backend/apps/payments/services/payments.py](backend/apps/payments/services/payments.py)` enforces provider choice (Moyasar only), idempotency, and maps webhook events into internal statuses. + - Webhook view in `[backend/apps/payments/views.py](backend/apps/payments/views.py)` authenticates via shared secret and applies provider events idempotently. +- **Notifications** (`[backend/apps/notifications](backend/apps/notifications)`) + - `Notification` model records booking-related notifications and enforces uniqueness per booking/recipient/event/channel. + - Services in `[backend/apps/notifications/services.py](backend/apps/notifications/services.py)` reuse OTP providers to send SMS/WhatsApp messages for booking created/confirmed/cancelled events, with localization via `preferred_language`. + - Booking views call `notify_booking_lifecycle` / `notify_on_status_change` to trigger notifications for customers and staff. +- **Testing** + - Solid tests around: + - Phone normalization, OTP rate limiting, and phone auth flows (`[backend/apps/accounts/tests](backend/apps/accounts/tests)`). + - Booking integrity and overlap rules (`[backend/apps/bookings/tests](backend/apps/bookings/tests)`). + - Payment idempotency and Moyasar webhook handling (`[backend/apps/payments/tests](backend/apps/payments/tests)`). + - Booking notifications on create/status change (`[backend/apps/notifications/tests](backend/apps/notifications/tests)`). + - `docs/risks.md` explicitly tracks several known gaps around auth, booking, payments, data/UX, and ops. -The core data model centers on users, salons, and time-bound bookings. A booking ties a customer to a service, a staff member, and a scheduled time. Payments are recorded per booking and reconcile to the external gateway. Notifications are stored for every booking lifecycle message for auditability. +### Frontend (React, Vite) -- `accounts.User` owns phone, locale, and auth preferences. -- `salons.Salon`, `salons.Service`, and `salons.Staff` define the catalog and scheduling surface. -- `bookings.Booking` links customer, staff, service, and scheduled time, with status transitions. -- `payments.Payment` tracks gateway state and idempotency per booking. -- `notifications.Notification` records each SMS/WhatsApp send attempt tied to a booking event. +- **Structure** + - Vite React app at `[frontend](frontend)` with entry in `[frontend/src/main.jsx](frontend/src/main.jsx)` and single top-level component in `[frontend/src/App.jsx](frontend/src/App.jsx)`. + - No `react-router` or multi-page routing; the entire experience is one composed screen. +- **Current Features** + - **Salon search** + - Text search field calling `/salons/?q=` via a small API client in `[frontend/src/api/client.js](frontend/src/api/client.js)`. + - Renders responsive list of salons with rating, city, and phone. + - **Localization/i18n** + - `react-i18next` setup in `[frontend/src/i18n/index.js](frontend/src/i18n/index.js)` with `en` and `ar-sa` translations. + - Locale preference stored in `localStorage`; applies `lang` and `dir` on the document. + - **Payments beta** + - A form in `App.jsx` that sends payment creation requests to `/api/payments/` using the Moyasar-style payload, with configurable `booking_id`, source type, token, and callback URL. + - Optionally includes a Bearer token from a manually-entered access token field. + - On success, can redirect to `redirect_url` and shows the raw JSON response. +- **State & Tests** + - All state is local to `App.jsx` via `useState`/`useEffect`; there is no centralized state management or domain hooks yet. + - A single test file `[frontend/src/App.test.jsx](frontend/src/App.test.jsx)` covers hero copy and locale/RTL behavior, but not search or payments. -## Data Flow +## Glaring Design And Architectural Issues -``` -User → React Frontend → Django API - ↓ - accounts (auth) ──→ OTP providers (Twilio/Unifonic/console) - salons (catalog) - bookings ──→ notifications ──→ OTP providers - payments ──→ Moyasar gateway +### Backend Risks + +- **Incomplete provider implementations for production-critical flows** + - Twilio/Unifonic providers in `[backend/apps/accounts/services/otp.py](backend/apps/accounts/services/otp.py)` are stubs with `NotImplementedError` for send methods, yet they are the backbone for both OTP and booking notifications. + - `MoyasarGateway` lacks `capture_payment` and `refund_payment` implementations, limiting payment lifecycle coverage. + - **Risk**: Code reads “production ready” at the API level, but the underlying integrations are not, which could cause outages if deployed naively. +- **Tight coupling between OTP and notifications** + - Notification services import the OTP provider mapping and default `NOTIFICATION_PROVIDER` to `OTP_PROVIDER`, binding booking notifications to auth configuration. + - **Risk**: Changing OTP providers or adding a second channel for marketing/ops notifications will be harder and could have unintended side effects. +- **Synchronous IO-heavy work in request/response path** + - OTP sends, booking notifications, and payment gateway calls all occur synchronously inside view methods (`perform_create`, `create`, etc.). + - **Risk**: Slow or flaky providers will degrade API latency and user experience; retries and backoff are hard to implement without background jobs. +- **Cross-app domain coupling without a clear orchestration layer** + - `apps.bookings` depends on salons and notifications; notifications depend on accounts (OTP providers) and bookings; payments depend on bookings. + - **Risk**: As you add more lifecycle rules (e.g., auto-confirm booking on payment, send reminders, handle refunds), the spaghetti of cross-imports will grow unless you introduce clearer service boundaries. +- **Auth model vs login patterns** + - `User.USERNAME_FIELD` is email, while phone-based JWT issuance happens via custom endpoints. + - **Risk**: This split can confuse clients and admin tooling and may complicate future flows like social login or SSO unless you standardize on an identifier strategy. +- **Docs drift around ExecPlans** + - `AGENTS.md` references `docs/execplans/payments-moyasar.md` as the active plan, while `PLANS.md` names `docs/execplans/booking-notifications.md`. + - **Risk**: Contributors may follow different “active” plans, causing architectural inconsistency. + +### Frontend Risks + +- **Monolithic `App` component with no routing** + - `App.jsx` mixes hero/search, salon listing, payments, and locale controls. + - There is no `react-router` or notion of separate flows (auth, booking, profile, payments). + - **Risk**: Extending to full MVP flows (auth, booking, history, management) will quickly become unmanageable without a routing/page system and domain separation. +- **Domain logic embedded in UI components** + - API payload construction, validation rules (e.g. for source types), and error handling are implemented directly in `App.jsx` rather than reusable hooks or service modules. + - **Risk**: Code reuse, testing, and evolution (e.g., adding booking pages or admin consoles) will be painful. +- **Minimal test coverage for critical flows** + - Only i18n and hero copy are tested; search behavior, API integration, and payments are untested. + - **Risk**: Regressions in search, booking, and payments UX will slip through as MVP grows. +- **Styling & layout fragility** + - `frontend/src/styles.css` uses `::root` instead of `:root`, which likely breaks intended global CSS variables or base styles. + - Global CSS is tightly bound to the monolithic `App` layout. + - **Risk**: Visual regressions and layout churn when introducing additional pages or components. +- **Ad hoc auth token handling** + - The “access token” is a free-form text field that gets persisted as `auth_token` in `localStorage` and injected into payment requests. + - **Risk**: This is a placeholder pattern that does not scale to full auth (refresh tokens, logout, token rotation) and will need to be replaced. + +### Cross-Cutting Risks + +- **Lack of async/background processing** + - No Celery/RQ or similar job queue; all side effects are synchronous. + - **Risk**: Scaling SMS/WhatsApp notifications, email, and payment webhook fan-out will be difficult. +- **Observability and admin tooling gaps** + - Errors for payments and notifications are recorded in model metadata but not clearly surfaced in logs, dashboards, or admin views. + - **Risk**: Operational debugging during MVP rollout will be slower and more error-prone. +- **Internationalization strategy vs future markets** + - Phone normalization and defaults are tailored to KSA, which is correct for MVP, but `docs/risks.md` already notes the need to broaden later. + - **Risk**: Without clear boundaries between KSA-specific logic and generic logic, future expansion may require invasive changes. + +## MVP Roadmap (Aligned To Phase 1) + +This roadmap assumes “MVP” is equivalent to **Phase 1: Core MVP Reliability** in `AGENTS.md`, with a thin but robust frontend on top. + +### Phase 0 – Architecture & Production Readiness Hardening + +- **Finalize critical provider implementations** + - Implement at least one real SMS/WhatsApp provider (Twilio or Unifonic) end-to-end, behind the existing provider abstraction in `[backend/apps/accounts/services/otp.py](backend/apps/accounts/services/otp.py)`, and wire it into `[backend/apps/notifications/services.py](backend/apps/notifications/services.py)`. + - Implement or deliberately fence off `capture_payment` and `refund_payment` in `[backend/apps/payments/services/gateway.py](backend/apps/payments/services/gateway.py)` so that the MVP either fully supports or explicitly does not support partial captures/refunds. +- **Clarify and document boundaries** + - Add a short architecture section in `README`/docs describing how `accounts`, `salons`, `bookings`, `payments`, and `notifications` interact, and what each service is responsible for. + - Resolve the ExecPlan drift by making `AGENTS.md` and `PLANS.md` agree on the current active plan. +- **Introduce minimal async infrastructure (optional but recommended)** + - Decide whether MVP will ship with a task queue (e.g., Celery with Redis) or keep everything synchronous for the initial launch. + - If yes, introduce a thin task layer for OTP sends and booking notifications while preserving current APIs; if not, at least add clear timeouts/logging to external calls. +- **Frontend scaffolding for growth** + - Introduce `react-router` and refactor `App.jsx` into route-based pages (e.g., `HomePage`, `BookPage`, `PaymentPage`, `ProfilePage`), with shared layout and navigation. + - Extract salon search, payment form, and locale controls into dedicated components and hooks. + +### Phase 1 – Core MVP Features (Backend + Frontend) + +- **Phone-first auth UX** + - Backend: reuse existing phone auth endpoints; ensure error messages and rate-limit responses are predictable and localized. + - Frontend: + - Build OTP-based login/registration screens that drive `/api/auth/phone/request/` and `/api/auth/phone/verify/`. + - Introduce an auth context (or similar) to store access/refresh tokens, current user profile, and handle logout. + - Defer social login beyond MVP, but keep API surface ready for it. +- **Booking search and creation** + - Backend is largely ready (booking validation and role-based access); review booking serializers in `[backend/apps/bookings/serializers.py](backend/apps/bookings/serializers.py)` to ensure they expose all fields needed for frontend booking forms. + - Frontend: + - Build a **booking flow**: pick a salon → choose service → select staff (optional) → select date/time slot (based on availability endpoints) → confirm booking. + - Add a “My bookings” page showing upcoming and past bookings, tied into the existing `/api/bookings/` endpoints. +- **Payments via Moyasar** + - Backend: confirm `create_payment_for_booking` contracts (inputs/outputs) are stable and documented. + - Frontend: + - Evolve the payments beta UI into a **post-booking payment step** that starts from a selected booking and guides the user into Moyasar’s hosted flow, then shows a status page. + - Handle callback/return from Moyasar (even if via manual redirect URL in MVP) and surface payment success/failure to the user. +- **Booking lifecycle notifications** + - Backend already sends notifications on booking create and status changes; align messaging templates with product UX and ensure localization strings exist. + - Frontend: surface notification results implicitly via booking status changes and explicit messages on the booking details page. +- **Localization foundations** + - Backend: ensure `UserLocaleMiddleware` and translation strings cover all user-visible errors in auth, bookings, payments, and notifications. + - Frontend: expand `en/ar-sa` translations to cover auth, booking, and payment flows; verify RTL layouts on the new screens. +- **Tests for critical flows** + - Backend: extend tests where needed to cover new booking/payment edge cases (e.g., tying booking status to payment status if/when introduced). + - Frontend: add Vitest tests for: + - Phone auth screen flows (request/verify success + errors). + - Booking flow (form validation, happy path, displaying server-side errors). + - Payment initiation from an existing booking. + +### Phase 2 – Manager Ops Lite (Post-MVP, partially covered now) + +- **Salon and staff management UI** + - Use existing salon and staff models to build basic management pages for salon owners/managers (create/update services, staff, availability). +- **Calendar views and rescheduling** + - Provide calendar views for staff/managers to view daily/weekly bookings and reschedule or cancel within defined rules. +- **Reviews and ratings** + - Implement review submission and rating recalculation on the backend, with corresponding frontend components. +- **Reporting basics** + - Lightweight reports for managers (upcoming bookings, simple revenue summaries based on payment status) using existing payments data. + +### Phase 3 – Scale & Compliance (Later) + +- **Audit logging** for admin actions and booking/payment state changes. +- **PDPL/GDPR retention policies** and data export tooling. +- **Observability**: structured logging, metrics, and basic dashboards for auth failures, OTP send failures, payment errors, and notification outcomes. + +## Architecture Overview Diagram + +A simplified view of the target MVP data flow: + +```mermaid +flowchart LR + user["User (web/mobile)"] --> frontend["ReactFrontend"] + frontend --> api["DjangoAPI"] + + api --> accounts["AccountsApp"] + api --> salons["SalonsApp"] + api --> bookings["BookingsApp"] + api --> payments["PaymentsApp"] + api --> notifications["NotificationsApp"] + + accounts --> otpProviders["OtpProviders"] + notifications --> otpProviders + payments --> moyasar["MoyasarGateway"] + + bookings --> notifications + bookings --> payments + salons --> bookings ``` -## Notification / OTP Provider Coupling (MVP Shortcut) -`notifications/services.py` imports `PROVIDERS` from `apps.accounts.services.otp` and uses OTP provider instances (e.g. `AuthenticaOtpProvider`) to send booking SMSes. This works today because Authentica handles both authentication OTPs and general SMS delivery. -Consequences of this coupling: -- Notifications and OTP delivery cannot independently use different providers (e.g. Twilio for OTP, Unifonic for notifications). -- The `notifications` app is conceptually coupled to the `accounts` app's auth infrastructure. +This diagram clarifies current coupling and highlights where future refactors (e.g., a dedicated messaging service or payment orchestrator) could sit. -This is an acceptable MVP shortcut. Before Phase 2, introduce a dedicated `NotificationProvider` abstraction in `notifications/` (mirroring `OtpProvider`) so the two systems can be configured and tested independently. +## Validation And Acceptance For This Plan -## Async and Observability (MVP Decision) +- The roadmap is accepted when: + - It clearly maps current backend and frontend capabilities to the Phase 1 MVP goals in `AGENTS.md`. + - It identifies the highest-risk design/architecture issues that could impede MVP reliability or future evolution. + - It provides a phased, concrete sequence of work packages that can be implemented via ExecPlans (e.g., booking notifications, Moyasar payments, phone auth UX). +- Each major feature area (auth, bookings, payments, notifications, localization, tests) should have or adopt an ExecPlan under `docs/execplans/` in line with `PLANS.md` before implementation begins. -**Decision (MVP):** All OTP sends, booking notifications, and payment gateway calls run **synchronously** in the request/response path. No Celery, RQ, or other task queue for the initial launch. -This is captured in ADR 0001 (`docs/adr/0001-synchronous-external-calls-mvp.md`). - -**Rationale:** -- Reduces deployment complexity (no Redis, no worker processes). -- MVP traffic is expected to be low; synchronous latency is acceptable. -- External calls already use timeouts (e.g. Moyasar: 10s, Twilio: SDK default). - -**Future:** When scaling, introduce a task queue (e.g. Celery + Redis) for OTP and notification sends. Payment creation and webhooks should remain synchronous for immediate feedback and idempotency. - -**Observability:** Errors are logged via Python `logging` and stored in model metadata (e.g. `Payment.metadata["gateway_error"]`, `Notification.error_message`). Structured logging and metrics are Phase 3 work. diff --git a/external-review.md b/external-review.md deleted file mode 100644 index 0840e2c..0000000 --- a/external-review.md +++ /dev/null @@ -1,248 +0,0 @@ ---- -name: salon-mvp-roadmap -overview: High-level roadmap to bring the existing Salon Django/React codebase to a reliable MVP aligned with Phase 1 goals in AGENTS.md, plus a review of current architecture and major risks. -todos: - - id: backend-providers-readiness - content: "Harden backend providers: implement at least one real SMS/WhatsApp provider and clarify Moyasar capture/refund behavior for MVP." - status: pending - - id: async-and-observability - content: Decide on async task infrastructure and observability basics for OTP, notifications, and payments, and document the choice. - status: pending - - id: frontend-structure-and-routing - content: Refactor frontend into routed pages with separated components/hooks for search, auth, booking, and payments. - status: pending - - id: auth-and-booking-flows - content: Implement phone-first auth and end-to-end booking flows on the frontend using existing backend APIs. - status: pending - - id: payments-and-notifications-ux - content: Integrate payment initiation and booking lifecycle notifications into user-facing flows, including success/error handling. - status: pending - - id: tests-for-critical-flows - content: Expand backend and frontend tests to cover auth, booking, payment, and notification critical paths for MVP reliability. - status: pending -isProject: false ---- - -# Salon MVP Roadmap And Architecture Review - -## Purpose / Big Picture - -This plan reviews the current Salon codebase (Django backend, React/Vite frontend), highlights architectural and design risks, and lays out a pragmatic roadmap to reach an MVP that aligns with **Phase 1: Core MVP Reliability** in `AGENTS.md`: phone-first auth with OTP, robust booking integrity, Moyasar payments, booking lifecycle notifications, localization foundations, and tests for critical flows. -The roadmap assumes a KSA-focused first launch (phone auth and Riyadh timezone defaults) with a clean path to expand later. - -## Current State Summary - -### Backend (Django, DRF) - -- **Project**: `salon_api` in `[backend/salon_api](backend/salon_api)` - - `settings.py` configures `AUTH_USER_MODEL = "accounts.User"`, DRF + SimpleJWT, KSA locale defaults, and custom settings for OTP, notifications, and payments. - - URLs route to app-level APIs at `/api/auth/`, `/api/salons/`, `/api/bookings/`, `/api/payments/`. -- **Auth & Accounts** (`[backend/apps/accounts](backend/apps/accounts)`) - - Custom `User` model with phone-first capabilities (`phone_number`, `is_phone_verified`, `preferred_language`, `role`). - - Phone normalization services tuned for KSA numbers (`[backend/apps/accounts/services/phone.py](backend/apps/accounts/services/phone.py)`). - - OTP domain + service layer with rate limits, cooldowns, expiry, and hashed codes (`[backend/apps/accounts/services/otp.py](backend/apps/accounts/services/otp.py)`). - - Phone-first auth endpoints that issue JWT tokens on successful OTP verification, plus basic email/password registration. - - Social login endpoint is a placeholder that always returns 501. -- **Salons & Catalog** (`[backend/apps/salons](backend/apps/salons)`) - - Models for `Salon`, `Service`, `StaffProfile`, `StaffAvailability`, `Review`, `SalonPhoto`. - - Read-only APIs for salon search, services, staff, and reviews. -- **Bookings** (`[backend/apps/bookings](backend/apps/bookings)`) - - `Booking` model ties together salon, service, staff, customer, time window, price, and status. - - `validate_booking_request` in `[backend/apps/bookings/services.py](backend/apps/bookings/services.py)` enforces staff membership, duration matching, availability windows, and overlap prevention for pending/confirmed bookings. - - `BookingViewSet` in `[backend/apps/bookings/views.py](backend/apps/bookings/views.py)` applies role-based access and hooks into notifications on create and relevant status changes. -- **Payments (Moyasar)** (`[backend/apps/payments](backend/apps/payments)`) - - `Payment` model tracks provider, status (fine-grained transitions), amount, idempotency key, external ID, payload, and timestamps. - - `MoyasarGateway` in `[backend/apps/payments/services/gateway.py](backend/apps/payments/services/gateway.py)` can create payments via HTTP but has `capture_payment`/`refund_payment` as TODOs. - - `create_payment_for_booking` in `[backend/apps/payments/services/payments.py](backend/apps/payments/services/payments.py)` enforces provider choice (Moyasar only), idempotency, and maps webhook events into internal statuses. - - Webhook view in `[backend/apps/payments/views.py](backend/apps/payments/views.py)` authenticates via shared secret and applies provider events idempotently. -- **Notifications** (`[backend/apps/notifications](backend/apps/notifications)`) - - `Notification` model records booking-related notifications and enforces uniqueness per booking/recipient/event/channel. - - Services in `[backend/apps/notifications/services.py](backend/apps/notifications/services.py)` reuse OTP providers to send SMS/WhatsApp messages for booking created/confirmed/cancelled events, with localization via `preferred_language`. - - Booking views call `notify_booking_lifecycle` / `notify_on_status_change` to trigger notifications for customers and staff. -- **Testing** - - Solid tests around: - - Phone normalization, OTP rate limiting, and phone auth flows (`[backend/apps/accounts/tests](backend/apps/accounts/tests)`). - - Booking integrity and overlap rules (`[backend/apps/bookings/tests](backend/apps/bookings/tests)`). - - Payment idempotency and Moyasar webhook handling (`[backend/apps/payments/tests](backend/apps/payments/tests)`). - - Booking notifications on create/status change (`[backend/apps/notifications/tests](backend/apps/notifications/tests)`). - - `docs/risks.md` explicitly tracks several known gaps around auth, booking, payments, data/UX, and ops. - -### Frontend (React, Vite) - -- **Structure** - - Vite React app at `[frontend](frontend)` with entry in `[frontend/src/main.jsx](frontend/src/main.jsx)` and single top-level component in `[frontend/src/App.jsx](frontend/src/App.jsx)`. - - No `react-router` or multi-page routing; the entire experience is one composed screen. -- **Current Features** - - **Salon search** - - Text search field calling `/salons/?q=` via a small API client in `[frontend/src/api/client.js](frontend/src/api/client.js)`. - - Renders responsive list of salons with rating, city, and phone. - - **Localization/i18n** - - `react-i18next` setup in `[frontend/src/i18n/index.js](frontend/src/i18n/index.js)` with `en` and `ar-sa` translations. - - Locale preference stored in `localStorage`; applies `lang` and `dir` on the document. - - **Payments beta** - - A form in `App.jsx` that sends payment creation requests to `/api/payments/` using the Moyasar-style payload, with configurable `booking_id`, source type, token, and callback URL. - - Optionally includes a Bearer token from a manually-entered access token field. - - On success, can redirect to `redirect_url` and shows the raw JSON response. -- **State & Tests** - - All state is local to `App.jsx` via `useState`/`useEffect`; there is no centralized state management or domain hooks yet. - - A single test file `[frontend/src/App.test.jsx](frontend/src/App.test.jsx)` covers hero copy and locale/RTL behavior, but not search or payments. - -## Glaring Design And Architectural Issues - -### Backend Risks - -- **Incomplete provider implementations for production-critical flows** - - Twilio/Unifonic providers in `[backend/apps/accounts/services/otp.py](backend/apps/accounts/services/otp.py)` are stubs with `NotImplementedError` for send methods, yet they are the backbone for both OTP and booking notifications. - - `MoyasarGateway` lacks `capture_payment` and `refund_payment` implementations, limiting payment lifecycle coverage. - - **Risk**: Code reads “production ready” at the API level, but the underlying integrations are not, which could cause outages if deployed naively. -- **Tight coupling between OTP and notifications** - - Notification services import the OTP provider mapping and default `NOTIFICATION_PROVIDER` to `OTP_PROVIDER`, binding booking notifications to auth configuration. - - **Risk**: Changing OTP providers or adding a second channel for marketing/ops notifications will be harder and could have unintended side effects. -- **Synchronous IO-heavy work in request/response path** - - OTP sends, booking notifications, and payment gateway calls all occur synchronously inside view methods (`perform_create`, `create`, etc.). - - **Risk**: Slow or flaky providers will degrade API latency and user experience; retries and backoff are hard to implement without background jobs. -- **Cross-app domain coupling without a clear orchestration layer** - - `apps.bookings` depends on salons and notifications; notifications depend on accounts (OTP providers) and bookings; payments depend on bookings. - - **Risk**: As you add more lifecycle rules (e.g., auto-confirm booking on payment, send reminders, handle refunds), the spaghetti of cross-imports will grow unless you introduce clearer service boundaries. -- **Auth model vs login patterns** - - `User.USERNAME_FIELD` is email, while phone-based JWT issuance happens via custom endpoints. - - **Risk**: This split can confuse clients and admin tooling and may complicate future flows like social login or SSO unless you standardize on an identifier strategy. -- **Docs drift around ExecPlans** - - `AGENTS.md` references `docs/execplans/payments-moyasar.md` as the active plan, while `PLANS.md` names `docs/execplans/booking-notifications.md`. - - **Risk**: Contributors may follow different “active” plans, causing architectural inconsistency. - -### Frontend Risks - -- **Monolithic `App` component with no routing** - - `App.jsx` mixes hero/search, salon listing, payments, and locale controls. - - There is no `react-router` or notion of separate flows (auth, booking, profile, payments). - - **Risk**: Extending to full MVP flows (auth, booking, history, management) will quickly become unmanageable without a routing/page system and domain separation. -- **Domain logic embedded in UI components** - - API payload construction, validation rules (e.g. for source types), and error handling are implemented directly in `App.jsx` rather than reusable hooks or service modules. - - **Risk**: Code reuse, testing, and evolution (e.g., adding booking pages or admin consoles) will be painful. -- **Minimal test coverage for critical flows** - - Only i18n and hero copy are tested; search behavior, API integration, and payments are untested. - - **Risk**: Regressions in search, booking, and payments UX will slip through as MVP grows. -- **Styling & layout fragility** - - `frontend/src/styles.css` uses `::root` instead of `:root`, which likely breaks intended global CSS variables or base styles. - - Global CSS is tightly bound to the monolithic `App` layout. - - **Risk**: Visual regressions and layout churn when introducing additional pages or components. -- **Ad hoc auth token handling** - - The “access token” is a free-form text field that gets persisted as `auth_token` in `localStorage` and injected into payment requests. - - **Risk**: This is a placeholder pattern that does not scale to full auth (refresh tokens, logout, token rotation) and will need to be replaced. - -### Cross-Cutting Risks - -- **Lack of async/background processing** - - No Celery/RQ or similar job queue; all side effects are synchronous. - - **Risk**: Scaling SMS/WhatsApp notifications, email, and payment webhook fan-out will be difficult. -- **Observability and admin tooling gaps** - - Errors for payments and notifications are recorded in model metadata but not clearly surfaced in logs, dashboards, or admin views. - - **Risk**: Operational debugging during MVP rollout will be slower and more error-prone. -- **Internationalization strategy vs future markets** - - Phone normalization and defaults are tailored to KSA, which is correct for MVP, but `docs/risks.md` already notes the need to broaden later. - - **Risk**: Without clear boundaries between KSA-specific logic and generic logic, future expansion may require invasive changes. - -## MVP Roadmap (Aligned To Phase 1) - -This roadmap assumes “MVP” is equivalent to **Phase 1: Core MVP Reliability** in `AGENTS.md`, with a thin but robust frontend on top. - -### Phase 0 – Architecture & Production Readiness Hardening - -- **Finalize critical provider implementations** - - Implement at least one real SMS/WhatsApp provider (Twilio or Unifonic) end-to-end, behind the existing provider abstraction in `[backend/apps/accounts/services/otp.py](backend/apps/accounts/services/otp.py)`, and wire it into `[backend/apps/notifications/services.py](backend/apps/notifications/services.py)`. - - Implement or deliberately fence off `capture_payment` and `refund_payment` in `[backend/apps/payments/services/gateway.py](backend/apps/payments/services/gateway.py)` so that the MVP either fully supports or explicitly does not support partial captures/refunds. -- **Clarify and document boundaries** - - Add a short architecture section in `README`/docs describing how `accounts`, `salons`, `bookings`, `payments`, and `notifications` interact, and what each service is responsible for. - - Resolve the ExecPlan drift by making `AGENTS.md` and `PLANS.md` agree on the current active plan. -- **Introduce minimal async infrastructure (optional but recommended)** - - Decide whether MVP will ship with a task queue (e.g., Celery with Redis) or keep everything synchronous for the initial launch. - - If yes, introduce a thin task layer for OTP sends and booking notifications while preserving current APIs; if not, at least add clear timeouts/logging to external calls. -- **Frontend scaffolding for growth** - - Introduce `react-router` and refactor `App.jsx` into route-based pages (e.g., `HomePage`, `BookPage`, `PaymentPage`, `ProfilePage`), with shared layout and navigation. - - Extract salon search, payment form, and locale controls into dedicated components and hooks. - -### Phase 1 – Core MVP Features (Backend + Frontend) - -- **Phone-first auth UX** - - Backend: reuse existing phone auth endpoints; ensure error messages and rate-limit responses are predictable and localized. - - Frontend: - - Build OTP-based login/registration screens that drive `/api/auth/phone/request/` and `/api/auth/phone/verify/`. - - Introduce an auth context (or similar) to store access/refresh tokens, current user profile, and handle logout. - - Defer social login beyond MVP, but keep API surface ready for it. -- **Booking search and creation** - - Backend is largely ready (booking validation and role-based access); review booking serializers in `[backend/apps/bookings/serializers.py](backend/apps/bookings/serializers.py)` to ensure they expose all fields needed for frontend booking forms. - - Frontend: - - Build a **booking flow**: pick a salon → choose service → select staff (optional) → select date/time slot (based on availability endpoints) → confirm booking. - - Add a “My bookings” page showing upcoming and past bookings, tied into the existing `/api/bookings/` endpoints. -- **Payments via Moyasar** - - Backend: confirm `create_payment_for_booking` contracts (inputs/outputs) are stable and documented. - - Frontend: - - Evolve the payments beta UI into a **post-booking payment step** that starts from a selected booking and guides the user into Moyasar’s hosted flow, then shows a status page. - - Handle callback/return from Moyasar (even if via manual redirect URL in MVP) and surface payment success/failure to the user. -- **Booking lifecycle notifications** - - Backend already sends notifications on booking create and status changes; align messaging templates with product UX and ensure localization strings exist. - - Frontend: surface notification results implicitly via booking status changes and explicit messages on the booking details page. -- **Localization foundations** - - Backend: ensure `UserLocaleMiddleware` and translation strings cover all user-visible errors in auth, bookings, payments, and notifications. - - Frontend: expand `en/ar-sa` translations to cover auth, booking, and payment flows; verify RTL layouts on the new screens. -- **Tests for critical flows** - - Backend: extend tests where needed to cover new booking/payment edge cases (e.g., tying booking status to payment status if/when introduced). - - Frontend: add Vitest tests for: - - Phone auth screen flows (request/verify success + errors). - - Booking flow (form validation, happy path, displaying server-side errors). - - Payment initiation from an existing booking. - -### Phase 2 – Manager Ops Lite (Post-MVP, partially covered now) - -- **Salon and staff management UI** - - Use existing salon and staff models to build basic management pages for salon owners/managers (create/update services, staff, availability). -- **Calendar views and rescheduling** - - Provide calendar views for staff/managers to view daily/weekly bookings and reschedule or cancel within defined rules. -- **Reviews and ratings** - - Implement review submission and rating recalculation on the backend, with corresponding frontend components. -- **Reporting basics** - - Lightweight reports for managers (upcoming bookings, simple revenue summaries based on payment status) using existing payments data. - -### Phase 3 – Scale & Compliance (Later) - -- **Audit logging** for admin actions and booking/payment state changes. -- **PDPL/GDPR retention policies** and data export tooling. -- **Observability**: structured logging, metrics, and basic dashboards for auth failures, OTP send failures, payment errors, and notification outcomes. - -## Architecture Overview Diagram - -A simplified view of the target MVP data flow: - -```mermaid -flowchart LR - user["User (web/mobile)"] --> frontend["ReactFrontend"] - frontend --> api["DjangoAPI"] - - api --> accounts["AccountsApp"] - api --> salons["SalonsApp"] - api --> bookings["BookingsApp"] - api --> payments["PaymentsApp"] - api --> notifications["NotificationsApp"] - - accounts --> otpProviders["OtpProviders"] - notifications --> otpProviders - payments --> moyasar["MoyasarGateway"] - - bookings --> notifications - bookings --> payments - salons --> bookings -``` - - - -This diagram clarifies current coupling and highlights where future refactors (e.g., a dedicated messaging service or payment orchestrator) could sit. - -## Validation And Acceptance For This Plan - -- The roadmap is accepted when: - - It clearly maps current backend and frontend capabilities to the Phase 1 MVP goals in `AGENTS.md`. - - It identifies the highest-risk design/architecture issues that could impede MVP reliability or future evolution. - - It provides a phased, concrete sequence of work packages that can be implemented via ExecPlans (e.g., booking notifications, Moyasar payments, phone auth UX). -- Each major feature area (auth, bookings, payments, notifications, localization, tests) should have or adopt an ExecPlan under `docs/execplans/` in line with `PLANS.md` before implementation begins. -