cleanup phase 1
This commit is contained in:
hamid
@@ -0,0 +1,159 @@
|
||||
# Code quality
|
||||
|
||||
The four rules that apply identically to both projects: no dead code, comment the *why*, no starter
|
||||
scaffolding, and a mock is only a mock behind a seam.
|
||||
|
||||
> Last verified: 2026-07-30 against commit `d3ec723`.
|
||||
|
||||
---
|
||||
|
||||
## 1. No dead code
|
||||
|
||||
Unused variables, imports/usings, parameters, and private members are removed — not left behind, not
|
||||
commented out, and **not suppressed**.
|
||||
|
||||
| Project | How it surfaces | Gate |
|
||||
| --- | --- | --- |
|
||||
| Client | `@typescript-eslint/no-unused-vars`, raised from eslint-config-next's default `warn` to **`error`** in `eslint.config.mjs` | Dead code **fails `npm run check`** |
|
||||
| Server | `CS0168` (declared, never used), `CS0219` (assigned, never read), `CS0169` (private field never used), `IDE0005` (unnecessary `using`) | The gate is **zero new warnings**, so dead code is a gate failure |
|
||||
|
||||
**Delete it — don't silence it.** No `#pragma warning disable`, no throwaway discards, no `_ =`
|
||||
assignments to quiet an analyzer, no file-wide ESLint disable.
|
||||
|
||||
Two sanctioned opt-outs, both narrow:
|
||||
|
||||
- **Client:** a deliberately-unused binding is prefixed with `_` — `_event`, `catch (_err)`.
|
||||
- **Server:** a parameter that must exist to satisfy an interface or delegate signature but is genuinely
|
||||
unused stays, named conventionally, with a one-line `// why` only if the reason isn't obvious.
|
||||
|
||||
When a lint disable is genuinely correct — a deliberate browser-only read after mount that trips
|
||||
`react-hooks/set-state-in-effect` is the real example in this codebase — use a scoped
|
||||
`// eslint-disable-next-line <rule>` with a one-line reason on the line above. Never a file-wide disable,
|
||||
and never in preference to fixing the code.
|
||||
|
||||
---
|
||||
|
||||
## 2. Comment the *why*, never the *what*
|
||||
|
||||
Code that needs a comment to be understood usually needs a **better name** instead. Reach for the name
|
||||
first, then a small helper, then a comment.
|
||||
|
||||
**Don't** write a comment that restates what the code already says:
|
||||
|
||||
```csharp
|
||||
// ❌ restates the obvious
|
||||
// increment the retry counter
|
||||
retryCount++;
|
||||
```
|
||||
|
||||
```tsx
|
||||
// ❌ restates the obvious
|
||||
// set the access token
|
||||
setClientCookie(COOKIE_NAMES.ACCESS_TOKEN, token);
|
||||
```
|
||||
|
||||
No XML-doc or JSDoc that merely echoes a function's name either.
|
||||
|
||||
**Do** add a tight comment where a non-obvious decision, constraint, business rule, workaround, ordering
|
||||
or security requirement, or deliberate deviation is *not* evident from the code. Explain the reasoning,
|
||||
not the mechanics:
|
||||
|
||||
```csharp
|
||||
// ✅ captures a constraint the code can't express on its own
|
||||
// Payment gateway rejects amounts above 50M IRR per call; split larger settlements upstream.
|
||||
if (amount > MaxPerCallRial) …
|
||||
```
|
||||
|
||||
The models to follow in this codebase:
|
||||
|
||||
| File | What its comment earns |
|
||||
| --- | --- |
|
||||
| `client/src/app/[locale]/layout.tsx` | Why `<html>` lives in the `[locale]` layout and not above it |
|
||||
| `client/src/lib/auth/token.ts` | Why the JWT `exp` check is UX-only and never a security boundary |
|
||||
| `client/src/layout/config.ts` | Why the two chrome-bar heights are measured rather than guessed, and must stay in sync with the bars |
|
||||
| `client/middleware.ts` | Why the matcher lists bare `'/'` explicitly alongside the catch-all regex |
|
||||
|
||||
Delete comments that no longer match the code. A wrong comment costs more than no comment.
|
||||
|
||||
---
|
||||
|
||||
## 3. Don't reintroduce starter scaffolding
|
||||
|
||||
Both projects were derived from open-source starters. Their branding, demo/showcase pages, and
|
||||
`_TITLE_`/`_DESCRIPTION_` placeholders were **intentionally removed**. Don't add them back — not as a
|
||||
convenience, not while editing docs, not as an example.
|
||||
|
||||
Specifically:
|
||||
|
||||
- No placeholder page, showcase route, or "example component" gallery.
|
||||
- No `_TITLE_` / `_DESCRIPTION_` / lorem-ipsum copy anywhere, including in message files.
|
||||
- No starter README boilerplate reinstated into a project README.
|
||||
- `PlaceholderScreen` exists for a genuinely not-yet-built screen and must not be reachable from a shell's
|
||||
navigation. `/admin/notifications` is the current example: it is a placeholder, and it is deliberately
|
||||
absent from `AdminLayout`'s nav for that reason.
|
||||
|
||||
---
|
||||
|
||||
## 4. A mock is only a mock behind a seam
|
||||
|
||||
Some integrations are intentionally out of scope and must be **mocked, not invented**: real PSP and BNPL
|
||||
connections, the Shahkar / MoH / INO / criminal-record vendors, MinIO/S3 credentials, the سامانه مودیان
|
||||
enrollment. Reaching one is not a blocker.
|
||||
|
||||
The only sanctioned form of "not real yet" is:
|
||||
|
||||
1. **An interface.** Server: an interface in `Application/Contracts/`, implemented twice, selected by
|
||||
configuration in `AddCrossCuttingSeams` — and **the default is the mock, with a typo falling closed to
|
||||
the mock**. Client: the domain's `Api` interface in `services/{domain}/types.ts`, implemented by
|
||||
`clientApi.ts` and `mockApi.ts`, selected in `apis/index.ts` by `USE_{DOMAIN}_MOCK`.
|
||||
2. **Selection by registration, never by branching.** No `if (mock)` inside a handler, hook, or component.
|
||||
Swapping a mock for the real thing is a one-line registration change and touches no caller.
|
||||
3. **A record**, in `docs/status/`: the seam (interface name + file), what is faked, why, the config keys
|
||||
it reads, and **step-by-step how to make it real** — which provider, which settings, which methods,
|
||||
what to test.
|
||||
|
||||
An unrecorded mock is a defect, because the next agent cannot tell a deliberate stand-in from a bug.
|
||||
|
||||
Two mocks in this repo are **deliberate MVP endpoints, not stand-ins waiting for a vendor**:
|
||||
`ICredentialVerifier` / `ILicenseVerificationService` stay mock because manual MoH / INO / eNamad review
|
||||
*is* the intended MVP — there is no public B2B API. Don't "finish" them.
|
||||
|
||||
---
|
||||
|
||||
## 5. Scale and cost are part of correctness
|
||||
|
||||
Every decision should consider what it costs at scale, not only whether it works once:
|
||||
|
||||
**Server** — indexing, pagination on every unbounded list, caching read-heavy and reference data behind
|
||||
the cache seam, idempotency and locks on the money path, the DB constraint as the authoritative backstop
|
||||
behind every friendly pre-check.
|
||||
|
||||
**Client** — query caching with a deliberate `staleTime` so you never refetch what you already hold,
|
||||
invalidation on mutation, re-render cost (stable references, `select` to subscribe to a slice, state
|
||||
colocated low), and bundle size.
|
||||
|
||||
And in both: the seam that lets a mock become real without touching a caller.
|
||||
|
||||
---
|
||||
|
||||
## 6. Configuration lives in files, not a secret store
|
||||
|
||||
`dotnet user-secrets` is **not used** in this repo, and the `<UserSecretsId>` was removed from
|
||||
`Baya.Web.Api.csproj`, so that store **is not read at all**. Any instruction telling you to set a value
|
||||
with `dotnet user-secrets` is stale.
|
||||
|
||||
| Where config lives | What |
|
||||
| --- | --- |
|
||||
| `server/src/API/Baya.Web.Api/appsettings.*.json` | Server config, including dev crypto keys |
|
||||
| `client/.env.development` / `.env.production` | Client config |
|
||||
| root `docker-compose.yml` | The deployment's container-specific overrides |
|
||||
|
||||
This is a deliberate pre-launch trade for a demo deployment, which means **the repo contains live
|
||||
credentials**. Before onboarding real users they must be rotated and the secret half moved out of git —
|
||||
see [`DEPLOY.md`](../../../DEPLOY.md) "Going to Production". Never hardcode a secret in code either way:
|
||||
keys, connection strings, and tokens come from configuration bound to typed settings, never a literal in
|
||||
a handler, service, or component.
|
||||
|
||||
One value is load-bearing and must never change: `Seams:FieldEncryption:Key` / `:HashKey` decrypt all
|
||||
existing PII and derive the phone-lookup hash. Changing them makes every PII read throw and every phone
|
||||
lookup miss.
|
||||
@@ -0,0 +1,131 @@
|
||||
# Git and the quality gates
|
||||
|
||||
What must pass before work is done, and what the repo refuses to let you commit.
|
||||
|
||||
> Last verified: 2026-07-30 against commit `d3ec723`.
|
||||
|
||||
---
|
||||
|
||||
## 1. The gates
|
||||
|
||||
Each project is built, linted, and tested **on its own**. There is no root-level build, package, or
|
||||
solution, so there is no single command that gates the repo. Run the gate for the side you edited.
|
||||
|
||||
### Client — `cd client`
|
||||
|
||||
| Command | What it runs |
|
||||
| --- | --- |
|
||||
| `npm run check` | **The gate.** `type` → `lint` → `lint:copy`, in that order |
|
||||
| `npm run type` | `tsc --noEmit` (`strict` on) |
|
||||
| `npm run lint` | `eslint .` (flat config) |
|
||||
| `npm run lint:copy` | `node scripts/check-copy.mjs` — greps `fa.json` for banned Persian orthography variants |
|
||||
| `npm run test:ci` | `jest --ci` — **also required** when you touched a component with a co-located `*.test.tsx` |
|
||||
|
||||
`npm run check` must be green. `en.json` and `fa.json` must be in sync.
|
||||
|
||||
> `lint:copy` is part of `check`, not a separate step you can forget. It is what stops a copy regression
|
||||
> — a hamza-less «تایید», a space in the brand name — from needing to be re-discovered by a human. The
|
||||
> rules it enforces are in [client/i18n.md](../client/i18n.md).
|
||||
|
||||
### Server — `cd server`
|
||||
|
||||
| Command | What it runs |
|
||||
| --- | --- |
|
||||
| `dotnet build Baya.sln` | **Zero new warnings.** Unused usings, locals, parameters, private fields or members count as failures — delete them, don't suppress them |
|
||||
| `dotnet test Baya.sln` | All tests pass, including the ones your change adds |
|
||||
|
||||
A reachable SQL Server is required to run the API (not to build or unit-test it).
|
||||
|
||||
### Both
|
||||
|
||||
Read your own diff as if you were reviewing the PR: **would a senior engineer approve it without
|
||||
comment?** A change that passes the mechanical gate and fails that question is not done.
|
||||
|
||||
---
|
||||
|
||||
## 2. What "done" means
|
||||
|
||||
A change is done when all of these hold:
|
||||
|
||||
- [ ] The full scope is implemented. No `// TODO: implement later`, no stub that returns fake data.
|
||||
Anything not real is behind a **DI-registered seam** and recorded (see [code-quality.md](code-quality.md)).
|
||||
- [ ] It follows the rules for that project — the relevant `CLAUDE.md` plus the one reference file for the
|
||||
area you touched.
|
||||
- [ ] No dead code. Comments explain *why*, not *what*.
|
||||
- [ ] The project's own gate above is green.
|
||||
- [ ] If the structure changed, the matching **architecture section** is updated in the same change
|
||||
(see [documentation.md](../documentation.md) §3).
|
||||
- [ ] If a business rule was discovered or decided, `product/` reflects it — recorded, not invented.
|
||||
- [ ] If a new reusable pattern or seam landed, the reference file for that area names it, so the next
|
||||
change reuses it instead of reinventing it.
|
||||
|
||||
A change that doesn't pass its own gate is **not done**, regardless of how complete the code looks.
|
||||
|
||||
---
|
||||
|
||||
## 3. The pre-commit secret scan
|
||||
|
||||
Repo-managed hooks live in `.githooks/` (in version control, unlike `.git/hooks`). **Enable them once per
|
||||
clone:**
|
||||
|
||||
```bash
|
||||
git config core.hooksPath .githooks
|
||||
```
|
||||
|
||||
`pre-commit` is a fast, dependency-free backstop against a credential leaking into a file that shouldn't
|
||||
hold one. It scans **only staged additions**, so it is quick. It rejects a commit that stages:
|
||||
|
||||
- the retired hardcoded admin password `qw123321`, anywhere;
|
||||
- private-key material or an AWS access-key id, anywhere;
|
||||
- the deployment's SQL Server host `87.107.152.16` **outside the declared config files**;
|
||||
- a **real** connection-string password in any `appsettings*.json` **outside the declared config files**
|
||||
(elsewhere only the `SET_VIA_USER_SECRETS_OR_ENV` placeholder is allowed).
|
||||
|
||||
### The declared-config allow-list
|
||||
|
||||
The pre-launch demo deployment configures itself from committed files rather than a secret store (see
|
||||
[`DEPLOY.md`](../../../DEPLOY.md)), so a short allow-list is exempt from the last two checks:
|
||||
|
||||
`appsettings.Development.json` · `docker-compose.yml` · `telegram-otp-bot/.env.example` · `DEPLOY.md`
|
||||
|
||||
It is maintained in the `declared_config` function in the hook, and it is **the honest record of where
|
||||
this repo's secrets are**. **Shrink it, never grow it.** Once real users exist, those values must be
|
||||
rotated and moved out of git.
|
||||
|
||||
### Limits
|
||||
|
||||
This is the local first line of defence, **not** a replacement for a full scanner (gitleaks, trufflehog)
|
||||
in CI. Bypass a false positive with `git commit --no-verify` — sparingly, and only when you are certain
|
||||
the flagged line is not a secret.
|
||||
|
||||
---
|
||||
|
||||
## 4. Branches and commits
|
||||
|
||||
`main` is the default branch and the base for PRs.
|
||||
|
||||
- **Commit or push only when asked.** If you are on `main` and about to commit, branch first.
|
||||
- One coherent change per commit. The repo's history reads as a sequence of completed units of work
|
||||
(`ui phase 11`, `remove user-secrets approach & prepare a pilot deploy`) — keep that.
|
||||
- Never skip hooks (`--no-verify`) or bypass signing unless explicitly asked. If a hook fails,
|
||||
fix the underlying issue.
|
||||
- Prefer a new commit over amending an existing one.
|
||||
- Before a destructive git operation (`reset --hard`, `push --force`, `checkout --`), consider whether a
|
||||
safer route reaches the same place.
|
||||
|
||||
---
|
||||
|
||||
## 5. Known pre-existing warnings
|
||||
|
||||
These are expected and **must not be "fixed"** unless a task says so — a change that touches them is
|
||||
scope creep, and one that silences them is worse.
|
||||
|
||||
| Warning | Project | Note |
|
||||
| --- | --- | --- |
|
||||
| `NU1510` on `Microsoft.Extensions.Logging.Debug` | `Baya.Web.Api` | Redundant transitive reference, harmless |
|
||||
| `NETSDK1057` (preview SDK) | all server projects | The .NET 10 SDK is preview on this machine |
|
||||
|
||||
On the client, `import/no-cycle` is disabled in `eslint.config.mjs` (its TypeScript resolver has an
|
||||
interface mismatch with this toolchain), and **ESLint is pinned to 9** — ESLint 10 crashes against this
|
||||
Next 16 toolchain with `scopeManager.addGlobals is not a function`. See
|
||||
[client/testing.md](../client/testing.md).
|
||||
@@ -0,0 +1,94 @@
|
||||
# Naming
|
||||
|
||||
The names that are load-bearing across both projects, and the ones that are only conventions.
|
||||
|
||||
> Last verified: 2026-07-30 against commit `d3ec723`.
|
||||
|
||||
---
|
||||
|
||||
## The two names, and why there are two
|
||||
|
||||
The product and brand are **Balinyaar** (Persian: «بالینیار»). The server's code namespace is **`Baya*`**
|
||||
— a legacy prefix from before the name settled.
|
||||
|
||||
| Layer | Name | Rule |
|
||||
| --- | --- | --- |
|
||||
| Server namespaces, projects, solution | `Baya.*` / `Baya.sln` | Keep it. **Do not rename without explicit instruction** — it touches 14 `.csproj` files, every namespace, and the solution. |
|
||||
| Client package | `balinyaar-client` | — |
|
||||
| Client import alias | `@/*` → `client/src/*` | Defined in `client/tsconfig.json`. Use it; don't write deep relative paths across folders. |
|
||||
| User-facing copy | «بالینیار» / "Balinyaar" | Never `Baya`. See [client/i18n.md](../client/i18n.md) for the ZWNJ rule — it is linted. |
|
||||
|
||||
So `Baya.Application` is correct in C# and wrong in a UI string, and «بالینیار» is correct in a UI string
|
||||
and would be wrong as a namespace. That is the whole split.
|
||||
|
||||
---
|
||||
|
||||
## Agent-facing docs
|
||||
|
||||
`CLAUDE.md` is the single source of truth at every level of the repo. `AGENTS.md` files exist only so the
|
||||
convention is discoverable under that name too — they are **thin pointers**, never content. If you find
|
||||
yourself writing a rule into an `AGENTS.md`, it belongs in the `CLAUDE.md` beside it.
|
||||
|
||||
Three `AGENTS.md` files exist: repo root, `client/`, `server/`.
|
||||
|
||||
---
|
||||
|
||||
## Server naming
|
||||
|
||||
Full C# conventions in [server/conventions.md](../server/conventions.md). The names that matter beyond
|
||||
style:
|
||||
|
||||
| Kind | Convention | Example |
|
||||
| --- | --- | --- |
|
||||
| Command | `{Verb}{Noun}Command` | `CreateOrderCommand` |
|
||||
| Query | `{Verb}{Noun}Query` | `GetUserOrdersQuery` |
|
||||
| Handler | `{RequestName}Handler` | `CreateOrderCommandHandler` |
|
||||
| Result DTO | `{RequestName}Result` | `CreateOrderCommandResult` |
|
||||
| Feature folder | `Features/<Area>/{Commands\|Queries}/<VerbNoun>/` | `Features/Payments/Commands/InitiatePayment/` |
|
||||
| EF config folder | `Persistence/Configuration/<Area>Config/` | `PaymentsConfig/` |
|
||||
| Seam interface | `I{Capability}` in `Application/Contracts/` | `IBankTransferProvider` |
|
||||
| Real adapter | `{Vendor}{Capability}` in `Seams/Real/` | `JibitBankTransferProvider` |
|
||||
| Mock adapter | `Mock{Capability}` in `Seams/` | `MockBankTransferProvider` |
|
||||
|
||||
**Controller and action names become URLs.** All URL segments are `snake_case`, produced automatically
|
||||
from `[controller]`/`[action]` tokens by `SnakeCaseParameterTransformer`. So `GetBySlug` becomes
|
||||
`get_by_slug`. If a method name doesn't read cleanly as a URL, **rename the method** — never hardcode the
|
||||
route string, which bypasses the transformer.
|
||||
|
||||
One type per file, and the file name matches the type name exactly.
|
||||
|
||||
---
|
||||
|
||||
## Client naming
|
||||
|
||||
| Kind | Convention | Example |
|
||||
| --- | --- | --- |
|
||||
| Shared component | `src/components/<Name>/<Name>.tsx` + `index.tsx` barrel | `components/TrustBadge/TrustBadge.tsx` |
|
||||
| Its test | co-located `<Name>.test.tsx` | `components/TrustBadge/TrustBadge.test.tsx` |
|
||||
| Page body | `<PageName>Screen.tsx`, co-located with `page.tsx` | `HomeScreen.tsx`, `SearchScreen.tsx` |
|
||||
| Private (non-route) folder under `app/` | `_`-prefixed | `_chrome/`, `_hub/` |
|
||||
| Route group (adds no URL segment) | parenthesised | `(customer)`, `(public-routes)` |
|
||||
| Service domain | `src/services/{domain}/` | `services/bookingRequests/` |
|
||||
| Query hook | one per file, `hooks/use{Action}.ts` | `hooks/useBookingDetail.ts` |
|
||||
| Icon registry key | **lowercase**, semantic | `icon="verification"`, not `icon="ShieldCheck"` |
|
||||
| i18n namespace | a top-level key in both message files | `booking`, `payouts` |
|
||||
| Constant | `SCREAMING_SNAKE` in a `constants.ts` | `APP_FRAME_MAX_WIDTH` |
|
||||
|
||||
`bookings` and `bookingRequests` are **siblings, not a rename** — a booking request is the money-free
|
||||
pre-payment intent, a booking exists only after capture. The same distinction is load-bearing in Persian
|
||||
copy («درخواست رزرو» vs «رزرو») and in the server's singular `Booking` vs plural `Bookings` feature areas.
|
||||
|
||||
---
|
||||
|
||||
## Directory conventions that carry meaning
|
||||
|
||||
| Path | Meaning |
|
||||
| --- | --- |
|
||||
| `client/src/components/common/` | Foundational primitives, imported via `@/components` |
|
||||
| `client/src/components/<domain>/` | Domain composites (`booking/`, `messaging/`, `admin/`, `geography/`, `notifications/`, `settings/`, `auth/`) |
|
||||
| `client/src/services/{domain}/apis/` | The seam: `clientApi.ts` (real), `mockApi.ts`, `serverApi.ts`, `index.ts` (selects) |
|
||||
| `server/src/Core/` | Domain + Application — no outward dependencies |
|
||||
| `server/src/Infrastructure/` | Implementations of Application contracts |
|
||||
| `server/src/API/` | Controllers, framework, plugins |
|
||||
| `dev/` | The finished build-plan chain. History, not a project — nothing to build in it |
|
||||
| `product/` | Business truth. Markdown canonical, HTML generated |
|
||||
Reference in New Issue
Block a user