### NetSuite Import Aliases – Design

#### Context
- We route SuiteQL through a RESTlet which wraps queries for pagination using ROW_NUMBER() OVER (ORDER BY ...).
- NetSuite `query.runSuiteQL(...).asMappedResults()` de-duplicates duplicate column names (e.g., `id`) by suffixing `_0`, `_0_0` when no aliases are provided. With `SELECT *` and joins (e.g., `transactionline` + `transaction`), plain `id` is not present; records arrive as `id_0`, `id_0_0` etc.
- Our preprocessing currently expects stable keys (e.g., `id`, or for lines `uniquekey`); missing keys cause drops like “NetSuite record missing ID field”.

#### Goals
- Eliminate key collisions and suffixes from `.asMappedResults()` by introducing canonical aliases for critical columns.
- Keep logic in the application (avoid pushing complexity into the RESTlet) while enabling per-record-type overrides.
- Preserve centralized protections: `SELECT *`, app-driven date-filter injection, pagination, and COUNT-vs-SELECT differences.

---

### Approach A (Option 1): Central Alias Defaults in the Application

Store a small registry of aliases and default ordering per record type in the app. The query builder prepends these aliases to the projection, followed by `*` to retain the full payload.

Recommended storage (either):
- `config/suiteql.php`
- `App\Services\NetSuite\SuiteQLAliasRegistry` (class returning arrays, simple to unit test)

Example (config/suiteql.php):
```php
<?php
return [
    'aliases' => [
        // Transaction Line page queries
        'TransactionLine' => [
            "transactionline.id AS line_id",
            "transactionline.uniquekey AS line_uniquekey",
            "transaction.id AS transaction_id",
        ],
        // Items (shared or per-sub-type)
        'Item' => [
            "item.id AS item_id",
            "item.externalid AS item_external_id",
        ],
        'ServiceItem' => [
            "item.id AS item_id",
            "item.externalid AS item_external_id",
        ],
        // Add other record types incrementally
    ],
    'order_by' => [
        'TransactionLine' => 'transactionline.id ASC, transaction.id ASC',
        'Item' => 'item.id ASC',
        'ServiceItem' => 'item.id ASC',
    ],
];
```

Builder composition changes:
1) Determine `recordType` name (e.g., `TransactionLine`, `Item`).
2) Load aliases for that type; build:
   - `SELECT {aliases..., *} FROM {criteria} {inner} {where}`
3) Inject date filter centrally (existing `buildDateRangeFilter()`), choosing WHERE vs AND correctly.
4) Use per-type `order_by` if present; else fallback to safe default (`{criteria}.id ASC`).
5) COUNT path remains separate and must not append `ORDER BY`.

Importer/preprocessor mapping (canonical keys to use):
- TransactionLine: prefer `line_uniquekey` for `refid`/`external_id` (string), fallback to `line_id`.
- Item/ServiceItem: prefer `item_external_id` for `external_id`, fallback to `string(item_id)`.

Pros:
- Minimal change; immediate fix for `id_0` suffix collisions.
- Central place to manage defaults; record types without entries keep using `SELECT *`.

Cons:
- Still requires maintaining a small per-record-type alias list in code.

---

### Approach B (Option 2): Criteria-Driven Overrides (Aliases + Order)

Augment each record type’s `criteria.json` with optional SuiteQL composition hints. The builder remains the source of truth for protections (pagination/date/COUNT), while criteria can override projection aliases and ordering.

Proposed `criteria.json` additions:
```json
{
  "criteria": "transactionline",
  "inner": "INNER JOIN transaction ON transactionline.transaction = transaction.ID",
  "where": "WHERE transactionline.mainline = 'F'",
  "suiteql": {
    "aliases": [
      "transactionline.id AS line_id",
      "transactionline.uniquekey AS line_uniquekey",
      "transaction.id AS transaction_id"
    ],
    "order_by": "transactionline.id ASC, transaction.id ASC"
  }
}
```

Builder precedence:
1) If `suiteql.aliases`/`suiteql.order_by` present → use them.
2) Else use Option 1 defaults from config/registry.
3) Else fallback to `SELECT *` + default `{criteria}.id ASC`.

Protections kept in the builder:
- Always `SELECT {aliases..., *} ...` for page queries; never `GROUP BY` on page queries.
- Centralized date filter injection using known field per `criteria` (e.g., `linecreateddate` for transaction lines).
- COUNT queries built separately without `ORDER BY` (and without unnecessary joins when feasible).

Pros:
- Data-driven flexibility without pushing raw SQL everywhere.
- Central wrapper still enforces safety and consistency.

Cons:
- Requires touching `criteria.json` for record types that need explicit aliases/ordering.

---

### Coexistence & Precedence
- Option 1 (app defaults) provides a safety net.
- Option 2 (criteria overrides) refines behavior per record type.
- Precedence: criteria-defined aliases/order_by > app defaults > fallback.

---

