From b5cad57baef56fe75b358cea07468c99579b999f Mon Sep 17 00:00:00 2001 From: "Md. Ashraful" Date: Fri, 16 Oct 2026 16:32:13 +0600 Subject: [PATCH 1/3] Add has_records flag to top-level list APIs List pages guessed their first-use empty state from `total === 0` and the active filters. The server now reports whether the table holds any record, independent of search and filters. - Add ListPaginator, a Paginator subclass that adds `has_records` to the paginated payload. It accepts a bool or a fresh unfiltered query, and runs `exists()` only when the page total is 0. - Return it from products, orders, variants, collections, categories, tags, brands, customers and coupons. Products ignore trashed rows; orders scope the check to the customer when one is given. - Read `has_records` on the nine list pages and remove the @todo guesses. - Cover it with a unit test, integration tests per endpoint, and a list-page empty-state test. - Update docs/data-table.md, add the list-record-presence spec and archive the list-api-has-record change. --- app/Services/BrandService.php | 5 +- app/Services/CategoryService.php | 5 +- app/Services/CollectionService.php | 5 +- app/Services/CouponService.php | 5 +- app/Services/CustomerService.php | 5 +- app/Services/OrderService.php | 9 +- app/Services/ProductService.php | 5 +- app/Services/TagService.php | 5 +- app/Services/VariantService.php | 5 +- app/Supports/ListPaginator.php | 66 ++++++++++ docs/data-table.md | 50 +++++--- .../.openspec.yaml | 2 + .../2026-10-16-list-api-has-record/design.md | 120 ++++++++++++++++++ .../proposal.md | 45 +++++++ .../specs/list-record-presence/spec.md | 115 +++++++++++++++++ .../2026-10-16-list-api-has-record/tasks.md | 27 ++++ openspec/specs/list-record-presence/spec.md | 115 +++++++++++++++++ .../app/features/brands/pages/brands.tsx | 4 +- .../features/categories/pages/categories.tsx | 4 +- .../collections/pages/collections.tsx | 4 +- .../app/features/coupons/pages/coupons.tsx | 4 +- .../features/customers/pages/customers.tsx | 4 +- .../features/inventory/pages/inventory.tsx | 4 +- .../features/list-page-empty-state.test.tsx | 86 +++++++++++++ .../app/features/orders/pages/orders.tsx | 4 +- .../app/features/products/pages/products.tsx | 4 +- resources/app/features/tags/pages/tags.tsx | 4 +- tests/Integration/BrandApiTest.php | 38 ++++++ tests/Integration/CategoryApiTest.php | 38 ++++++ tests/Integration/CollectionApiTest.php | 38 ++++++ tests/Integration/CouponApiTest.php | 37 ++++++ tests/Integration/CustomerApiTest.php | 38 ++++++ tests/Integration/OrderApiTest.php | 37 ++++++ tests/Integration/ProductApiTest.php | 58 +++++++++ tests/Integration/TagApiTest.php | 38 ++++++ tests/Integration/VariantApiTest.php | 38 ++++++ tests/Support/ListsHaveRecords.php | 30 +++++ tests/Unit/Supports/ListPaginatorTest.php | 66 ++++++++++ 38 files changed, 1123 insertions(+), 44 deletions(-) create mode 100644 app/Supports/ListPaginator.php create mode 100644 openspec/changes/archive/2026-10-16-list-api-has-record/.openspec.yaml create mode 100644 openspec/changes/archive/2026-10-16-list-api-has-record/design.md create mode 100644 openspec/changes/archive/2026-10-16-list-api-has-record/proposal.md create mode 100644 openspec/changes/archive/2026-10-16-list-api-has-record/specs/list-record-presence/spec.md create mode 100644 openspec/changes/archive/2026-10-16-list-api-has-record/tasks.md create mode 100644 openspec/specs/list-record-presence/spec.md create mode 100644 resources/app/features/list-page-empty-state.test.tsx create mode 100644 tests/Support/ListsHaveRecords.php create mode 100644 tests/Unit/Supports/ListPaginatorTest.php diff --git a/app/Services/BrandService.php b/app/Services/BrandService.php index 0b8c28e21..1d808aa14 100644 --- a/app/Services/BrandService.php +++ b/app/Services/BrandService.php @@ -6,6 +6,7 @@ use Kirki\Ecommerce\App\Concerns\HasSortableColumns; use Kirki\Ecommerce\App\Models\Brand; +use Kirki\Ecommerce\App\Supports\ListPaginator; use Kirki\Ecommerce\App\Constants\Pagination; use Kirki\Ecommerce\Framework\Collections\Collection; use Kirki\Ecommerce\Framework\Database\Query\Paginator; @@ -58,7 +59,9 @@ protected function sortable_columns() */ public function paginated(ListFilterDTO $filters) { - return $this->list_query($filters)->paginate($filters->limit ?? Pagination::LIMIT, $filters->page ?? 1); + $page = $this->list_query($filters)->paginate($filters->limit ?? Pagination::LIMIT, $filters->page ?? 1); + + return ListPaginator::from($page, Brand::query()); } /** diff --git a/app/Services/CategoryService.php b/app/Services/CategoryService.php index ddf999f96..7fa373ffb 100644 --- a/app/Services/CategoryService.php +++ b/app/Services/CategoryService.php @@ -6,6 +6,7 @@ use Kirki\Ecommerce\App\Concerns\HasSortableColumns; use Kirki\Ecommerce\App\Models\Category; +use Kirki\Ecommerce\App\Supports\ListPaginator; use Kirki\Ecommerce\App\Constants\Pagination; use Kirki\Ecommerce\Framework\Collections\Collection; use Kirki\Ecommerce\Framework\Database\Query\Paginator; @@ -70,7 +71,9 @@ protected function sortable_columns() */ public function paginated(ListFilterDTO $filters) { - return $this->list_query($filters)->paginate($filters->limit ?? Pagination::LIMIT, $filters->page ?? 1); + $page = $this->list_query($filters)->paginate($filters->limit ?? Pagination::LIMIT, $filters->page ?? 1); + + return ListPaginator::from($page, Category::query()); } /** diff --git a/app/Services/CollectionService.php b/app/Services/CollectionService.php index 454672ca9..6a4ae9315 100644 --- a/app/Services/CollectionService.php +++ b/app/Services/CollectionService.php @@ -6,6 +6,7 @@ use Kirki\Ecommerce\App\Concerns\HasSortableColumns; use Kirki\Ecommerce\App\Models\Collection; +use Kirki\Ecommerce\App\Supports\ListPaginator; use Kirki\Ecommerce\App\Constants\Pagination; use Kirki\Ecommerce\App\DTO\Collection\CreateCollectionDTO; use Kirki\Ecommerce\App\DTO\Collection\UpdateCollectionDTO; @@ -82,7 +83,9 @@ protected function sortable_columns() */ public function paginated(ListFilterDTO $filters) { - return $this->list_query($filters)->paginate($filters->limit ?? Pagination::LIMIT, $filters->page ?? 1); + $page = $this->list_query($filters)->paginate($filters->limit ?? Pagination::LIMIT, $filters->page ?? 1); + + return ListPaginator::from($page, Collection::query()); } /** diff --git a/app/Services/CouponService.php b/app/Services/CouponService.php index d725c6564..381e348b0 100644 --- a/app/Services/CouponService.php +++ b/app/Services/CouponService.php @@ -10,6 +10,7 @@ use Kirki\Ecommerce\App\Constants\Pagination; use Kirki\Ecommerce\App\DTO\Coupon\CouponFilterDTO; use Kirki\Ecommerce\App\Models\Coupon; +use Kirki\Ecommerce\App\Supports\ListPaginator; use Kirki\Ecommerce\Framework\Database\Query\Paginator; use Kirki\Ecommerce\Framework\Database\Query\QueryBuilder; use Kirki\Ecommerce\Framework\Collections\Collection; @@ -81,7 +82,9 @@ protected function sortable_columns() */ public function paginated(CouponFilterDTO $filters) { - return $this->list_query($filters)->paginate($filters->limit ?? Pagination::LIMIT, $filters->page ?? 1); + $page = $this->list_query($filters)->paginate($filters->limit ?? Pagination::LIMIT, $filters->page ?? 1); + + return ListPaginator::from($page, Coupon::query()); } /** diff --git a/app/Services/CustomerService.php b/app/Services/CustomerService.php index 80bdef0e3..f0e897189 100644 --- a/app/Services/CustomerService.php +++ b/app/Services/CustomerService.php @@ -8,6 +8,7 @@ use Kirki\Ecommerce\App\Models\Address; use Kirki\Ecommerce\App\Constants\Order\PaymentStatus; use Kirki\Ecommerce\App\Models\Customer; +use Kirki\Ecommerce\App\Supports\ListPaginator; use Kirki\Ecommerce\App\Constants\Pagination; use Kirki\Ecommerce\Framework\Database\Query\Paginator; use Kirki\Ecommerce\Framework\Database\Query\QueryBuilder; @@ -72,7 +73,9 @@ protected function sortable_columns() */ public function paginated(ListFilterDTO $filters) { - return $this->list_query($filters)->paginate($filters->limit ?? Pagination::LIMIT, $filters->page ?? 1); + $page = $this->list_query($filters)->paginate($filters->limit ?? Pagination::LIMIT, $filters->page ?? 1); + + return ListPaginator::from($page, Customer::query()); } /** diff --git a/app/Services/OrderService.php b/app/Services/OrderService.php index 53685a0e7..aaa5c8de0 100644 --- a/app/Services/OrderService.php +++ b/app/Services/OrderService.php @@ -27,6 +27,7 @@ use Kirki\Ecommerce\App\DTO\Order\UpdateOrderDTO; use Kirki\Ecommerce\App\DTO\Order\UpdateOrderItemDTO; use Kirki\Ecommerce\App\Resources\Site\Order\OrderListResource; +use Kirki\Ecommerce\App\Supports\ListPaginator; use Kirki\Ecommerce\App\Supports\OrderNumberGenerator; use Kirki\Ecommerce\Framework\Exceptions\NotFoundException; use Kirki\Ecommerce\Framework\Http\Response; @@ -117,9 +118,15 @@ public function get_current_customer_orders($filters) */ public function paginated_orders(OrderListFilterDTO $dto) { - return $this->list_query($dto) + $page = $this->list_query($dto) ->with('items') ->paginate($dto->limit ?? Pagination::LIMIT, $dto->page ?? 1); + + $has_records = Order::query()->when(is_numeric($dto->customer_id), function (QueryBuilder $query) use ($dto) { + return $query->where('customer_id', $dto->customer_id); + }); + + return ListPaginator::from($page, $has_records); } /** diff --git a/app/Services/ProductService.php b/app/Services/ProductService.php index 8db2f9dd3..609abfd69 100644 --- a/app/Services/ProductService.php +++ b/app/Services/ProductService.php @@ -19,6 +19,7 @@ use Kirki\Ecommerce\App\DTO\Product\UpdateProductDTO; use Kirki\Ecommerce\App\DTO\Product\CreateProductDTO; use Kirki\Ecommerce\App\Jobs\PublishScheduledProductJob; +use Kirki\Ecommerce\App\Supports\ListPaginator; use Kirki\Ecommerce\App\Supports\Url; use Kirki\Ecommerce\Framework\Exceptions\NotFoundException; use Kirki\Ecommerce\Framework\Http\Response; @@ -70,7 +71,9 @@ public function paginated(ProductListFilterDTO $filters) { $query = $this->list_query(); - return $this->apply_filters($query, $filters)->paginate($filters->limit ?? Pagination::LIMIT, $filters->page ?? 1); + $page = $this->apply_filters($query, $filters)->paginate($filters->limit ?? Pagination::LIMIT, $filters->page ?? 1); + + return ListPaginator::from($page, Product::query()->where_not('status', ProductStatus::TRASHED)); } /** diff --git a/app/Services/TagService.php b/app/Services/TagService.php index e7a1d4c6c..192aedef2 100644 --- a/app/Services/TagService.php +++ b/app/Services/TagService.php @@ -6,6 +6,7 @@ use Kirki\Ecommerce\App\Concerns\HasSortableColumns; use Kirki\Ecommerce\App\Models\Tag; +use Kirki\Ecommerce\App\Supports\ListPaginator; use Kirki\Ecommerce\App\Constants\Pagination; use Kirki\Ecommerce\Framework\Collections\Collection; use Kirki\Ecommerce\Framework\Database\Query\Paginator; @@ -58,7 +59,9 @@ protected function sortable_columns() */ public function paginated(ListFilterDTO $filters) { - return $this->list_query($filters)->paginate($filters->limit ?? Pagination::LIMIT, $filters->page ?? 1); + $page = $this->list_query($filters)->paginate($filters->limit ?? Pagination::LIMIT, $filters->page ?? 1); + + return ListPaginator::from($page, Tag::query()); } /** diff --git a/app/Services/VariantService.php b/app/Services/VariantService.php index eb91924f7..bcc7656e0 100644 --- a/app/Services/VariantService.php +++ b/app/Services/VariantService.php @@ -10,6 +10,7 @@ use Kirki\Ecommerce\App\Constants\Product\ProductStatus; use Kirki\Ecommerce\App\DTO\Variant\VariantListFilterDTO; use Kirki\Ecommerce\App\Models\Variant; +use Kirki\Ecommerce\App\Supports\ListPaginator; use Kirki\Ecommerce\App\Constants\Pagination; use Kirki\Ecommerce\Framework\Collections\Collection; use Kirki\Ecommerce\Framework\Database\Query\Paginator; @@ -80,7 +81,9 @@ public function all(VariantListFilterDTO $filters) */ public function paginated(VariantListFilterDTO $filters) { - return $this->list_query($filters)->paginate($filters->limit ?? Pagination::LIMIT, $filters->page ?? 1); + $page = $this->list_query($filters)->paginate($filters->limit ?? Pagination::LIMIT, $filters->page ?? 1); + + return ListPaginator::from($page, Variant::query()); } /** diff --git a/app/Supports/ListPaginator.php b/app/Supports/ListPaginator.php new file mode 100644 index 000000000..ef4c1b7e4 --- /dev/null +++ b/app/Supports/ListPaginator.php @@ -0,0 +1,66 @@ +items(), + $paginator->total(), + $paginator->get_per_page(), + $paginator->get_current_page() + ); + + if ($has_records instanceof QueryBuilder) { + $has_records = $paginator->total() > 0 || $has_records->exists(); + } + + $list->has_records = (bool) $has_records; + + return $list; + } + + /** + * Convert the paginator to an array, with the record presence flag. + * + * @since 1.0.0 + * + * @return array + */ + public function to_array() + { + return array_merge(parent::to_array(), ['has_records' => $this->has_records]); + } +} diff --git a/docs/data-table.md b/docs/data-table.md index 531c4543b..8b0d7eacc 100644 --- a/docs/data-table.md +++ b/docs/data-table.md @@ -28,16 +28,16 @@ must also decide whether to show the table at all ```tsx const Brands = () => { - const { params, hasActiveFilters } = useDataTableParams(brandListOptions); + const { params } = useDataTableParams(brandListOptions); const { data, isFetching } = useBrandsQuery(params); - const hasNoRecord = !isFetching && !hasActiveFilters && data?.total === 0; + const hasRecord = data?.has_records !== false; return ( - } /> + } /> - {hasNoRecord ? ( + {!hasRecord ? ( ) : ( @@ -297,22 +297,23 @@ An empty list is one of two situations, and they are handled in two places: | Situation | Handled by | Shown | Toolbar, headers, pagination | | --- | --- | --- | --- | -| No filters, `total` of `0` (first use) | The **page** | `NotFoundState` instead of the table | Not rendered; the page card holds only the message | +| The table holds no record (`has_records` is `false`) | The **page** | `NotFoundState` instead of the table | Not rendered; the page card holds only the message | | Filters active, or any other case with no rows | `DataTable` | `emptyState` in the table body | Kept, so the criteria can be changed | **First use belongs to the page.** `DataTable` does not know about first use and has no `total`-based branch for it. The page already holds the query and the -filters, so it makes the call and hides the table: +filters, so it makes the call and hides the table. The call reads +`has_records` from the list response: ```tsx -const { params, hasActiveFilters } = useDataTableParams(productListOptions); +const { params } = useDataTableParams(productListOptions); const { data, isFetching } = useProductsQuery(params); -const hasNoRecord = !isFetching && !hasActiveFilters && data?.total === 0; +const hasRecord = data?.has_records !== false; -} /> +} /> - {hasNoRecord ? ( + {!hasRecord ? ( } text={__('Add a product', 'kirki-ecommerce')} @@ -326,8 +327,8 @@ const hasNoRecord = !isFetching && !hasActiveFilters && data?.total === 0; ``` The page's header action is hidden while `NotFoundState` is shown, so the -screen offers one call to action, not two. The page number plays no part: -`total` of `0` with no filters is first use on every page. +screen offers one call to action, not two. Neither the page number nor the +filters play a part: only the server says if the table is empty. **Filtered-empty belongs to the table.** The table passes `emptyState` to `DataTable`, which renders it in a single full-width row of the body, with the @@ -357,10 +358,25 @@ const { ..., clearFilters } = useDataTableParams(productListOptions); `emptyState` is omitted, `DataTable` renders that default. The entity icons used by `NotFoundState` live in `@/icons/empty-state`. -**Wait for `isFetching`.** `hasNoRecord` requires `!isFetching`, so a refetch -never flashes the first-use screen. A failed request leaves `data` undefined, and -`data?.total === 0` is then false, so an error never reads as "add your first -item". +**Only an explicit `false` shows the first-use screen.** While the first request +loads, or after a failed request, `data` is undefined and `has_records` is +undefined. The check `!== false` keeps the table, so an error never reads as "add +your first item", and a refetch never flashes the first-use screen. + +**Where `has_records` comes from.** The nine top-level list endpoints (products, +orders, inventory, collections, categories, tags, brands, customers, coupons) +return `has_records` in the `data` object, beside `total`. It is `true` when the +table holds at least one record, whatever the search or filters in the request. +A search that matches nothing therefore gives `total` of `0` and `has_records` of +`true`, and the page keeps the table and shows the filtered-empty message. + +The service builds it with `ListPaginator::from( $page, $query )`. The query is a +fresh, unfiltered query, for example `Brand::query()`. It runs only when the page +total is `0`; when the page has rows, no extra query runs. To change which +records count, change that query in the service. Products pass +`Product::query()->where_not( 'status', ProductStatus::TRASHED )`, so a store with +only trashed products gets the first-use screen. The `limit=-1` branch (all +records, for pickers) and storefront lists do not return the key. **`hasActiveFilters` and `clearFilters`.** `useDataTableParams` returns both. The flag is true when the search term, a date range or any declared filter holds @@ -391,7 +407,7 @@ page calls the hook and the table calls it again for `clearFilters`. - **Row identity is `row.id`.** `getRowId` returns `String(row.id)`, so selection survives a refetch that reorders rows. Every item must have an `id`. - **An empty result is not one state.** TanStack has no notion of "no rows". - Here first use (no filters, `total` of `0`) replaces the whole table with a + Here first use (the server reports `has_records` as `false`) replaces the whole table with a page-level `NotFoundState`, while a filtered-empty table keeps its toolbar and headers and shows `emptyState` in the body. `DataTable` itself only knows the second case. See [7. Empty states](#7-empty-states). diff --git a/openspec/changes/archive/2026-10-16-list-api-has-record/.openspec.yaml b/openspec/changes/archive/2026-10-16-list-api-has-record/.openspec.yaml new file mode 100644 index 000000000..0cb476561 --- /dev/null +++ b/openspec/changes/archive/2026-10-16-list-api-has-record/.openspec.yaml @@ -0,0 +1,2 @@ +schema: spec-driven +created: 2026-10-16 diff --git a/openspec/changes/archive/2026-10-16-list-api-has-record/design.md b/openspec/changes/archive/2026-10-16-list-api-has-record/design.md new file mode 100644 index 000000000..bef3d982c --- /dev/null +++ b/openspec/changes/archive/2026-10-16-list-api-has-record/design.md @@ -0,0 +1,120 @@ +# Design + +## Context + +Nine services return `->paginate(...)` straight from the query builder. The result is the framework +`Paginator` (`vendor/libraries/framework`, scoped and regenerated by `composer scope`, so it cannot be +edited). `Resource::paginated()` calls `$paginator->to_array()` and maps `results` through the resource. +Any extra key in `to_array()` therefore reaches the response with no resource or controller change. + +The framework `Paginator` keeps its state in `protected` properties and exposes `items()`, `total()`, +`get_per_page()` and `get_current_page()`, so a subclass in `app/` can be built from a finished paginator. + +The nine pages each compute `hasRecord` as `!(!isFetching && !hasActiveFilters && data?.total === 0)` with an +`@todo` to use the server value. `PaginatedData` in `types/api/response.ts` and `PaginatedDataSchema` in +`schemas/shared/api.ts` already declare an optional `has_records` (plural). Nothing reads it. + +Controllers have a second branch for `limit === Pagination::ALL`. It builds a bare `Paginator` from +`all()`. That branch is for pickers and exports, not for list pages. + +See proposal.md for motivation and `specs/list-record-presence` for requirements. + +## Goals / Non-Goals + +**Goals:** + +- One place defines the `has_records` key and its cost rule. +- A service opts in with one line and can change the presence rule with its own query. +- No change to resources, controllers or the existing keys. + +**Non-Goals:** + +- The `limit=ALL` branch, storefront lists (wishlist, account orders view), and settings lists. +- Changing how `DataTable` decides its own first-use row. Pages stop rendering the table when the server + says the table is empty. The inner logic is left as is. + +## Decisions + +### D1. Subclass `Paginator` in `app/Supports/ListPaginator.php` + +`ListPaginator extends Paginator`. It adds a `has_records` property, a static `from()` and a `to_array()` +that returns the parent array plus `has_records`. + +- **Why:** `Resource::paginated()` already calls `to_array()`, so nothing else changes. The service return + type stays `Paginator` (a child class), so no caller breaks. +- **Alternative:** a trait on models. Rejected: the rule depends on the screen, not on the model. +- **Alternative:** add the key in `Resource::paginated()`. Rejected: the resource has no query and the + framework class cannot be edited. An app base resource would run database queries in a formatter. +- **Alternative:** the controller adds the key. Rejected: nine controllers repeat it, and the `ALL` branch + makes it easy to forget one. + +### D2. `from( Paginator $paginator, bool|QueryBuilder $has_records )` + +The second argument is either a ready `bool`, or a query. A query is run with `exists()` and only when +`$paginator->total() === 0`. When `total() > 0` the value is `true` with no lookup. + +- **Why:** one rule, held in one place. A service never repeats the `total() > 0` shortcut. +- **Why `total()` and not `count()`:** `total()` is the filtered total over all pages. A page past the last + page has `count() === 0` but `total() > 0`, and the answer is still "has records". +- **Alternative:** accept a `Closure`. Rejected: nothing needs it yet. A `bool` already covers a caller that + has the answer, and a query covers the rest. + +### D3. The query must be fresh and unfiltered + +Each service passes a new query such as `Brand::query()`, never the filtered list query. A filtered query +would turn "has any record" into "has a match", and a search with no results would show the first-use state. + +- **Note:** a list query is a mutable builder and `paginate()` has already applied filters to it. Reuse is + wrong even if it looks cheaper. +- **Order exception:** the order service also serves the storefront account list through the same method, + scoped by `customer_id`. The query it passes carries that one scope, so the flag is "this customer has + orders" there and "the store has orders" for admin. + +### D4. Default rule is the plain model query; products ignore trashed + +Collections, categories, tags, brands, customers, coupons, variants and orders pass `Model::query()`. +Products pass `Product::query()->where_not('status', ProductStatus::TRASHED)`, the same rule the product +list applies by default. + +- **Why:** a store that holds only trashed products should see "add your first product", like an empty store. +- **Note:** the trash view stays reachable through the product list's status filter once the page shows the + table, but a store with only trashed products shows the first-use state and no filter. Accepted by the user. + +### D5. Name the key `has_records` + +The key is plural, matching the unused `has_records` field that `PaginatedData` and `PaginatedDataSchema` +already declare. The Zod object drops keys it does not list, so any other name would never reach a page. + +- **Alternative:** `has_records` (singular), as in the page `@todo` comments. Rejected: needs a type and + schema rename for no gain. The `@todo` comments are removed in this change. + +### D6. Page logic: show the empty state only on an explicit `false` + +``` +hasRecord = data?.has_records !== false +``` + +`undefined` (loading, error, no data) keeps the table path, so a failed request is never shown as +"add your first item". This also removes the dependency on `hasActiveFilters` and `isFetching` in the +empty-state decision. `hasActiveFilters` stays in use inside `DataTable` for its own filtered-empty message. + +- **Alternative:** keep the old derived guess and use `has_records` only as a tiebreaker. Rejected: two + sources of truth for one decision. + +## Risks / Trade-offs + +- **[Risk] A service forgets to wrap its result** → the key is absent, `data.has_records` is `undefined`, + and the page shows the table, never a false empty state. The failure is safe. Each service gets a test. +- **[Risk] A future caller passes the filtered query** → wrong flag. → The `from()` docblock states the rule, + and the nine call sites show the pattern. +- **[Risk] Extra `exists()` query on an empty list** → one cheap query, only when `total` is 0. +- **[Trade-off] A pure subclass cannot change what the base class stores** → `ListPaginator::from()` copies + the four constructor inputs from getters. If the framework adds a property later, `from()` must copy it too. +- **[Trade-off] Products with only trashed rows** → `has_records: false`, the page shows the first-use state + and the trash cannot be reached from it until a product is created. + +## Migration Plan + +No data or schema change. Ship backend and frontend together. If the frontend ships first, `has_records` is +`undefined`, pages show the table, and the first-use state is lost until the backend lands. If the backend +ships first, the new key is ignored. No rollback step beyond a normal revert. diff --git a/openspec/changes/archive/2026-10-16-list-api-has-record/proposal.md b/openspec/changes/archive/2026-10-16-list-api-has-record/proposal.md new file mode 100644 index 000000000..349129494 --- /dev/null +++ b/openspec/changes/archive/2026-10-16-list-api-has-record/proposal.md @@ -0,0 +1,45 @@ +# Proposal + +## Why + +Each top-level list page decides between its first-use empty state ("add your first item") and its normal +table with this guess: no filters are active and the server total is 0. The guess is marked `@todo` on all nine +pages. It is wrong in one case: while the first request is still loading, or after a request that returns no +total, the page cannot know if the table is truly empty. The server knows. It must say it. + +## What Changes + +- A new `ListPaginator` class extends the framework `Paginator`. It adds one value, `has_records`, to the + paginated payload. `has_records` is `true` when the table holds at least one row, whatever the search or filters. +- `ListPaginator::from( $paginator, $has_records )` converts a normal paginator. The second argument is either + a `bool`, or a fresh unfiltered query. A query is run with `exists()` only when the page total is 0. When + the page has rows, `has_records` is already `true` and no extra query runs. +- The `paginated` method of nine services returns a `ListPaginator`: products, orders, variants (inventory), + collections, categories, tags, brands, customers and coupons. No controller or resource changes. + `Resource::paginated()` calls `to_array()`, so the key reaches the response by itself. +- A service that needs a different rule later (for example skip trashed rows) passes a different query. + This change passes the plain model query for eight and a not-trashed query for products. +- The nine list pages read `data.has_records` and remove the `@todo` guess. The shared TypeScript type and + Zod schema already declare `has_records` and do not change. + +## Capabilities + +### New Capabilities + +- `list-record-presence`: a list endpoint reports whether its table holds any record at all, separate from + the filtered result, and the list page uses that report to choose its empty state. + +### Modified Capabilities + +None. The empty-state presentation in `data-table` does not change. Only the source of the decision changes. + +## Impact + +- New: `app/Supports/ListPaginator.php`. +- `app/Services/{Product,Order,Variant,Collection,Category,Tag,Brand,Customer,Coupon}Service.php` + (`paginated` / `paginated_orders` only). +- The nine pages under `resources/app/features/*/pages/`. +- `docs/data-table.md` (where the empty state gets its input). +- API: one new boolean key in the `data` object of nine list responses. No existing key changes. Not breaking. +- Not in scope: the `limit=ALL` branch in controllers (a hand-built `Paginator`, no empty state), storefront + lists, and settings lists. diff --git a/openspec/changes/archive/2026-10-16-list-api-has-record/specs/list-record-presence/spec.md b/openspec/changes/archive/2026-10-16-list-api-has-record/specs/list-record-presence/spec.md new file mode 100644 index 000000000..340c2eb0a --- /dev/null +++ b/openspec/changes/archive/2026-10-16-list-api-has-record/specs/list-record-presence/spec.md @@ -0,0 +1,115 @@ +# Spec Delta + +## Purpose + +Lets a list page tell an empty table from a search that matched nothing. The list endpoint reports if the +table holds any record at all, independent of the search and filters, and the page uses that report to +choose between its first-use empty state and its filtered table. + +## ADDED Requirements + +### Requirement: A paginated list response states whether the table holds any record + +A paginated response from the product, order, inventory, collection, category, tag, brand, customer and +coupon list endpoints SHALL include a boolean `has_records` in its `data` object. It SHALL be `true` when +the underlying table holds at least one record under the endpoint's presence rule, and `false` otherwise. +The key SHALL sit beside the existing pagination keys, which SHALL NOT change. + +#### Scenario: Table has records + +- **WHEN** a client requests a page of a listed resource and the table holds at least one record +- **THEN** `has_records` is `true` + +#### Scenario: Table is empty + +- **WHEN** a client requests a page of a listed resource and the table holds no record +- **THEN** `has_records` is `false` +- **AND** `total` is 0 and `results` is empty + +#### Scenario: Existing keys are unchanged + +- **WHEN** a client reads a paginated list response +- **THEN** `results`, `total`, `count`, `per_page`, `current_page`, `last_page`, `from`, `to` and + `has_more_pages` keep their meaning and values + +### Requirement: Record presence ignores the request's search and filters + +`has_records` SHALL describe the table, not the request. Search text, status, date range, taxonomy and every +other filter in the request SHALL NOT change its value. + +#### Scenario: Search matches nothing + +- **WHEN** the table holds records and a client searches for text that matches none +- **THEN** `total` is 0 +- **AND** `has_records` is `true` + +#### Scenario: Filter matches nothing + +- **WHEN** the table holds records and a client applies a filter that matches none +- **THEN** `has_records` is `true` + +#### Scenario: Page past the last page + +- **WHEN** the table holds records and a client requests a page beyond the last page +- **THEN** `results` is empty +- **AND** `has_records` is `true` + +### Requirement: Record presence follows a per-resource rule + +Each listed resource SHALL define which records count towards `has_records`. By default every row of the +resource counts. A resource MAY define a narrower rule, such as ignoring trashed records, without any +change to the response shape or to other resources. + +#### Scenario: Default rule + +- **WHEN** a listed resource has no narrower rule +- **THEN** `has_records` is `true` if the table holds any row + +#### Scenario: Narrower rule + +- **WHEN** a resource defines a rule that ignores some records and every record in the table is ignored +- **THEN** `has_records` is `false` + +### Requirement: Record presence costs no extra lookup when the page has rows + +When the returned page holds at least one result, the endpoint SHALL report `has_records` as `true` without +a separate lookup of the table. A separate lookup MAY run only when the page is empty. + +#### Scenario: Page has rows + +- **WHEN** a request returns one or more results +- **THEN** `has_records` is `true` +- **AND** no additional table lookup runs to produce it + +#### Scenario: Page is empty + +- **WHEN** a request returns no results +- **THEN** the endpoint checks the table's record presence to set `has_records` + +### Requirement: List pages choose their empty state from the reported presence + +Each of the nine list pages SHALL present its first-use empty state only when the response reports +`has_records` as `false`. Until a response is available the page SHALL NOT present the first-use empty state. +A response with `has_records` as `true` and no results SHALL show the table with its filtered-empty message. + +#### Scenario: Empty table + +- **WHEN** a list page receives a response with `has_records` as `false` +- **THEN** the page shows its first-use empty state with its create action + +#### Scenario: Search matched nothing + +- **WHEN** a list page receives a response with `total` 0 and `has_records` as `true` +- **THEN** the page shows the table with the filtered-empty message +- **AND** the first-use empty state is not shown + +#### Scenario: Filters cleared on a table with records + +- **WHEN** the user clears all filters on a page whose table holds records +- **THEN** the table shows its rows +- **AND** the first-use empty state is not shown at any point + +#### Scenario: Request has not completed + +- **WHEN** a list page has no response yet, or its request failed +- **THEN** the first-use empty state is not shown diff --git a/openspec/changes/archive/2026-10-16-list-api-has-record/tasks.md b/openspec/changes/archive/2026-10-16-list-api-has-record/tasks.md new file mode 100644 index 000000000..62a43c7aa --- /dev/null +++ b/openspec/changes/archive/2026-10-16-list-api-has-record/tasks.md @@ -0,0 +1,27 @@ +# Tasks + +## 1. ListPaginator + +- [x] 1.1 Add `app/Supports/ListPaginator.php`: extends the framework `Paginator`, holds `has_records`, has a static `from( Paginator $paginator, bool|QueryBuilder $has_records )` that copies items, total, per-page and current page, and a `to_array()` that returns the parent array plus `has_records`. Add full docblocks and the `ABSPATH` guard. Verify with `composer phpcs:docblocks` and `composer phpcs:wporg` on the file. +- [x] 1.2 Add `tests/Unit/Supports/ListPaginatorTest.php`: a `bool` is passed through; a query is not run when `total() > 0`; a query is run when `total() === 0` and its result is used; a page past the last page (empty items, `total() > 0`) gives `true`; the parent keys are unchanged. Verify with `composer test:unit`. + +## 2. Wrap the nine services + +- [x] 2.1 Wrap `paginated` in the collection, category, tag, brand and coupon services with `ListPaginator::from( $page, Model::query() )`. Keep the `@return Paginator` docblock. Verify with `composer phpcs:wporg` and `composer phpcs:docblocks`. +- [x] 2.2 Wrap `paginated` in the customer and variant services the same way (variant uses `Variant::query()`). Verify as in 2.1. +- [x] 2.3 Wrap `ProductService::paginated` with `Product::query()->where_not('status', ProductStatus::TRASHED)`. Leave `paginate_with_variants` unchanged. Verify as in 2.1. +- [x] 2.4 Wrap `OrderService::paginated_orders` with an `Order::query()` that adds `customer_id` only when the DTO has one (D3 order exception). Verify as in 2.1. +- [x] 2.5 Add to the list test of each of the nine endpoints (`BrandApiTest`, `CategoryApiTest`, `CollectionApiTest`, `CouponApiTest`, `CustomerApiTest`, `OrderApiTest`, `ProductApiTest`, `TagApiTest`, `VariantApiTest`): `has_records` is `false` on an empty table; `true` with rows; `true` when a search matches nothing. For products: `false` when only trashed products exist. Verify with `composer test:integration` for those files. + +## 3. Frontend pages + +- [x] 3.1 In the nine pages under `resources/app/features/*/pages/` (products, orders, inventory, collections, categories, tags, brands, customers, coupons), replace the derived `hasRecord` and its `@todo` with `data?.has_records !== false` (D6). Drop `hasActiveFilters` and `isFetching` from the destructuring only where this leaves them unused. Verify with `grep -rn "@todo: later will use" resources/app` returning nothing, then `npm run typecheck && npm test` in `resources/app/`. +- [x] 3.2 Check each page's existing test, if any, and add or update one case per page that `has_records: false` shows the first-use state and `total: 0` with `has_records: true` shows the table. Verify with `npm test` in `resources/app/`. + +## 4. Docs + +- [x] 4.1 Update `docs/data-table.md` section 7 (the "No filters, `total` of `0`" row, the sample page code and the "page number plays no part" paragraph) to say the page reads `has_records` from the list response. Add one short paragraph on the response key and on how a service changes its presence rule. Add a "Where this differs from" line only if the section already has one. Verify by reading the section against the final page code from 3.2. + +## 5. Integration check + +- [x] 5.1 Run `composer test`, `composer phpcs:wporg`, `composer phpcs:docblocks` and `npm run typecheck && npm test` in `resources/app/`. Verify all pass. Visual check of the nine empty states is left to the user (no browser verification in this project). diff --git a/openspec/specs/list-record-presence/spec.md b/openspec/specs/list-record-presence/spec.md new file mode 100644 index 000000000..9f151d930 --- /dev/null +++ b/openspec/specs/list-record-presence/spec.md @@ -0,0 +1,115 @@ +# list-record-presence Specification + +## Purpose + +Lets a list page tell an empty table from a search that matched nothing. The list endpoint reports if the +table holds any record at all, independent of the search and filters, and the page uses that report to +choose between its first-use empty state and its filtered table. + +## Requirements + +### Requirement: A paginated list response states whether the table holds any record + +A paginated response from the product, order, inventory, collection, category, tag, brand, customer and +coupon list endpoints SHALL include a boolean `has_records` in its `data` object. It SHALL be `true` when +the underlying table holds at least one record under the endpoint's presence rule, and `false` otherwise. +The key SHALL sit beside the existing pagination keys, which SHALL NOT change. + +#### Scenario: Table has records + +- **WHEN** a client requests a page of a listed resource and the table holds at least one record +- **THEN** `has_records` is `true` + +#### Scenario: Table is empty + +- **WHEN** a client requests a page of a listed resource and the table holds no record +- **THEN** `has_records` is `false` +- **AND** `total` is 0 and `results` is empty + +#### Scenario: Existing keys are unchanged + +- **WHEN** a client reads a paginated list response +- **THEN** `results`, `total`, `count`, `per_page`, `current_page`, `last_page`, `from`, `to` and + `has_more_pages` keep their meaning and values + +### Requirement: Record presence ignores the request's search and filters + +`has_records` SHALL describe the table, not the request. Search text, status, date range, taxonomy and every +other filter in the request SHALL NOT change its value. + +#### Scenario: Search matches nothing + +- **WHEN** the table holds records and a client searches for text that matches none +- **THEN** `total` is 0 +- **AND** `has_records` is `true` + +#### Scenario: Filter matches nothing + +- **WHEN** the table holds records and a client applies a filter that matches none +- **THEN** `has_records` is `true` + +#### Scenario: Page past the last page + +- **WHEN** the table holds records and a client requests a page beyond the last page +- **THEN** `results` is empty +- **AND** `has_records` is `true` + +### Requirement: Record presence follows a per-resource rule + +Each listed resource SHALL define which records count towards `has_records`. By default every row of the +resource counts. A resource MAY define a narrower rule, such as ignoring trashed records, without any +change to the response shape or to other resources. + +#### Scenario: Default rule + +- **WHEN** a listed resource has no narrower rule +- **THEN** `has_records` is `true` if the table holds any row + +#### Scenario: Narrower rule + +- **WHEN** a resource defines a rule that ignores some records and every record in the table is ignored +- **THEN** `has_records` is `false` + +### Requirement: Record presence costs no extra lookup when the page has rows + +When the returned page holds at least one result, the endpoint SHALL report `has_records` as `true` without +a separate lookup of the table. A separate lookup MAY run only when the page is empty. + +#### Scenario: Page has rows + +- **WHEN** a request returns one or more results +- **THEN** `has_records` is `true` +- **AND** no additional table lookup runs to produce it + +#### Scenario: Page is empty + +- **WHEN** a request returns no results +- **THEN** the endpoint checks the table's record presence to set `has_records` + +### Requirement: List pages choose their empty state from the reported presence + +Each of the nine list pages SHALL present its first-use empty state only when the response reports +`has_records` as `false`. Until a response is available the page SHALL NOT present the first-use empty state. +A response with `has_records` as `true` and no results SHALL show the table with its filtered-empty message. + +#### Scenario: Empty table + +- **WHEN** a list page receives a response with `has_records` as `false` +- **THEN** the page shows its first-use empty state with its create action + +#### Scenario: Search matched nothing + +- **WHEN** a list page receives a response with `total` 0 and `has_records` as `true` +- **THEN** the page shows the table with the filtered-empty message +- **AND** the first-use empty state is not shown + +#### Scenario: Filters cleared on a table with records + +- **WHEN** the user clears all filters on a page whose table holds records +- **THEN** the table shows its rows +- **AND** the first-use empty state is not shown at any point + +#### Scenario: Request has not completed + +- **WHEN** a list page has no response yet, or its request failed +- **THEN** the first-use empty state is not shown diff --git a/resources/app/features/brands/pages/brands.tsx b/resources/app/features/brands/pages/brands.tsx index c1b4d8d9a..9863a52e1 100644 --- a/resources/app/features/brands/pages/brands.tsx +++ b/resources/app/features/brands/pages/brands.tsx @@ -9,11 +9,11 @@ import { BrandEmptyStateIcon } from '@/icons/empty-state'; import { __ } from '@/wpi18n'; const Brands = () => { - const { params, hasActiveFilters } = useDataTableParams(brandListOptions); + const { params } = useDataTableParams(brandListOptions); const { data, isFetching } = useBrandsQuery(params); - const hasRecord = !(!isFetching && !hasActiveFilters && data?.total === 0); // @todo: later will use isDefined(data) && isDefined(data.has_record) && data?.has_record + const hasRecord = data?.has_records !== false; return ( diff --git a/resources/app/features/categories/pages/categories.tsx b/resources/app/features/categories/pages/categories.tsx index 1e63cfd0c..daeb91cb6 100644 --- a/resources/app/features/categories/pages/categories.tsx +++ b/resources/app/features/categories/pages/categories.tsx @@ -9,11 +9,11 @@ import { CategoryEmptyStateIcon } from '@/icons/empty-state'; import { __ } from '@/wpi18n'; const Categories = () => { - const { params, hasActiveFilters } = useDataTableParams(categoryListOptions); + const { params } = useDataTableParams(categoryListOptions); const { data, isFetching } = useCategoriesQuery(params); - const hasRecord = !(!isFetching && !hasActiveFilters && data?.total === 0); // @todo: later will use isDefined(data) && isDefined(data.has_record) && data?.has_record + const hasRecord = data?.has_records !== false; return ( diff --git a/resources/app/features/collections/pages/collections.tsx b/resources/app/features/collections/pages/collections.tsx index ac6db35c0..11c49bb2a 100644 --- a/resources/app/features/collections/pages/collections.tsx +++ b/resources/app/features/collections/pages/collections.tsx @@ -15,11 +15,11 @@ import { __ } from '@/wpi18n'; const Collections = () => { const navigate = useNavigate(); - const { params, hasActiveFilters } = useDataTableParams(collectionListOptions); + const { params } = useDataTableParams(collectionListOptions); const { data, isFetching } = useCollectionsQuery(params); - const hasRecord = !(!isFetching && !hasActiveFilters && data?.total === 0); // @todo: later will use isDefined(data) && isDefined(data.has_record) && data?.has_record + const hasRecord = data?.has_records !== false; return ( diff --git a/resources/app/features/coupons/pages/coupons.tsx b/resources/app/features/coupons/pages/coupons.tsx index 4d63fa9f2..26f19186c 100644 --- a/resources/app/features/coupons/pages/coupons.tsx +++ b/resources/app/features/coupons/pages/coupons.tsx @@ -15,11 +15,11 @@ import { __ } from '@/wpi18n'; const Coupons = () => { const navigate = useNavigate(); - const { params, hasActiveFilters } = useDataTableParams(couponListOptions); + const { params } = useDataTableParams(couponListOptions); const { data, isFetching } = useCouponsQuery(params); - const hasRecord = !(!isFetching && !hasActiveFilters && data?.total === 0); // @todo: later will use isDefined(data) && isDefined(data.has_record) && data?.has_record + const hasRecord = data?.has_records !== false; return ( diff --git a/resources/app/features/customers/pages/customers.tsx b/resources/app/features/customers/pages/customers.tsx index ad9dec564..703cc5428 100644 --- a/resources/app/features/customers/pages/customers.tsx +++ b/resources/app/features/customers/pages/customers.tsx @@ -15,11 +15,11 @@ import { __ } from '@/wpi18n'; const Customers = () => { const navigate = useNavigate(); - const { params, hasActiveFilters } = useDataTableParams(customerListOptions); + const { params } = useDataTableParams(customerListOptions); const { data, isFetching } = useCustomersQuery(params); - const hasRecord = !(!isFetching && !hasActiveFilters && data?.total === 0); // @todo: later will use isDefined(data) && isDefined(data.has_record) && data?.has_record + const hasRecord = data?.has_records !== false; const handleAddNewCustomer = () => { void navigate(RouteConfig.Customers.get('CustomerDetail').buildLink({ id: NEW_ITEM_ID })); diff --git a/resources/app/features/inventory/pages/inventory.tsx b/resources/app/features/inventory/pages/inventory.tsx index 4cde3df19..f43a8240e 100644 --- a/resources/app/features/inventory/pages/inventory.tsx +++ b/resources/app/features/inventory/pages/inventory.tsx @@ -13,11 +13,11 @@ import { __ } from '@/wpi18n'; const Inventory = () => { const navigate = useNavigate(); - const { params, hasActiveFilters } = useDataTableParams(inventoryListOptions); + const { params } = useDataTableParams(inventoryListOptions); const { data, isFetching } = useInventoryQuery(params); - const hasRecord = !(!isFetching && !hasActiveFilters && data?.total === 0); // @todo: later will use isDefined(data) && isDefined(data.has_record) && data?.has_record + const hasRecord = data?.has_records !== false; return ( diff --git a/resources/app/features/list-page-empty-state.test.tsx b/resources/app/features/list-page-empty-state.test.tsx new file mode 100644 index 000000000..21f1be18b --- /dev/null +++ b/resources/app/features/list-page-empty-state.test.tsx @@ -0,0 +1,86 @@ +import { QueryClient, QueryClientProvider } from '@tanstack/react-query'; +import { cleanup, render, screen } from '@testing-library/react'; +import type { ComponentType } from 'react'; +import { createMemoryRouter, RouterProvider } from 'react-router'; +import { afterEach, describe, expect, it, vi } from 'vitest'; + +const { state, withQuery, table } = vi.hoisted(() => { + const hoisted = { data: undefined as unknown }; + + return { + state: hoisted, + withQuery: (hook: string) => async (importOriginal: () => Promise) => ({ + ...(await importOriginal()), + [hook]: () => ({ data: hoisted.data, isFetching: false }), + }), + table: () => ({ default: () => 'TABLE_STUB' }), + }; +}); + +vi.mock('@/features/products/services/product', withQuery('useProductsQuery')); +vi.mock('@/features/orders/services/order', withQuery('useOrdersQuery')); +vi.mock('@/features/inventory/services/inventory', withQuery('useInventoryQuery')); +vi.mock('@/features/collections/services/collection', withQuery('useCollectionsQuery')); +vi.mock('@/features/categories/services/category', withQuery('useCategoriesQuery')); +vi.mock('@/features/tags/services/tag', withQuery('useTagsQuery')); +vi.mock('@/features/brands/services/brand', withQuery('useBrandsQuery')); +vi.mock('@/features/customers/services/customer', withQuery('useCustomersQuery')); +vi.mock('@/features/coupons/services/coupon', withQuery('useCouponsQuery')); + +vi.mock('@/features/products/components/product-table/product-table', table); +vi.mock('@/features/orders/components/order-table/order-table', table); +vi.mock('@/features/inventory/components/inventory-table/inventory-table', table); +vi.mock('@/features/collections/components/collection-table/collection-table', table); +vi.mock('@/features/categories/components/category-table/category-table', table); +vi.mock('@/features/tags/components/tag-table/tag-table', table); +vi.mock('@/features/brands/components/brand-table/brand-table', table); +vi.mock('@/features/customers/components/customer-table/customer-table', table); +vi.mock('@/features/coupons/components/coupon-table/coupon-table', table); + +const pages: Record Promise<{ default: ComponentType }>> = { + products: () => import('@/features/products/pages/products'), + orders: () => import('@/features/orders/pages/orders'), + inventory: () => import('@/features/inventory/pages/inventory'), + collections: () => import('@/features/collections/pages/collections'), + categories: () => import('@/features/categories/pages/categories'), + tags: () => import('@/features/tags/pages/tags'), + brands: () => import('@/features/brands/pages/brands'), + customers: () => import('@/features/customers/pages/customers'), + coupons: () => import('@/features/coupons/pages/coupons'), +}; + +const renderPage = async (name: string, data: unknown) => { + state.data = data; + const { default: Page } = await pages[name](); + const router = createMemoryRouter([{ path: '/', element: }]); + + return render( + + + , + ); +}; + +const results = { results: [], per_page: 10 }; + +afterEach(cleanup); + +describe.each(Object.keys(pages))('%s list page empty state', (name) => { + it('shows the first-use state when the table holds no record', async () => { + await renderPage(name, { ...results, total: 0, has_records: false }); + + expect(screen.queryByText('TABLE_STUB')).not.toBeInTheDocument(); + }); + + it('keeps the table when a search matched nothing but records exist', async () => { + await renderPage(name, { ...results, total: 0, has_records: true }); + + expect(screen.getByText('TABLE_STUB')).toBeInTheDocument(); + }); + + it('keeps the table while no response is available', async () => { + await renderPage(name, undefined); + + expect(screen.getByText('TABLE_STUB')).toBeInTheDocument(); + }); +}); diff --git a/resources/app/features/orders/pages/orders.tsx b/resources/app/features/orders/pages/orders.tsx index a70edc2dd..a57d45a08 100644 --- a/resources/app/features/orders/pages/orders.tsx +++ b/resources/app/features/orders/pages/orders.tsx @@ -13,11 +13,11 @@ import { __ } from '@/wpi18n'; const Orders = () => { const navigate = useNavigate(); - const { params, hasActiveFilters } = useDataTableParams(orderListOptions); + const { params } = useDataTableParams(orderListOptions); const { data, isFetching } = useOrdersQuery(params); - const hasRecord = !(!isFetching && !hasActiveFilters && data?.total === 0); // @todo: later will use isDefined(data) && isDefined(data.has_record) && data?.has_record + const hasRecord = data?.has_records !== false; return ( diff --git a/resources/app/features/products/pages/products.tsx b/resources/app/features/products/pages/products.tsx index 09a106d5b..725b8cffa 100644 --- a/resources/app/features/products/pages/products.tsx +++ b/resources/app/features/products/pages/products.tsx @@ -13,11 +13,11 @@ import { __ } from '@/wpi18n'; const Products = () => { const navigate = useNavigate(); - const { params, hasActiveFilters } = useDataTableParams(productListOptions); + const { params } = useDataTableParams(productListOptions); const { data, isFetching } = useProductsQuery(params); - const hasRecord = !(!isFetching && !hasActiveFilters && data?.total === 0); // @todo: later will use isDefined(data) && isDefined(data.has_record) && data?.has_record + const hasRecord = data?.has_records !== false; return ( diff --git a/resources/app/features/tags/pages/tags.tsx b/resources/app/features/tags/pages/tags.tsx index dc8e5b979..fc658ed56 100644 --- a/resources/app/features/tags/pages/tags.tsx +++ b/resources/app/features/tags/pages/tags.tsx @@ -9,11 +9,11 @@ import { TagEmptyStateIcon } from '@/icons/empty-state'; import { __ } from '@/wpi18n'; const Tags = () => { - const { params, hasActiveFilters } = useDataTableParams(tagListOptions); + const { params } = useDataTableParams(tagListOptions); const { data, isFetching } = useTagsQuery(params); - const hasRecord = !(!isFetching && !hasActiveFilters && data?.total === 0); // @todo: later will use isDefined(data) && isDefined(data.has_record) && data?.has_record + const hasRecord = data?.has_records !== false; return ( diff --git a/tests/Integration/BrandApiTest.php b/tests/Integration/BrandApiTest.php index 0c62ea27e..1dc9934cf 100644 --- a/tests/Integration/BrandApiTest.php +++ b/tests/Integration/BrandApiTest.php @@ -4,10 +4,14 @@ use Kirki\Ecommerce\App\Constants\BulkActions; use Kirki\Ecommerce\Tests\Support\CreatesTestProducts; +use Kirki\Ecommerce\App\Models\Brand; +use Kirki\Ecommerce\Tests\Support\ListsHaveRecords; use Kirki\Ecommerce\Tests\Support\RestTestCase; class BrandApiTest extends RestTestCase { + use ListsHaveRecords; + use CreatesTestProducts; /** @@ -333,4 +337,38 @@ protected function create_brand(array $overrides = []): array return $payload['data']; } + + /** + * List brands reports whether the table holds any record. + * + * @return void + * @since 1.0.0 + */ + public function test_list_brands_reports_has_records(): void + { + Brand::query()->delete(); + + $this->assertFalse($this->list_has_records('brands')); + + $this->create_brand(['name' => 'Has Records Brand', 'slug' => 'has-records-brand-' . wp_generate_password(6, false)]); + + $this->assertTrue($this->list_has_records('brands')); + } + + /** + * List brands keeps has_records true when a search matches nothing. + * + * @return void + * @since 1.0.0 + */ + public function test_list_brands_has_records_ignores_search(): void + { + $this->create_brand(['name' => 'Has Records Brand', 'slug' => 'has-records-brand-' . wp_generate_password(6, false)]); + + $response = $this->request('GET', 'brands', ['search' => 'no-such-record-zzz']); + $payload = $this->assert_api_success($response); + + $this->assertSame(0, $payload['data']['total']); + $this->assertTrue($payload['data']['has_records']); + } } diff --git a/tests/Integration/CategoryApiTest.php b/tests/Integration/CategoryApiTest.php index 595c37179..3145789dc 100644 --- a/tests/Integration/CategoryApiTest.php +++ b/tests/Integration/CategoryApiTest.php @@ -3,10 +3,14 @@ namespace Kirki\Ecommerce\Tests\Integration; use Kirki\Ecommerce\App\Constants\BulkActions; +use Kirki\Ecommerce\App\Models\Category; +use Kirki\Ecommerce\Tests\Support\ListsHaveRecords; use Kirki\Ecommerce\Tests\Support\RestTestCase; class CategoryApiTest extends RestTestCase { + use ListsHaveRecords; + /** * Category id for the current test. * @@ -205,4 +209,38 @@ protected function create_category(array $overrides = []): array return $payload['data']; } + + /** + * List categories reports whether the table holds any record. + * + * @return void + * @since 1.0.0 + */ + public function test_list_categories_reports_has_records(): void + { + Category::query()->delete(); + + $this->assertFalse($this->list_has_records('categories')); + + $this->create_category(['name' => 'Has Records Category', 'slug' => 'has-records-category-' . wp_generate_password(6, false)]); + + $this->assertTrue($this->list_has_records('categories')); + } + + /** + * List categories keeps has_records true when a search matches nothing. + * + * @return void + * @since 1.0.0 + */ + public function test_list_categories_has_records_ignores_search(): void + { + $this->create_category(['name' => 'Has Records Category', 'slug' => 'has-records-category-' . wp_generate_password(6, false)]); + + $response = $this->request('GET', 'categories', ['search' => 'no-such-record-zzz']); + $payload = $this->assert_api_success($response); + + $this->assertSame(0, $payload['data']['total']); + $this->assertTrue($payload['data']['has_records']); + } } diff --git a/tests/Integration/CollectionApiTest.php b/tests/Integration/CollectionApiTest.php index 7d6d9b616..c81ee0c60 100644 --- a/tests/Integration/CollectionApiTest.php +++ b/tests/Integration/CollectionApiTest.php @@ -3,10 +3,14 @@ namespace Kirki\Ecommerce\Tests\Integration; use Kirki\Ecommerce\App\Constants\BulkActions; +use Kirki\Ecommerce\App\Models\Collection; +use Kirki\Ecommerce\Tests\Support\ListsHaveRecords; use Kirki\Ecommerce\Tests\Support\RestTestCase; class CollectionApiTest extends RestTestCase { + use ListsHaveRecords; + /** * Collection id for the current test. * @@ -205,4 +209,38 @@ protected function create_collection(array $overrides = []): array return $payload['data']; } + + /** + * List collections reports whether the table holds any record. + * + * @return void + * @since 1.0.0 + */ + public function test_list_collections_reports_has_records(): void + { + Collection::query()->delete(); + + $this->assertFalse($this->list_has_records('collections')); + + $this->create_collection(['title' => 'Has Records Collection', 'slug' => 'has-records-collection-' . wp_generate_password(6, false)]); + + $this->assertTrue($this->list_has_records('collections')); + } + + /** + * List collections keeps has_records true when a search matches nothing. + * + * @return void + * @since 1.0.0 + */ + public function test_list_collections_has_records_ignores_search(): void + { + $this->create_collection(['title' => 'Has Records Collection', 'slug' => 'has-records-collection-' . wp_generate_password(6, false)]); + + $response = $this->request('GET', 'collections', ['search' => 'no-such-record-zzz']); + $payload = $this->assert_api_success($response); + + $this->assertSame(0, $payload['data']['total']); + $this->assertTrue($payload['data']['has_records']); + } } diff --git a/tests/Integration/CouponApiTest.php b/tests/Integration/CouponApiTest.php index c0a04d81c..85a76e146 100644 --- a/tests/Integration/CouponApiTest.php +++ b/tests/Integration/CouponApiTest.php @@ -13,10 +13,13 @@ use Kirki\Ecommerce\App\Constants\Coupon\EligibleItemType; use Kirki\Ecommerce\App\Models\Coupon; use Kirki\Ecommerce\Tests\Support\CreatesTestProducts; +use Kirki\Ecommerce\Tests\Support\ListsHaveRecords; use Kirki\Ecommerce\Tests\Support\RestTestCase; class CouponApiTest extends RestTestCase { + use ListsHaveRecords; + use CreatesTestProducts; /** @@ -633,4 +636,38 @@ protected function assert_status_filter_lists(string $status, int $expected_id, $this->assertNotContains($excluded_id, $ids, 'Unexpected coupon in the ' . $status . ' list.'); } } + + /** + * List coupons reports whether the table holds any record. + * + * @return void + * @since 1.0.0 + */ + public function test_list_coupons_reports_has_records(): void + { + Coupon::query()->delete(); + + $this->assertFalse($this->list_has_records('coupons')); + + $this->create_coupon(['title' => 'Has Records Coupon']); + + $this->assertTrue($this->list_has_records('coupons')); + } + + /** + * List coupons keeps has_records true when a search matches nothing. + * + * @return void + * @since 1.0.0 + */ + public function test_list_coupons_has_records_ignores_search(): void + { + $this->create_coupon(['title' => 'Has Records Coupon']); + + $response = $this->request('GET', 'coupons', ['search' => 'no-such-record-zzz']); + $payload = $this->assert_api_success($response); + + $this->assertSame(0, $payload['data']['total']); + $this->assertTrue($payload['data']['has_records']); + } } diff --git a/tests/Integration/CustomerApiTest.php b/tests/Integration/CustomerApiTest.php index db22588bd..06a6fefe7 100644 --- a/tests/Integration/CustomerApiTest.php +++ b/tests/Integration/CustomerApiTest.php @@ -4,11 +4,15 @@ use Kirki\Ecommerce\App\Constants\BulkActions; use Kirki\Ecommerce\App\Supports\CountryData; +use Kirki\Ecommerce\App\Models\Customer; +use Kirki\Ecommerce\Tests\Support\ListsHaveRecords; use Kirki\Ecommerce\Tests\Support\RestTestCase; use Kirki\Ecommerce\Tests\Support\SeedsTestCurrency; class CustomerApiTest extends RestTestCase { + use ListsHaveRecords; + use SeedsTestCurrency; /** @@ -855,4 +859,38 @@ protected function listed_customer_ids(array $params = []): array return array_map('intval', array_column($payload['data']['results'], 'id')); } + + /** + * List customers reports whether the table holds any record. + * + * @return void + * @since 1.0.0 + */ + public function test_list_customers_reports_has_records(): void + { + Customer::query()->delete(); + + $this->assertFalse($this->list_has_records('customers')); + + $this->create_customer(['first_name' => 'HasRecords']); + + $this->assertTrue($this->list_has_records('customers')); + } + + /** + * List customers keeps has_records true when a search matches nothing. + * + * @return void + * @since 1.0.0 + */ + public function test_list_customers_has_records_ignores_search(): void + { + $this->create_customer(['first_name' => 'HasRecords']); + + $response = $this->request('GET', 'customers', ['search' => 'no-such-record-zzz']); + $payload = $this->assert_api_success($response); + + $this->assertSame(0, $payload['data']['total']); + $this->assertTrue($payload['data']['has_records']); + } } diff --git a/tests/Integration/OrderApiTest.php b/tests/Integration/OrderApiTest.php index a9443e162..5489b7fa5 100644 --- a/tests/Integration/OrderApiTest.php +++ b/tests/Integration/OrderApiTest.php @@ -45,6 +45,7 @@ use Kirki\Ecommerce\App\Supports\Facades\Settings; use Kirki\Ecommerce\Tests\Support\CreatesTestProducts; use Kirki\Ecommerce\Tests\Support\EnablesPaymentProviders; +use Kirki\Ecommerce\Tests\Support\ListsHaveRecords; use Kirki\Ecommerce\Tests\Support\RestTestCase; use Kirki\Ecommerce\Tests\Support\SeedsTestShipping; use Exception; @@ -54,6 +55,8 @@ class OrderApiTest extends RestTestCase { + use ListsHaveRecords; + use CreatesTestProducts; use EnablesPaymentProviders; use SeedsTestShipping; @@ -3352,4 +3355,38 @@ public function test_signup_only_consent_does_not_block_storefront_order(): void $this->assert_api_success($response, 201); } + + /** + * List orders reports whether the table holds any record. + * + * @return void + * @since 1.0.0 + */ + public function test_list_orders_reports_has_records(): void + { + Order::query()->delete(); + + $this->assertFalse($this->list_has_records('orders')); + + $this->create_order(); + + $this->assertTrue($this->list_has_records('orders')); + } + + /** + * List orders keeps has_records true when a search matches nothing. + * + * @return void + * @since 1.0.0 + */ + public function test_list_orders_has_records_ignores_search(): void + { + $this->create_order(); + + $response = $this->request('GET', 'orders', ['search' => 'no-such-record-zzz']); + $payload = $this->assert_api_success($response); + + $this->assertSame(0, $payload['data']['total']); + $this->assertTrue($payload['data']['has_records']); + } } diff --git a/tests/Integration/ProductApiTest.php b/tests/Integration/ProductApiTest.php index 9ed2c74d3..5626e5287 100644 --- a/tests/Integration/ProductApiTest.php +++ b/tests/Integration/ProductApiTest.php @@ -7,10 +7,14 @@ use Kirki\Ecommerce\App\Constants\Product\ProductStatus; use Kirki\Ecommerce\App\Models\AttributeValue; use Kirki\Ecommerce\Tests\Support\CreatesTestProducts; +use Kirki\Ecommerce\App\Models\Product; +use Kirki\Ecommerce\Tests\Support\ListsHaveRecords; use Kirki\Ecommerce\Tests\Support\RestTestCase; class ProductApiTest extends RestTestCase { + use ListsHaveRecords; + use CreatesTestProducts; /** @@ -1131,4 +1135,58 @@ public function test_list_products_filters_by_out_of_stock_availability(): void $payload = $this->assert_api_success($response); $this->assertEquals([$unique . ' OutOfStock'], $this->listed_titles($payload)); } + + /** + * List products reports whether the table holds any record. + * + * @return void + * @since 1.0.0 + */ + public function test_list_products_reports_has_records(): void + { + Product::query()->delete(); + + $this->assertFalse($this->list_has_records('products')); + + $this->create_product(['title' => 'Has Records Product']); + + $this->assertTrue($this->list_has_records('products')); + } + + /** + * List products keeps has_records true when a search matches nothing. + * + * @return void + * @since 1.0.0 + */ + public function test_list_products_has_records_ignores_search(): void + { + $this->create_product(['title' => 'Has Records Product']); + + $response = $this->request('GET', 'products', ['search' => 'no-such-record-zzz']); + $payload = $this->assert_api_success($response); + + $this->assertSame(0, $payload['data']['total']); + $this->assertTrue($payload['data']['has_records']); + } + + /** + * Products report no record when only trashed products exist. + * + * @return void + * @since 1.0.0 + */ + public function test_list_products_has_records_ignores_trashed(): void + { + Product::query()->delete(); + + $product = $this->create_product(['title' => 'Has Records Trashed']); + + $this->request('POST', 'products/bulk', [ + 'action' => BulkActions::TRASH, + 'ids' => [$product['id']], + ]); + + $this->assertFalse($this->list_has_records('products')); + } } diff --git a/tests/Integration/TagApiTest.php b/tests/Integration/TagApiTest.php index 3cc6e6dc9..b09c83a28 100644 --- a/tests/Integration/TagApiTest.php +++ b/tests/Integration/TagApiTest.php @@ -3,10 +3,14 @@ namespace Kirki\Ecommerce\Tests\Integration; use Kirki\Ecommerce\App\Constants\BulkActions; +use Kirki\Ecommerce\App\Models\Tag; +use Kirki\Ecommerce\Tests\Support\ListsHaveRecords; use Kirki\Ecommerce\Tests\Support\RestTestCase; class TagApiTest extends RestTestCase { + use ListsHaveRecords; + /** * Tag id for the current test. * @@ -201,4 +205,38 @@ protected function create_tag(array $overrides = []): array return $payload['data']; } + + /** + * List tags reports whether the table holds any record. + * + * @return void + * @since 1.0.0 + */ + public function test_list_tags_reports_has_records(): void + { + Tag::query()->delete(); + + $this->assertFalse($this->list_has_records('tags')); + + $this->create_tag(['name' => 'Has Records Tag', 'slug' => 'has-records-tag-' . wp_generate_password(6, false)]); + + $this->assertTrue($this->list_has_records('tags')); + } + + /** + * List tags keeps has_records true when a search matches nothing. + * + * @return void + * @since 1.0.0 + */ + public function test_list_tags_has_records_ignores_search(): void + { + $this->create_tag(['name' => 'Has Records Tag', 'slug' => 'has-records-tag-' . wp_generate_password(6, false)]); + + $response = $this->request('GET', 'tags', ['search' => 'no-such-record-zzz']); + $payload = $this->assert_api_success($response); + + $this->assertSame(0, $payload['data']['total']); + $this->assertTrue($payload['data']['has_records']); + } } diff --git a/tests/Integration/VariantApiTest.php b/tests/Integration/VariantApiTest.php index 8cbc91f6b..fbbd0523d 100644 --- a/tests/Integration/VariantApiTest.php +++ b/tests/Integration/VariantApiTest.php @@ -4,10 +4,14 @@ use Kirki\Ecommerce\App\Constants\InventoryType; use Kirki\Ecommerce\Tests\Support\CreatesTestProducts; +use Kirki\Ecommerce\App\Models\Variant; +use Kirki\Ecommerce\Tests\Support\ListsHaveRecords; use Kirki\Ecommerce\Tests\Support\RestTestCase; class VariantApiTest extends RestTestCase { + use ListsHaveRecords; + use CreatesTestProducts; /** @@ -492,4 +496,38 @@ protected function variant_payload(string $sku, int $quantity = 100, bool $in_st 'attribute_values' => [], ]; } + + /** + * List variants reports whether the table holds any record. + * + * @return void + * @since 1.0.0 + */ + public function test_list_variants_reports_has_records(): void + { + Variant::query()->delete(); + + $this->assertFalse($this->list_has_records('variants')); + + $this->create_product(); + + $this->assertTrue($this->list_has_records('variants')); + } + + /** + * List variants keeps has_records true when a search matches nothing. + * + * @return void + * @since 1.0.0 + */ + public function test_list_variants_has_records_ignores_search(): void + { + $this->create_product(); + + $response = $this->request('GET', 'variants', ['search' => 'no-such-record-zzz']); + $payload = $this->assert_api_success($response); + + $this->assertSame(0, $payload['data']['total']); + $this->assertTrue($payload['data']['has_records']); + } } diff --git a/tests/Support/ListsHaveRecords.php b/tests/Support/ListsHaveRecords.php new file mode 100644 index 000000000..32073fb21 --- /dev/null +++ b/tests/Support/ListsHaveRecords.php @@ -0,0 +1,30 @@ + $params Extra query parameters. + * @return bool + */ + protected function list_has_records(string $route, array $params = []): bool + { + $response = $this->request('GET', $route, array_merge(['page' => 1, 'limit' => 10], $params)); + $payload = $this->assert_api_success($response); + + $this->assertArrayHasKey('has_records', $payload['data']); + + return $payload['data']['has_records']; + } +} diff --git a/tests/Unit/Supports/ListPaginatorTest.php b/tests/Unit/Supports/ListPaginatorTest.php new file mode 100644 index 000000000..d7e555f72 --- /dev/null +++ b/tests/Unit/Supports/ListPaginatorTest.php @@ -0,0 +1,66 @@ +assertTrue(ListPaginator::from($page, true)->to_array()['has_records']); + $this->assertFalse(ListPaginator::from($page, false)->to_array()['has_records']); + } + + public function test_query_is_not_run_when_page_has_rows(): void + { + $query = $this->createMock(QueryBuilder::class); + $query->expects($this->never())->method('exists'); + + $page = new Paginator(new Collection([1, 2]), 2, 10, 1); + + $this->assertTrue(ListPaginator::from($page, $query)->to_array()['has_records']); + } + + public function test_query_result_is_used_when_total_is_zero(): void + { + $page = new Paginator(new Collection([]), 0, 10, 1); + + $present = $this->createMock(QueryBuilder::class); + $present->expects($this->once())->method('exists')->willReturn(true); + + $absent = $this->createMock(QueryBuilder::class); + $absent->expects($this->once())->method('exists')->willReturn(false); + + $this->assertTrue(ListPaginator::from($page, $present)->to_array()['has_records']); + $this->assertFalse(ListPaginator::from($page, $absent)->to_array()['has_records']); + } + + public function test_page_past_the_last_page_has_records(): void + { + $query = $this->createMock(QueryBuilder::class); + $query->expects($this->never())->method('exists'); + + $page = new Paginator(new Collection([]), 5, 10, 3); + + $this->assertTrue(ListPaginator::from($page, $query)->to_array()['has_records']); + } + + public function test_parent_keys_are_unchanged(): void + { + $page = new Paginator(new Collection([1, 2]), 12, 2, 1); + + $array = ListPaginator::from($page, true)->to_array(); + $expected = $page->to_array(); + + $this->assertSame($expected, array_diff_key($array, ['has_records' => true])); + $this->assertSame(12, $array['total']); + $this->assertSame(6, $array['last_page']); + } +} From 8406ae707003d98b6222aab568a09eb330a978e2 Mon Sep 17 00:00:00 2001 From: "Md. Ashraful" Date: Fri, 16 Oct 2026 16:33:44 +0600 Subject: [PATCH 2/3] Fix dashboard net sales taking shipping tax off twice An order's tax total already holds its shipping tax, and its shipping total is gross of that tax. Net sales subtracted both in full, so the shipping tax was removed twice. Subtract the shipping total without its tax instead. - Sum base_shipping_tax_amount in the dashboard totals query. - Add an integration test for an order with shipping tax. - Update the net sales formula in docs/home.md and the merchant-home-dashboard spec. --- app/Services/DashboardService.php | 5 +++-- docs/home.md | 2 +- .../specs/merchant-home-dashboard/spec.md | 10 +++++++-- tests/Integration/DashboardApiTest.php | 21 +++++++++++++++++++ 4 files changed, 33 insertions(+), 5 deletions(-) diff --git a/app/Services/DashboardService.php b/app/Services/DashboardService.php index 4539045c0..d830b1e4a 100644 --- a/app/Services/DashboardService.php +++ b/app/Services/DashboardService.php @@ -72,7 +72,8 @@ protected function get_metrics() 'COUNT(*) AS orders_count, ' . 'COALESCE(SUM(base_total), 0) AS base_total, ' . 'COALESCE(SUM(base_tax_total), 0) AS base_tax_total, ' - . 'COALESCE(SUM(base_shipping_total), 0) AS base_shipping_total' + . 'COALESCE(SUM(base_shipping_total), 0) AS base_shipping_total, ' + . 'COALESCE(SUM(base_shipping_tax_amount), 0) AS base_shipping_tax_amount' ) ->first(); @@ -84,7 +85,7 @@ protected function get_metrics() $base_total_sales = (int) ($totals->base_total ?? 0); $base_net_sales = $base_total_sales - (int) ($totals->base_tax_total ?? 0) - - (int) ($totals->base_shipping_total ?? 0) + - ((int) ($totals->base_shipping_total ?? 0) - (int) ($totals->base_shipping_tax_amount ?? 0)) - $this->get_base_refunded_total(); return [ diff --git a/docs/home.md b/docs/home.md index 24fe14d41..9039256e8 100644 --- a/docs/home.md +++ b/docs/home.md @@ -175,7 +175,7 @@ processing and cancelled payments are never counted. All figures are **all time* | Card | Value | |---|---| | Total sales | Sum of `base_total` of counted orders. | -| Net sales | Total sales − sum of `base_tax_total` − sum of `base_shipping_total` − completed refunds of counted orders, in the base currency. | +| Net sales | Total sales − sum of `base_tax_total` − (sum of `base_shipping_total` − sum of `base_shipping_tax_amount`) − completed refunds of counted orders, in the base currency. `base_tax_total` and `base_shipping_total` both hold the shipping tax, so it is taken off once. | | Orders | Number of counted orders. | | Products sold | Sum of order item `quantity` of counted orders. | diff --git a/openspec/specs/merchant-home-dashboard/spec.md b/openspec/specs/merchant-home-dashboard/spec.md index 9bd850a69..449e3aa21 100644 --- a/openspec/specs/merchant-home-dashboard/spec.md +++ b/openspec/specs/merchant-home-dashboard/spec.md @@ -103,8 +103,9 @@ The overview SHALL show four metric cards, in this order: 1. **Total sales**: the sum of the base-currency total of all counted orders. 2. **Net sales**: Total sales minus the base-currency tax total, minus the - base-currency shipping total, minus the base-currency amount of completed - refunds on counted orders. + base-currency shipping total without its tax, minus the base-currency amount + of completed refunds on counted orders. The tax total already holds the + shipping tax, so the shipping tax is not taken off a second time. 3. **Orders**: the number of counted orders. 4. **Products sold**: the sum of item quantities on counted orders. @@ -118,6 +119,11 @@ or a trend. - **WHEN** the only counted order has base total 120.00, tax 10.00 and shipping 10.00, with no refunds - **THEN** Total sales shows 120.00, Net sales shows 100.00, Orders shows 1 and Products sold shows the item quantity of that order +#### Scenario: Shipping with tax + +- **WHEN** the only counted order has base total 120.00, tax 10.00 of which 2.00 is shipping tax, and shipping 10.00 that includes the 2.00 tax +- **THEN** Net sales shows 102.00 + #### Scenario: Partial refund - **WHEN** that order then gets a completed refund of 20.00 in the base currency diff --git a/tests/Integration/DashboardApiTest.php b/tests/Integration/DashboardApiTest.php index 9e6e7716d..940383e21 100644 --- a/tests/Integration/DashboardApiTest.php +++ b/tests/Integration/DashboardApiTest.php @@ -105,6 +105,27 @@ public function test_net_sales_removes_tax_shipping_and_completed_refunds(): voi $this->assertSame(2, $metrics['products_sold']); } + /** + * Net sales takes the shipping tax off once, because the tax total and the shipping total both hold it. + * + * @return void + * @since 1.0.0 + */ + public function test_net_sales_does_not_remove_shipping_tax_twice(): void + { + $this->create_order([ + 'base_total' => 12000, + 'invoiced_total' => 12000, + 'base_tax_total' => 1000, + 'base_shipping_total' => 1000, + 'base_shipping_tax_amount' => 200, + ], [['quantity' => 1]]); + + $metrics = $this->get_overview()['metrics']; + + $this->assertSame(10200, $this->minor($metrics['base_net_sales_money_object'])); + } + /** * A refund in another currency is converted with its own order's base-to-invoiced ratio. * From 898f7493be5eab3568080f4d3b1bab00683ce010 Mon Sep 17 00:00:00 2001 From: "Md. Ashraful" Date: Thu, 22 Oct 2026 11:59:17 +0600 Subject: [PATCH 3/3] Use isDefined for hasRecord on list pages Replace the optional-chaining check with an isDefined guard on the nine list pages so the table skeleton shows while loading and the empty state only shows once the API confirms has_records is false. Also stop scoping the order list has_records check by customer. --- app/Services/OrderService.php | 6 +----- resources/app/features/brands/pages/brands.tsx | 3 ++- resources/app/features/categories/pages/categories.tsx | 3 ++- resources/app/features/collections/pages/collections.tsx | 3 ++- resources/app/features/coupons/pages/coupons.tsx | 3 ++- resources/app/features/customers/pages/customers.tsx | 3 ++- resources/app/features/inventory/pages/inventory.tsx | 3 ++- resources/app/features/orders/pages/orders.tsx | 3 ++- resources/app/features/products/pages/products.tsx | 3 ++- resources/app/features/tags/pages/tags.tsx | 3 ++- 10 files changed, 19 insertions(+), 14 deletions(-) diff --git a/app/Services/OrderService.php b/app/Services/OrderService.php index aaa5c8de0..be76fa69c 100644 --- a/app/Services/OrderService.php +++ b/app/Services/OrderService.php @@ -122,11 +122,7 @@ public function paginated_orders(OrderListFilterDTO $dto) ->with('items') ->paginate($dto->limit ?? Pagination::LIMIT, $dto->page ?? 1); - $has_records = Order::query()->when(is_numeric($dto->customer_id), function (QueryBuilder $query) use ($dto) { - return $query->where('customer_id', $dto->customer_id); - }); - - return ListPaginator::from($page, $has_records); + return ListPaginator::from($page, Order::query()); } /** diff --git a/resources/app/features/brands/pages/brands.tsx b/resources/app/features/brands/pages/brands.tsx index 9863a52e1..ecfe4f6d7 100644 --- a/resources/app/features/brands/pages/brands.tsx +++ b/resources/app/features/brands/pages/brands.tsx @@ -6,6 +6,7 @@ import { useBrandsQuery } from '@/features/brands/services/brand'; import { brandListOptions } from '@/features/brands/types'; import { useDataTableParams } from '@/hooks'; import { BrandEmptyStateIcon } from '@/icons/empty-state'; +import { isDefined } from '@/utils/object'; import { __ } from '@/wpi18n'; const Brands = () => { @@ -13,7 +14,7 @@ const Brands = () => { const { data, isFetching } = useBrandsQuery(params); - const hasRecord = data?.has_records !== false; + const hasRecord = !isDefined(data) || data.has_records !== false; return ( diff --git a/resources/app/features/categories/pages/categories.tsx b/resources/app/features/categories/pages/categories.tsx index daeb91cb6..99d3752e9 100644 --- a/resources/app/features/categories/pages/categories.tsx +++ b/resources/app/features/categories/pages/categories.tsx @@ -6,6 +6,7 @@ import { useCategoriesQuery } from '@/features/categories/services/category'; import { categoryListOptions } from '@/features/categories/types'; import { useDataTableParams } from '@/hooks'; import { CategoryEmptyStateIcon } from '@/icons/empty-state'; +import { isDefined } from '@/utils/object'; import { __ } from '@/wpi18n'; const Categories = () => { @@ -13,7 +14,7 @@ const Categories = () => { const { data, isFetching } = useCategoriesQuery(params); - const hasRecord = data?.has_records !== false; + const hasRecord = !isDefined(data) || data.has_records !== false; return ( diff --git a/resources/app/features/collections/pages/collections.tsx b/resources/app/features/collections/pages/collections.tsx index 11c49bb2a..831ca2ddf 100644 --- a/resources/app/features/collections/pages/collections.tsx +++ b/resources/app/features/collections/pages/collections.tsx @@ -10,6 +10,7 @@ import { useCollectionsQuery } from '@/features/collections/services/collection' import { collectionListOptions } from '@/features/collections/types'; import { useDataTableParams } from '@/hooks'; import { CollectionEmptyStateIcon } from '@/icons/empty-state'; +import { isDefined } from '@/utils/object'; import { __ } from '@/wpi18n'; const Collections = () => { @@ -19,7 +20,7 @@ const Collections = () => { const { data, isFetching } = useCollectionsQuery(params); - const hasRecord = data?.has_records !== false; + const hasRecord = !isDefined(data) || data.has_records !== false; return ( diff --git a/resources/app/features/coupons/pages/coupons.tsx b/resources/app/features/coupons/pages/coupons.tsx index 26f19186c..f08237522 100644 --- a/resources/app/features/coupons/pages/coupons.tsx +++ b/resources/app/features/coupons/pages/coupons.tsx @@ -11,6 +11,7 @@ import type { CouponListFilter } from '@/features/coupons/types'; import { couponListOptions } from '@/features/coupons/types'; import { useDataTableParams } from '@/hooks'; import { CouponEmptyStateIcon } from '@/icons/empty-state'; +import { isDefined } from '@/utils/object'; import { __ } from '@/wpi18n'; const Coupons = () => { @@ -19,7 +20,7 @@ const Coupons = () => { const { data, isFetching } = useCouponsQuery(params); - const hasRecord = data?.has_records !== false; + const hasRecord = !isDefined(data) || data.has_records !== false; return ( diff --git a/resources/app/features/customers/pages/customers.tsx b/resources/app/features/customers/pages/customers.tsx index 703cc5428..72ce7053d 100644 --- a/resources/app/features/customers/pages/customers.tsx +++ b/resources/app/features/customers/pages/customers.tsx @@ -11,6 +11,7 @@ import { useCustomersQuery } from '@/features/customers/services/customer'; import { customerListOptions } from '@/features/customers/types'; import { useDataTableParams } from '@/hooks'; import { CustomerEmptyStateIcon } from '@/icons/empty-state'; +import { isDefined } from '@/utils/object'; import { __ } from '@/wpi18n'; const Customers = () => { @@ -19,7 +20,7 @@ const Customers = () => { const { data, isFetching } = useCustomersQuery(params); - const hasRecord = data?.has_records !== false; + const hasRecord = !isDefined(data) || data.has_records !== false; const handleAddNewCustomer = () => { void navigate(RouteConfig.Customers.get('CustomerDetail').buildLink({ id: NEW_ITEM_ID })); diff --git a/resources/app/features/inventory/pages/inventory.tsx b/resources/app/features/inventory/pages/inventory.tsx index f43a8240e..90b759a04 100644 --- a/resources/app/features/inventory/pages/inventory.tsx +++ b/resources/app/features/inventory/pages/inventory.tsx @@ -9,6 +9,7 @@ import { useInventoryQuery } from '@/features/inventory/services/inventory'; import { inventoryListOptions } from '@/features/inventory/types'; import { useDataTableParams } from '@/hooks'; import { InventoryEmptyStateIcon } from '@/icons/empty-state'; +import { isDefined } from '@/utils/object'; import { __ } from '@/wpi18n'; const Inventory = () => { @@ -17,7 +18,7 @@ const Inventory = () => { const { data, isFetching } = useInventoryQuery(params); - const hasRecord = data?.has_records !== false; + const hasRecord = !isDefined(data) || data.has_records !== false; return ( diff --git a/resources/app/features/orders/pages/orders.tsx b/resources/app/features/orders/pages/orders.tsx index a57d45a08..31879edb5 100644 --- a/resources/app/features/orders/pages/orders.tsx +++ b/resources/app/features/orders/pages/orders.tsx @@ -9,6 +9,7 @@ import OrderTable from '@/features/orders/components/order-table/order-table'; import { useOrdersQuery } from '@/features/orders/services/order'; import { useDataTableParams } from '@/hooks'; import { OrderEmptyStateIcon } from '@/icons/empty-state'; +import { isDefined } from '@/utils/object'; import { __ } from '@/wpi18n'; const Orders = () => { @@ -17,7 +18,7 @@ const Orders = () => { const { data, isFetching } = useOrdersQuery(params); - const hasRecord = data?.has_records !== false; + const hasRecord = !isDefined(data) || data.has_records !== false; return ( diff --git a/resources/app/features/products/pages/products.tsx b/resources/app/features/products/pages/products.tsx index 725b8cffa..152007605 100644 --- a/resources/app/features/products/pages/products.tsx +++ b/resources/app/features/products/pages/products.tsx @@ -9,6 +9,7 @@ import ProductTable from '@/features/products/components/product-table/product-t import { useProductsQuery } from '@/features/products/services/product'; import { useDataTableParams } from '@/hooks'; import { ProductEmptyStateIcon } from '@/icons/empty-state'; +import { isDefined } from '@/utils/object'; import { __ } from '@/wpi18n'; const Products = () => { @@ -17,7 +18,7 @@ const Products = () => { const { data, isFetching } = useProductsQuery(params); - const hasRecord = data?.has_records !== false; + const hasRecord = !isDefined(data) || data.has_records !== false; return ( diff --git a/resources/app/features/tags/pages/tags.tsx b/resources/app/features/tags/pages/tags.tsx index fc658ed56..272e5e56f 100644 --- a/resources/app/features/tags/pages/tags.tsx +++ b/resources/app/features/tags/pages/tags.tsx @@ -6,6 +6,7 @@ import { useTagsQuery } from '@/features/tags/services/tag'; import { tagListOptions } from '@/features/tags/types'; import { useDataTableParams } from '@/hooks'; import { TagEmptyStateIcon } from '@/icons/empty-state'; +import { isDefined } from '@/utils/object'; import { __ } from '@/wpi18n'; const Tags = () => { @@ -13,7 +14,7 @@ const Tags = () => { const { data, isFetching } = useTagsQuery(params); - const hasRecord = data?.has_records !== false; + const hasRecord = !isDefined(data) || data.has_records !== false; return (