Files
tehran_tala/docs/parity/ISSUES.md
T
s.esmaeiliandClaude Opus 5 64e76a8616 parity(admin): start the Phase 1 matrix — rid mapping, price and order sections
Answers D1's 172-vs-174 question with real counts: the catalogues hold 176
(reference) and 174 (target) unique keys, and the ENTIRE difference is the
dashboard. The reference has `dashboard` + `dashboard.omde` + `dashboard.tak`;
the target replaces all three with `dashboard.view`, because the admin
Dashboard is a PROTECTED zone that merged the two reference dashboards.
176 - 3 + 1 = 174. All 173 other keys match exactly, including the same 5
keys that legitimately appear twice in the tree.

Route-guard rids are 57 vs 57, differing only by that dashboard
consolidation plus one target-only route reusing an existing catalogue rid.
All 67 reachable reference admin routes exist in the target at the same
paths; 2 target-only routes are additive.

Three findings logged:

PRICE-01 (critical) — no product price is computed or shown anywhere in the
admin panel. The reference appends price/price_with_discount to every
variant and renders it in three places in its product list; the target's
variantRow() returns neither, nothing fetches a gold rate for the admin
list, and computeVariantPrice() — ported and unit-tested earlier today — is
called from nowhere. This is fix #1.

PERM-01 (medium) — dashboard.omde and dashboard.tak are seeded into the
permission table but absent from the permission UI, so they are grantable
yet unmanageable; the DB holds 178 distinct rids against the UI's 174. The
fix deletes permission rows, so it waits for the owner's approval.

PRICE-02 (info) — rial_pay belongs only to the superseded commented-out
formula; the target correctly omits it. Recorded so it is not "restored".

Contrary to the Phase 0 expectation, the order module is in good shape: the
gram_level credit limit matches the reference exactly including the strict
'>' comparison, and debt recognition fires at the same status.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
2026-09-22 16:17:31 +03:30

129 lines
22 KiB
Markdown
Raw Blame History