### Date Filters & COUNT
- Date filters remain injected centrally by the app. For page queries, inject `AND ...` vs `WHERE ...` based on presence of `WHERE`.
- COUNT queries must not include `ORDER BY` and should avoid unnecessary joins when possible.

---

### Acceptance Criteria
1) No `.asMappedResults()` suffix keys for critical columns in parsed responses (e.g., no reliance on `id_0`, `id_0_0`).
2) TransactionLine import:
   - `refid`/`external_id` are populated from `line_uniquekey` (preferred) or `line_id` fallback.
   - No “missing ID field”/preprocessing skip logs for valid rows.
3) Item/ServiceItem import:
   - `external_id` populated from `item_external_id` (preferred) or `string(item_id)` fallback.
   - Validation errors “external id field must be a string” do not occur for valid rows.
4) Page queries continue to use `SELECT {aliases..., *}` without `GROUP BY`; COUNT queries succeed without `ORDER BY`.
5) Date filter injection still applies correctly per record type.
6) Logs show `first_record_keys` with canonical alias keys for each record type (e.g., `line_id`, `line_uniquekey`, `transaction_id`; `item_id`, `item_external_id`).

---

### Testing Recommendations (Pseudo-code)

Unit – Builder alias injection
```php
it('prepends aliases and keeps select star', function () {
    // Given: recordType = TransactionLine, config aliases present
    $sql = buildNetSuiteQuery(TransactionLine);
    expect($sql)->toContain('transactionline.id AS line_id');
    expect($sql)->toContain('transactionline.uniquekey AS line_uniquekey');
    expect($sql)->toContain('SELECT');
    expect($sql)->toContain('* FROM transactionline');
    expect($sql)->toContain('ORDER BY transactionline.id');
});
```

Unit – Criteria overrides precedence
```php
it('uses criteria-defined aliases when present', function () {
    // Given: criteria.json defines suiteql.aliases and suiteql.order_by
    $sql = buildNetSuiteQuery(TransactionLine);
    expect($sql)->toContain('line_id');
    // And not contain config default if different
});
```

Unit – COUNT query behavior
```php
it('builds count query without ORDER BY and avoids unnecessary joins', function () {
    $sql = buildCountQuery(TransactionLine);
    expect($sql)->not->toContain('ORDER BY');
    // Optional: expect reduced joins for COUNT
});
```

Integration – RESTlet smoke
```php
it('returns canonical alias keys via RESTlet', function () {
    $payload = [
        'query' => 'SELECT transactionline.id AS line_id, transactionline.uniquekey AS line_uniquekey, transaction.id AS transaction_id, * FROM transactionline INNER JOIN transaction ON transactionline.transaction = transaction.ID WHERE transactionline.mainline = \"F\" ORDER BY transactionline.id ASC',
        'offset' => 0,
        'limit' => 100
    ];
    $res = restletPost($payload);
    expect($res['success'])->toBeTrue();
    expect(array_keys($res['data'][0]))->toContain('line_id');
    expect(array_keys($res['data'][0]))->toContain('line_uniquekey');
});
```

Unit – Preprocessing mapping
```php
it('sets refid/external_id from canonical aliases for TransactionLine', function () {
    $row = ['line_uniquekey' => '123', 'line_id' => 456];
    $mapped = preprocessTransactionLine($row);
    expect($mapped['refid'])->toBe(123);
    expect($mapped['external_id'])->toBe('123');
});

it('sets external_id for Item from item_external_id or item_id string', function () {
    $row = ['item_id' => 999, 'item_external_id' => null];
    $mapped = preprocessItem($row);
    expect($mapped['external_id'])->toBe('999');
});
```

End-to-end – Small batch import
```php
it('imports a page of TransactionLine records', function () {
    dispatchBatch(TransactionLine, offset: 0, limit: 100);
    expect(logs())->toContain('NetSuite response parsed');
    expect(db('transaction_lines')->count())->toBeGreaterThan(0);
    expect(logs())->not->toContain('missing ID field');
});
```

---

### Rollout Plan
1) Implement Approach A defaults in app builder; update preprocessing to use canonical keys (with fallbacks).
2) Verify TransactionLine and Item imports create records (monitor logs for `first_record_keys`).
3) Introduce Approach B incrementally by adding `suiteql.aliases`/`order_by` to problematic record types’ `criteria.json`.
4) Keep protections (pagination/date/COUNT) centralized in the builder; RESTlet stays generic.


#### Future Operational Considerations (Queue and Concurrency)
- Per-tenant worker limits on import queue: Cap the number of concurrent workers that any single tenant can consume to prevent excessive NetSuite concurrency rejections and to provide fair-share scheduling across tenants.
- Global worker pool sizing: Maintain a sufficiently large overall worker pool so multiple tenants can import simultaneously without starving throughput. Balance per-tenant caps with total capacity.
- Concurrency slot reservation pitfalls: Previous designs that “reserve” external API slots and release them after completion led to slots being held until timeout under failure or retry conditions. A reservation approach may still be feasible, but it needs a more robust design (lease/TTL, heartbeats, idempotent release) to avoid leaked reservations and cascading stalls.