From 864b9abcefafee6f4e8f5985eb88f36820801c11 Mon Sep 17 00:00:00 2001 From: Ivan Bochkarev Date: Tue, 28 Jul 2026 11:48:26 +0600 Subject: [PATCH] fix(vue): unwrap empty object/data in request.js connector responses Empty {} or [] payloads were falling through to the full success envelope, so grid callers expecting .results saw Invalid response without a toast. --- .github/workflows/ci.yml | 3 ++ vueManager/package.json | 1 + vueManager/src/request.js | 49 +++++++++++++---------- vueManager/tests/requestUnwrap.test.js | 55 ++++++++++++++++++++++++++ 4 files changed, 88 insertions(+), 20 deletions(-) create mode 100644 vueManager/tests/requestUnwrap.test.js diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 71ea0984..90e089b7 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -100,3 +100,6 @@ jobs: - name: Vitest run: npm test + + - name: Smoke tests + run: npm run test:smoke diff --git a/vueManager/package.json b/vueManager/package.json index 2213434d..956e92da 100644 --- a/vueManager/package.json +++ b/vueManager/package.json @@ -11,6 +11,7 @@ "preview": "vite preview", "lint": "eslint . --fix", "lint:ci": "eslint . --max-warnings 0", + "test:smoke": "node --test tests/*.test.js", "format": "prettier --write \"src/**/*.{js,vue,scss,css}\"", "format:check": "prettier --check \"src/**/*.{js,vue,scss,css}\"", "lint:all": "npm run lint && npm run format:check && npm run lint:scss", diff --git a/vueManager/src/request.js b/vueManager/src/request.js index f2f10365..6784f5a2 100644 --- a/vueManager/src/request.js +++ b/vueManager/src/request.js @@ -1,3 +1,30 @@ +/** + * Extract payload from MODX connector JSON envelope. + * + * Connector responses use `{ success, message, object?, data? }`. Callers expect + * the inner payload (e.g. `{ results, total }`), not the envelope with `success`. + * + * @param {unknown} responseData - Parsed JSON body from connector.php + * @returns {unknown} Unwrapped payload, or the original value when not an envelope + * + * Uses `!= null` so falsy scalars in `object`/`data` (0, false, "") are valid payloads. + */ +export function unwrapResponsePayload(responseData) { + if (responseData === null || typeof responseData !== 'object' || Array.isArray(responseData)) { + return responseData + } + + if ('object' in responseData && responseData.object != null) { + return responseData.object + } + + if ('data' in responseData && responseData.data != null) { + return responseData.data + } + + return responseData +} + /** * API Request class for working with MiniShop3 API through MODX connector * @@ -126,19 +153,7 @@ class Request { ) } - if (responseData.object && Object.keys(responseData.object).length > 0) { - return responseData.object - } else if ( - responseData.data && - Array.isArray(responseData.data) && - responseData.data.length > 0 - ) { - return responseData.data - } else if (responseData.data && !Array.isArray(responseData.data)) { - return responseData.data - } - - return responseData + return unwrapResponsePayload(responseData) } catch (error) { if (error instanceof RequestError) { throw error @@ -236,13 +251,7 @@ class Request { ) } - if (responseData.object && Object.keys(responseData.object).length > 0) { - return responseData.object - } else if (responseData.data) { - return responseData.data - } - - return responseData + return unwrapResponsePayload(responseData) } catch (error) { if (error instanceof RequestError) { throw error diff --git a/vueManager/tests/requestUnwrap.test.js b/vueManager/tests/requestUnwrap.test.js new file mode 100644 index 00000000..b99c2921 --- /dev/null +++ b/vueManager/tests/requestUnwrap.test.js @@ -0,0 +1,55 @@ +import assert from 'node:assert/strict' +import test from 'node:test' + +import { unwrapResponsePayload } from '../src/request.js' + +test('returns empty object payload instead of envelope', () => { + const envelope = { success: true, message: '', object: {} } + assert.deepEqual(unwrapResponsePayload(envelope), {}) +}) + +test('returns list payload with empty results', () => { + const envelope = { success: true, message: '', object: { results: [], total: 0 } } + assert.deepEqual(unwrapResponsePayload(envelope), { results: [], total: 0 }) +}) + +test('returns empty data array instead of envelope', () => { + const envelope = { success: true, message: '', data: [] } + assert.deepEqual(unwrapResponsePayload(envelope), []) +}) + +test('returns object array payload', () => { + const envelope = { success: true, message: '', object: [] } + assert.deepEqual(unwrapResponsePayload(envelope), []) +}) + +test('prefers object field over data field', () => { + const envelope = { success: true, object: { a: 1 }, data: { b: 2 } } + assert.deepEqual(unwrapResponsePayload(envelope), { a: 1 }) +}) + +test('returns non-array data object', () => { + const envelope = { success: true, data: { import_id: 'abc' } } + assert.deepEqual(unwrapResponsePayload(envelope), { import_id: 'abc' }) +}) + +test('falls back to envelope when payload fields are null', () => { + const envelope = { success: true, message: 'Deleted', object: null, data: null } + assert.equal(unwrapResponsePayload(envelope), envelope) +}) + +test('returns envelope when no payload keys', () => { + const envelope = { success: true, message: 'ok' } + assert.equal(unwrapResponsePayload(envelope), envelope) +}) + +test('returns falsy scalar object payload', () => { + assert.equal(unwrapResponsePayload({ success: true, object: 0 }), 0) + assert.equal(unwrapResponsePayload({ success: true, object: false }), false) + assert.equal(unwrapResponsePayload({ success: true, object: '' }), '') +}) + +test('passes through non-object values', () => { + assert.equal(unwrapResponsePayload(null), null) + assert.equal(unwrapResponsePayload('ok'), 'ok') +})