This file contains ambiguous Unicode characters
This file contains Unicode characters that might be confused with other characters. If you think that this is intentional, you can safely ignore this warning. Use the Escape button to reveal them.
# ISSUES — logged, NOT fixed during parity
Anything found during the parity audit that is a bug, risk, smell or improvement
opportunity goes here. **Do not fix these while parity work is in progress** (see
`CLAUDE.md` §3). They are worked through in Phase 3, only when the user asks.
Severity: `Critical` (money loss / security / data loss) · `High` · `Medium` · `Low`
> Paths are relative to the workspace root (`gold bank/`).
> "REF" = reference project (read-only). "TGT" = `gold-banking-monorepo`.
> Items marked **[replicated]** exist in the reference and were faithfully ported —
> fixing them is a product decision, not a parity fix.
## Security & auth
| ID | Severity | Where | Description | Evidence | Suggested fix |
|---|---|---|---|---|---|
| SEC-01 | Critical | REF `henza_server/routes/admin.php:12,272` | The whole admin route file is **unauthenticated** — the `Route::middleware('auth:api')->group(...)` wrapper is commented out at both ends, leaving all ~215 admin routes public (incl. `wallet/charge`, `users`, `roles`). | Verified by reading both lines. | n/a in reference (read-only). Drives TGT decision below. |
| SEC-02 | ~~Critical~~ **FIXED — hotfix stage 3** | TGT `apps/api/src/modules/accounting/accounting.controller.ts:7`, `orders/orders.controller.ts:7`, `customers/customers.controller.ts:5`, `catalog/catalog.controller.ts:5`, `pricing/pricing.controller.ts:5` | 15 routes carry `@Public()`, including **ledger writes** (`POST accounting/repayments/*`, `entries/:id/confirm`) and `POST /api/orders` + `PATCH /api/orders/:id/status`. The public status DTO accepts a client-supplied `actorAdminUserId`, so a caller can attribute an action to any admin. | `grep -rn "@Public()"`; source comments say "pre-existing unauthenticated endpoint". | ✅ All closed across stages 1-3. Ledger + order-status behind admin rids; customer routes behind the customer JWT; `POST /orders` takes the customer from the token and ignores the body. Only `catalog/products`, `pricing/latest`, `home/slider` and `uploads/:filename` remain public, all by design. |
| SEC-03 | ~~Critical~~ **FIXED — hotfix stage 2** | TGT `apps/user-app/src/views/user/LoginView.vue`, `apps/user-app-v2/` same | OTP is **not verified** — any 4-digit code logs the user in. This is a **parity gap**: the reference does verify it (`ApiMobileController.php:51-57`, mismatch → HTTP 422). | `DECISIONS.md` D6 | ✅ Real server-side OTP: bcrypt-hashed code, 2-min expiry, 5 attempts, single use, send throttle, no enumeration. Verified end-to-end by `auth-smoke.sh`. |
| SEC-08 | ~~Critical~~ **FIXED — hotfix stage 2** | TGT `apps/api/src/modules/customers/customers.service.ts:6-23,44-50` + `customers.controller.ts:10` (`@Public()`) | **Full customer book exposed to anonymous callers.** Login downloads `GET /customers` and matches the phone client-side, so the endpoint returns **every** active customer with `fullName`, `mobile`, **`nationalCode`**, `goldDebtGrams`, `fiatDebtRial`, `goldCreditLimitGrams`, and the URLs of **national-ID / business-licence / shop scans** (and `GET uploads/:filename` is public too, so those URLs resolve). `passwordHash` is correctly excluded. Not possible in the reference, where `userVerfiy` returned **one** customer after the code matched. | Read `customers.service.ts:5-23`; `DECISIONS.md` D8 | ✅ Closed. The whole controller is deleted — no route accepts a customer id any more. Reads moved to token-derived `/customer/me*`. Stage 1 had only narrowed the fields, which left the ids and the `:id` routes exposed. |
| SEC-09 | ~~Critical~~ **FIXED — hotfix stage 1** | TGT `apps/api/src/modules/accounting/accounting.controller.ts:7` | `@Public()` on the whole accounting controller exposes **5 routes, 4 of them writes**: reconcile a balance, record a gold repayment, record a rial repayment, confirm a ledger entry. Faithful to the reference (whose admin routes were all unauthenticated) but contradicts the confirmed decision D1. | `DECISIONS.md` §"@Public audit" group A | ✅ Done: rids `wallet.list`, `wallet.charge`, `document.create` ×2, `document.list.confirm`. Verified 401/403/pass-through by `apps/api/scripts/auth-smoke.sh`. |
| SEC-10 | ~~High~~ **FIXED — hotfix stage 1** | TGT `apps/api/src/modules/orders/orders.controller.ts:22` | `PATCH orders/:id/status` is public **and** its DTO accepts a client-supplied `actorAdminUserId`, so an anonymous caller can move an order's status and attribute it to any admin. In the reference this was an admin operation. | `DECISIONS.md` §"@Public audit" B3 | ✅ Done: `order.list.sts.submit`. Had no caller — the admin panel uses `admin/orders/status`. |
| SEC-04 | High | TGT `apps/api` (two files) | `JWT_SECRET` falls back to `'dev-only-insecure-secret'` and is absent from `.env.example`. `enableCors()` is wide open. | Reported by api inventory; see `_inv-tgt-api.md` §11. | Require the env var at boot; restrict CORS origins. |
| SEC-05 | High | TGT `apps/api/prisma/seed.ts` | Seed creates a SUPER_ADMIN (`09128081392`) whose **password equals the phone number**, holding all 144 rids. Marked DEV-ONLY in comments but lives in the normal seed. | `docs/CHANGELOG-PORT-HISTORY.md` §4 documents this as intentional for dev. | Gate behind an explicit env flag so it can never run outside dev. |
| SEC-06 | High | REF `henza_server` | Hardcoded secrets committed to the repo: SMS panel password, PayPing bearer token, Tahesab bearer + IP allowlist, wallet hash pepper (`"yhoenszeaf"`). | See `_inv-ref-server.md` §11.4. | Rotate all of them before any deployment that reuses these credentials. |
| SEC-07 | Medium | TGT `apps/api/src/modules/admin-customers/admin-customers.service.ts` | `setRoles` stores arbitrary strings as rids with no validation against `ALL_RIDS`. | api inventory §9. | Validate against the rid catalogue. |
## Customer / person module (found during the user-requested deep check)
> These are **parity defects being fixed**, not deferred items — see
> `DECISIONS.md` D0, D9, D10, D11. Listed here so the trail is complete.
| ID | Severity | Where | Description | Evidence | Status |
|---|---|---|---|---|---|
| CUST-01 | Critical | TGT `apps/api/src/modules/admin-customers/admin-customers.service.ts:231-233` | **First and last name collapse.** The reference stores `fname`/`lname` as two columns; the target merged them into `fullName` and returns `fname: c.fullName, lname: ''`. Opening any edit form puts the whole name in the first-name box and blanks the surname; saving then persists that. `singleCustomer.vue` tries to compensate with `split(' ')`, which mis-splits multi-part names. | REF `database/migrations/2020_10_01_120430_customers_table.php:18-19` (two columns) vs TGT `:231-233`. Reported by the user from live use. | **FIXING — D0** |
| CUST-02 | Critical | TGT same file `:245`, `:600-658` | **`percent` read from the wrong entity and never written.** Target returns `c.referredByAgent?.wagePercent`; the reference reads the customer's **own** `percent` column. `update()` ignores `data.percent` entirely, so editing the field does nothing. The create-time default (3) and the admin-path `required` rule are both missing, as is the app-side `filterPercentPay` offset. | REF `database/migrations/2020_10_19_153228_customer_add.php:32`; `AdminController.php:338,354`; `ApiMobileController.php:391-396,76-77`. Full trace in `DECISIONS.md` §percent. | **FIXING — D9** |
| CUST-03 | High | TGT `apps/admin-panel/src/views/apps/customer/ReagentCustomer.vue:97-110` | **Sales-expert form is missing 5 reference fields:** `ncode`, `phone`, `city_id`, `gram_level`, `reagent_id`. The reference form had 15; the target has 8. Likely because `AdminUser` lacks those columns. | REF `henza_admin/src/views/apps/customer/ReagentCustomer.vue` (15 `v-decorator` fields) vs TGT form object. | **INVESTIGATING — D10** |
| CUST-04 | Low | TGT `apps/admin-panel/src/views/apps/customer/singleCustomer.vue:91` | Still spreads the whole form on save while the other two customer forms send a diff. Harmless today (4 fields, all repopulated) but the same shape that caused the earlier data loss. | read at `:80-106` | **FIXING — D11** |
| CUST-05 | Low | TGT `apps/admin-panel/src/views/apps/customer/bulkCustomer.vue` | Diff-send on edit is a **deliberate deviation** from the reference, which posts all 15 fields on every save. Recorded so it is not "fixed" back. | `DECISIONS.md` D11 | accepted deviation |
## Money, stock & data integrity
| ID | Severity | Where | Description | Evidence | Suggested fix |
|---|---|---|---|---|---|
| MON-01 | Critical | TGT `apps/user-app-v2/src/stores/cartStore.ts` | v2 has **no `capQty` / `maxQty` stock ceiling**, which `user-app` does have (`apps/user-app/src/stores/cartStore.ts:62,65`; `components/product/CartItem.vue:82-83`). A v2 user can order more units than exist. | Verified by grep across both apps — zero hits in v2. | Port the ceiling to v2 (this is also a **Phase 2 parity gap**, row to be added to `02-APPS-MATRIX.md`). |
| MON-02 | High | REF `henza_server` wallet package | The wallet `balance` column **mixes grams and toman in one field**, and money operations are not wrapped in DB transactions. | `_inv-ref-server.md` §7. | Separate the units; wrap mutations in transactions. |
| MON-03 | High | TGT `apps/api/src/modules/accounting/ledger.service.ts:189` | `GRAMS_PER_MESGHAL = 4.331802` is a JS **float** used inside an otherwise-`Prisma.Decimal` CASH repayment formula, so the divisor is pre-rounded before the Decimal division. | Read at `ledger.service.ts:182-190`; the source comment already flags the formula for stakeholder review. | Make the constant a `Decimal` (or a string literal passed to `Decimal`). |
| MON-04 | High | TGT `apps/api/src/modules/orders/orders.service.ts` | Debt is recognised at `DELIVERED_TO_AGENT`, but the idempotency guard checks `deliveredAt`, which is only set by `DELIVERED_TO_CUSTOMER` — so the debt trigger is not reliably idempotent. There is also **no status-transition validation** anywhere. | api inventory §8. | Guard on the same field the trigger fires on; add an explicit transition table. |
| MON-05 | Medium | TGT `apps/api` | `canPlaceOrders` and `verificationStep` are stored and toggled by the admin panel but **never enforced** on any order path. | api inventory §8. | Enforce at order creation, or remove the toggles. |
| MON-06 | Medium | TGT `apps/api` | `Setting` rows are never seeded — `wallet.config`, `wallet.birthday`, `promotion.auto`, `mobile_main_slider` only materialise on first write, so a fresh install has undefined behaviour until an admin saves each one. | api inventory §7. | Seed the defaults. |
| MON-07 | Medium | REF `henza_server/app/Models/ProductVariant.php:183-190` vs `SingleCustomerOrders.php:67`, `Order.php:66` | Two **divergent order-status enums** coexist (0–6 and 1–7) for what is conceptually one flow. | Read both models. | Unify, or document the mapping explicitly. |
## Validation & contracts
| ID | Severity | Where | Description | Evidence | Suggested fix |
|---|---|---|---|---|---|
| VAL-01 | High | REF `henza_server` | **Zero FormRequests**; all validation is inline, and `setOrder`, wallet `charge`, `setConst` and file upload have **no validation at all**. The wallet package's own validator is dead code (it calls `failed()` instead of `fails()`). | `_inv-ref-server.md` §12. | n/a in reference; informs how much validation the target must add. |
| VAL-02 | High | TGT `apps/api` | Only **8 DTO classes** exist; most admin request bodies are untyped `Record<string, unknown>`, so class-validator never runs on them. | api inventory §5. | Add DTOs for the admin write paths. |
| VAL-03 | Medium | TGT both user apps | HTTP client is a bare `fetch` wrapper: **no interceptors, no Authorization header, no refresh, no 401 handling, no error toasts**. Non-2xx throws a generic `Error`; call sites mostly `console.log(e)`. Order-submit failure is **silent to the user**. | `apps/user-app/src/api/client.ts`, `apps/user-app-v2/src/api/client.ts:3`. | Add a shared client with error surfacing; compare against `henza_pwa/src/service/*` in Phase 2. |
## Parity-relevant divergence between user-app and user-app-v2
> `CLAUDE.md` §2: the only allowed difference is visual design. These are logic
> differences, so they are **Phase 2 parity gaps** as well as issues.
| ID | Severity | Where | Description | Evidence |
|---|---|---|---|---|
| V2-01 | Critical | v2 missing stock ceiling | see MON-01 | grep |
| V2-02 | Medium | v2 missing `getHomeSlider()` / `GET /home/slider` admin banner | v1 `api/client.ts:152-170` defines `AdminSlide` + `getHomeSlider`; v2 has neither | `diff apps/user-app/src/api/client.ts apps/user-app-v2/src/api/client.ts` |
| V2-03 | Medium | v2 missing per-variant images | v1 defines `ProductImageSummary` (`id/url/sortOrder/role`) and `imageId`/`image` on the variant; v2's `images` is just `Array<{url:string}>` | same diff |
| V2-04 | Medium | v2 missing orderable-first variant defaulting | v1 `composables/useProduct.ts:26` picks the first orderable variant; v2 has no equivalent | grep |
| V2-05 | Low | v2 missing variant-chip preview cap + per-chip stock badges | v1 `components/product/SingleProduct.vue:388-392` | grep |
## Dead code & maintenance
| ID | Severity | Where | Description |
|---|---|---|---|
| DEAD-01 | Low | TGT `apps/user-app` | 8 components with no importers, `loadingStore`, `api.getBalance` (zero call sites), unused `gsap` dependency. |
| DEAD-02 | Low | TGT `apps/user-app-v2` | 11 dead components, `loadingStore`, `getBalance`, `priceUnit`/`priceUnitRialLess`, 12 of 15 validators, unused `gsap` + `@ant-design/icons-vue` deps. |
| DEAD-03 | Low | TGT `gold-banking-monorepo/user-app-v2/` (repo root, outside `apps/`) | Stray leftover folder containing only `package.json` and an **older** `vite.config.ts` (hardcoded port 5179, hardcoded proxy, imports a `themeConfig` that doesn't exist there). Not matched by `pnpm-workspace.yaml`, so pnpm ignores it. Left in place — deleting it needs user approval. |
| DEAD-04 | Low | REF `henza_server/app/Console/Kernel.php` | `schedule()` is **empty** — no cron. Price refresh / expiry / birthday logic must be driven by OS cron or the `GET /` web route. |
| DEAD-05 | Low | REF `henza_server` | PayPing `returnUrl` points at `site/transaction/verify`, which is **not registered in any route file** — the payment callback is unreachable. |
## Known stubs in the target (not bugs, but not parity either)
| ID | Where | Description |
|---|---|---|
| STUB-01 | TGT both user apps | Registration persists nothing; favourites are in-memory only (lost on reload); notifications is a static empty state; home slides are 3 hardcoded placeholders in v2. |
| STUB-02 | TGT `apps/api` | No OTP, no SMS provider, no payment gateway/callbacks, no cron, no websockets. SMS and notifications are DB-persistence only. |
| STUB-03 | TGT both user apps | All list filtering and "pagination" is **client-side** over one flat `GET /catalog/products`; no server `page`/`limit` params. "Pagination" is a 20-item IntersectionObserver window. |
| STUB-04 | TGT both user apps | Live price is **pull-on-mount only** (`GET /pricing/latest?karatPerMille=750`) — no socket, no SSE, no polling interval. `quotedAt` renders as a Jalali **date only**, so a stale rate looks current. |
## Found during the stage-3 OTP audit
| ID | Severity | Where | Description | Evidence | Status |
|---|---|---|---|---|---|
| SEC-11 | High | TGT `apps/api/src/modules/customer-auth/customer-auth.service.ts` (deliver) | The OTP message text, **code included**, was persisted to `SmsMessage.body`, which the admin SMS grid lists and full-text searches (`admin-misc.service.ts:135-136`). Any operator holding the `sms` rid could read live codes — defeating the bcrypt hashing of `codeHash`. | Read both files; grid `allowed` includes `body`. | ✅ Fixed in stage 3: the persisted row is a redacted audit line. |
| MON-08 | Medium | TGT `apps/api/src/modules/orders/orders.service.ts:40-52` | `wagePercent`, `discountPercent` and `flatRialCharge` are taken from the **client** on order creation, so a caller can set their own making charge. `rashWeightGrams` is correctly read from the variant. **The reference is worse** — `ApiMobileController.php:538-539,619` takes `rash_weight` from the client too — so this is a partial improvement already. Reported per the user's instruction to flag client-supplied pricing. | REF `:536-545,615-632` vs TGT `:40-52` | Log only (D4). Fix in Phase 2 alongside the price-formula work. |
## Found while classifying scope for D13
| ID | Severity | Where | Description | Evidence | Status |
|---|---|---|---|---|---|
| DOC-01 | Medium | DOC `docs/parity/_inv-ref-server.md:588` (§3.6) | The Phase 0 inventory undercounted `routes/admin.php`: it recorded **117** explicit routes where the file has **151** live ones (6 more are commented out). Every downstream total inherited the error, so the reference endpoint count was reported as **369** when it is **412** (admin 249, not 215). No code is affected — only the planning figures. | `grep -E "^\s*Route::(get\|post\|put\|patch\|delete\|any)" routes/admin.php \| grep -v "^\s*//" \| wc -l` → 151; resources 14×7=98; 151+98=249 | ✅ Corrected in `00-INVENTORY.md` §2.1 and `DECISIONS.md` §D13. `_inv-ref-server.md` left as the raw subagent record. |
| DOC-02 | Low | REF `routes/admin.php:38` | `GET admin/get-analytics-tak` reads `SingleCustomerOrders` (`Adm/AdminController.php:557-579`), so it is a **retail-storefront** endpoint despite its neutral name. A keyword scan for `wallet\|single\|comment` misses it. Noted so the D13 scope split stays correct — anything else named neutrally but querying `SingleCustomerOrders`, `Fav`, `Address`, `Comment` or `Transaction` is storefront too. | `AdminController.php:563` — `SingleCustomerOrders::select(...)` | ✅ Classified as ⏸️ storefront in D13 (the 31st admin storefront route). |
## Found during the SMS / MON-08 investigation (2026-09-22)
| ID | Severity | Where | Description | Evidence | Status |
|---|---|---|---|---|---|
| SEC-12 | **Critical** | REF `henza_server/app/Models/Sms.php:73-77,101-105` | Live SMS-panel production credentials (username, plaintext password, sender line) are **committed to git** in the reference, duplicated across two methods. An older provider's credentials sit commented out beside them. Anyone with repo access can send SMS on the owner's account at their cost. | Read the file. | Not ported — the target will read them from env only. **The owner should rotate these credentials**; they must be assumed compromised. |
| STUB-05 | Medium | REF `ApiMobileController.php:154-158,601-605`; `Adm/OrderController.php:54-55`; `Customer/OrderController.php:141-145` | Admin notification recipients are **hardcoded mobile numbers** scattered across controllers (six distinct numbers). Staff changes require a code deploy, and an ex-employee keeps receiving order data. | Read the files. | Port as `SMS_ADMIN_RECIPIENTS` (comma-separated, default empty) rather than copying the literals. Pending the SMS approval. |
| DEAD-06 | Low | REF `henza_pwa/src/views/cart.vue:181-182` | The cart sends `wage_weight: 5.7856000000000005` and `weight: 6.303744000000001` as **literal constants**, each with a `// how to get?!` comment. The server ignores both: it recomputes weight from `rash_weight`, and the `wage_weight` use is commented out (`ApiMobileController.php:549-551`). | Read both files. | Dead payload fields — do not port. |
## Found while applying the D0 / percent migrations (2026-09-22)
| ID | Severity | Where | Description | Evidence | Status |
|---|---|---|---|---|---|
| DEAD-07 | Low | TGT DB table `_goldbank_migrations` + `apps/api/src/prisma/migrate.ts` | **Re-assessed 2026-09-22 — not dead code.** It is the tracking table of the app's OWN migration runner (`src/prisma/migrate.ts:5`), which exists because the Docker deploy image is copy-only and ships no Prisma CLI, so `migrate deploy` cannot run there (`migrate.ts:7-13`). So the project has TWO ledgers on purpose: `_prisma_migrations` (CLI, used in development) and `_goldbank_migrations` (runtime runner, used in the container). `migrate diff` proposed `DROP TABLE` only because the table was not declared in the schema; the runner already refused to execute such a statement (`migrate.ts:190-196`). | Read `src/prisma/migrate.ts`. | ✅ Resolved: declared as an `@@ignore`d model, so drift is now **zero**. Table and all rows kept. |
## Found in the Phase 1 admin matrix (2026-09-22)
| ID | Severity | Where | Description | Evidence | Status |
|---|---|---|---|---|---|
| PRICE-01 | **Critical** | TGT `apps/api/src/modules/admin-products/admin-products.service.ts:188-208`; `apps/admin-panel/src/views/apps/product/ProductList.vue` | **No product price is computed or shown anywhere in the admin panel.** The reference appends `price` and `price_with_discount` to every variant (`ProductVariant.php:115-119`), sets the live gold rate before listing (`AdminController.php:97-98`), and renders `{{ item.price }}` in three places in its product list (`ProductList.vue:283,287,301`). The target's `variantRow()` returns neither field, nothing fetches a gold price for the admin list, and `computeVariantPrice()` — ported and unit-tested today — is called from nowhere. | `grep computeVariantPrice src/ --include=*.ts` matches only its own file and spec. | Open — **fix #1 of Phase 1**. |
| PRICE-02 | Info | REF `app/Models/ProductVariant.php:163` vs TGT `admin-products.service.ts:206-208` | `rial_pay` (a flat rial charge per variant) appears only in the **superseded, commented-out** formula, not the live one at `:183-190`. The target returns null rather than faking it, which matches the live reference. Recorded so nobody "restores" it from the dead code. | Read both. | ✅ No action — matches. |
| PERM-01 | Medium | TGT `prisma/seed.ts`; `apps/admin-panel/src/views/user/permissionConst.ts` | `dashboard.omde` and `dashboard.tak` are seeded into the `permission` table but are **absent from the permission UI tree**, so they can be held by a row yet never seen, granted or revoked from the admin screen — and nothing reads them any more (the protected Dashboard uses `dashboard.view`). The DB holds **178** distinct rids against the UI's **174**. | `SELECT rid FROM permission GROUP BY rid` → 178, including all three dashboard rids. | Open — the fix removes permission rows, which is a data deletion and needs the owner's approval first. |