From 73f39499985f241a3cf2735cac0716378ed710b3 Mon Sep 17 00:00:00 2001 From: Evelyn Lo Date: Thu, 23 Jan 2025 01:21:21 -0500 Subject: [PATCH 1/6] Refactor xhr function to reduce cognitive complexity --- public/src/modules/api.js | 37 ++++++++++++------------------------- 1 file changed, 12 insertions(+), 25 deletions(-) diff --git a/public/src/modules/api.js b/public/src/modules/api.js index fcee6b94b0..a3b36d522b 100644 --- a/public/src/modules/api.js +++ b/public/src/modules/api.js @@ -35,7 +35,7 @@ async function call(options, callback) { async function xhr(options) { // Normalize body based on type - const { url } = options; + const url = options.url; delete options.url; if (options.data && !(options.data instanceof FormData)) { @@ -44,17 +44,9 @@ async function xhr(options) { } // Allow options to be modified by plugins, etc. - ({ options } = await fireHook('filter:api.options', { options })); - - /** - * Note: pre-v4 backwards compatibility - * - * This module now passes in "data" to xhr(). - * This is because the "filter:api.options" hook (and plugins using it) expect "data". - * fetch() expects body, so we rename it here. - * - * In v4, replace all instances of "data" with "body" and record as breaking change. - */ + const hookResult = await fireHook('filter:api.options', { options }); + options = hookResult.options; + if (options.data) { options.body = options.data; delete options.data; @@ -65,25 +57,20 @@ async function xhr(options) { const contentType = headers.get('content-type'); const isJSON = contentType && contentType.startsWith('application/json'); - let response; - if (options.method !== 'HEAD') { + if (!res.ok) { + let errorMessage; if (isJSON) { - response = await res.json(); + const jsonResponse = await res.json(); + errorMessage = jsonResponse.status?.message || res.statusText; } else { - response = await res.text(); + errorMessage = await res.text(); } + throw new Error(errorMessage); } - if (!res.ok) { - if (response) { - throw new Error(isJSON ? response.status.message : response); - } - throw new Error(res.statusText); + if (options.method !== 'HEAD') { + return isJSON ? await res.json() : await res.text(); } - - return isJSON && response && response.hasOwnProperty('status') && response.hasOwnProperty('response') ? - response.response : - response; } export function get(route, data, onSuccess) { From 6383a1e50ea4b9ea1c254c24397275bb5a55d79b Mon Sep 17 00:00:00 2001 From: Evelyn Lo Date: Thu, 23 Jan 2025 13:26:01 -0500 Subject: [PATCH 2/6] Refactor xhr function to reduce cognitive complexity --- public/src/modules/api.js | 65 +++++++++++++++++++++++---------------- 1 file changed, 38 insertions(+), 27 deletions(-) diff --git a/public/src/modules/api.js b/public/src/modules/api.js index a3b36d522b..cd37b86d5d 100644 --- a/public/src/modules/api.js +++ b/public/src/modules/api.js @@ -6,17 +6,14 @@ import { fire as fireHook } from 'hooks'; import { confirm } from 'bootbox'; const baseUrl = config.relative_path + '/api/v3'; - async function call(options, callback) { options.url = options.url.startsWith('/api') ? config.relative_path + options.url : baseUrl + options.url; - if (typeof callback === 'function') { xhr(options).then(result => callback(null, result), err => callback(err)); return; } - try { const result = await xhr(options); return result; @@ -33,59 +30,76 @@ async function call(options, callback) { } } +async function parseResponse(res, isJson) { + return isJson ? await res.json : await res.text(); +} + +function handleErrorResponse(res, response, isJSON) { + if (response) { + throw new Error(isJSON ? response.status.message : response); + } + throw new Error(res.statusText); +} async function xhr(options) { // Normalize body based on type - const url = options.url; - delete options.url; + console.log("Test Log: Code execution reached here - Evelyn Lo"); + const { url } = options; + delete options.url; if (options.data && !(options.data instanceof FormData)) { options.data = JSON.stringify(options.data || {}); options.headers['content-type'] = 'application/json; charset=utf-8'; } - // Allow options to be modified by plugins, etc. - const hookResult = await fireHook('filter:api.options', { options }); - options = hookResult.options; - + ({ options } = await fireHook('filter:api.options', { options })); + /** + * Note: pre-v4 backwards compatibility + * + * This module now passes in "data" to xhr(). + * This is because the "filter:api.options" hook (and plugins using it) expect "data". + * fetch() expects body, so we rename it here. + * + * In v4, replace all instances of "data" with "body" and record as breaking change. + */ if (options.data) { options.body = options.data; delete options.data; } - const res = await fetch(url, options); const { headers } = res; const contentType = headers.get('content-type'); const isJSON = contentType && contentType.startsWith('application/json'); - - if (!res.ok) { - let errorMessage; - if (isJSON) { - const jsonResponse = await res.json(); - errorMessage = jsonResponse.status?.message || res.statusText; - } else { - errorMessage = await res.text(); - } - throw new Error(errorMessage); + let response; + if (options.method !== 'HEAD') { + response = await parseResponse(res, isJSON); } + // if (!res.ok) { + // if (response) { + // throw new Error(isJSON ? response.status.message : response); + // } + // throw new Error(res.statusText); + // } - if (options.method !== 'HEAD') { - return isJSON ? await res.json() : await res.text(); + if (!res.ok) { + handleErrorResponse(res, response, isJSON); } + return isJSON && response && response.hasOwnProperty('status') && response.hasOwnProperty('response') ? + response.response : + response; } + export function get(route, data, onSuccess) { return call({ url: route + (data && Object.keys(data).length ? ('?' + $.param(data)) : ''), }, onSuccess); } - export function head(route, data, onSuccess) { return call({ url: route + (data && Object.keys(data).length ? ('?' + $.param(data)) : ''), method: 'HEAD', }, onSuccess); } - export function post(route, data, onSuccess) { return call({ url: route, @@ -96,7 +110,6 @@ export function post(route, data, onSuccess) { }, }, onSuccess); } - export function patch(route, data, onSuccess) { return call({ url: route, @@ -107,7 +120,6 @@ export function patch(route, data, onSuccess) { }, }, onSuccess); } - export function put(route, data, onSuccess) { return call({ url: route, @@ -118,7 +130,6 @@ export function put(route, data, onSuccess) { }, }, onSuccess); } - export function del(route, data, onSuccess) { return call({ url: route, From 6bbbb3aeabc60c9573801cb4f2f987c16c28f429 Mon Sep 17 00:00:00 2001 From: Evelyn Lo Date: Thu, 23 Jan 2025 13:37:54 -0500 Subject: [PATCH 3/6] double quote to single quote --- public/src/modules/api.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/public/src/modules/api.js b/public/src/modules/api.js index cd37b86d5d..249ef65c0a 100644 --- a/public/src/modules/api.js +++ b/public/src/modules/api.js @@ -42,7 +42,7 @@ function handleErrorResponse(res, response, isJSON) { } async function xhr(options) { // Normalize body based on type - console.log("Test Log: Code execution reached here - Evelyn Lo"); + console.log('Test Log: Code execution reached here - Evelyn Lo'); const { url } = options; delete options.url; From 428ff597045c428bc37a28206f17e1237b59fde8 Mon Sep 17 00:00:00 2001 From: Evelyn Lo Date: Thu, 23 Jan 2025 17:03:11 -0500 Subject: [PATCH 4/6] add log statements --- src/posts/votes.js | 24 +++++- test/modules/api.test.js | 170 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 192 insertions(+), 2 deletions(-) create mode 100644 test/modules/api.test.js diff --git a/src/posts/votes.js b/src/posts/votes.js index bfe5e1e47f..ca5444c17c 100644 --- a/src/posts/votes.js +++ b/src/posts/votes.js @@ -153,9 +153,29 @@ module.exports = function (Posts) { if (isPrivileged) { return; } - if (reputation < meta.config[`min:rep:${type}`]) { - throw new Error(`[[error:not-enough-reputation-to-${type}, ${meta.config[`min:rep:${type}`]}]]`); + + console.log('Reputation:', reputation); + const minReputationRequired = meta.config[`min:rep:${type}`]; + console.log('Min Reputation Required:', minReputationRequired); + const errorMessage = `[[error:not-enough-reputation-to-${type}, ${minReputationRequired}]]`; + console.log('Error Message:', errorMessage); + + if (reputation < minReputationRequired) { + console.log('here'); + throw new Error(errorMessage); } + + console.log('**Evelyn**'); + console.log('Voted PIDs Today:', votedPidsToday); + console.log('Votes Today Limit:', votesToday); + console.log('Voter Per User Today:', voterPerUserToday); + console.log('Target UID:', targetUid); + + + /* + if (reputation < meta.config[`min:rep:${type}`]) { + throw new Error(`[[error:not-enough-reputation-to-${type}, ${meta.config[`min:rep:${type}`]}]]`); + } */ const votesToday = meta.config[`${type}sPerDay`]; if (votesToday && votedPidsToday.length >= votesToday) { throw new Error(`[[error:too-many-${type}s-today, ${votesToday}]]`); diff --git a/test/modules/api.test.js b/test/modules/api.test.js new file mode 100644 index 0000000000..e9e76e7b56 --- /dev/null +++ b/test/modules/api.test.js @@ -0,0 +1,170 @@ +// Import the functions to be tested +import * as api from '../../public/src/modules/api'; +import { fire as fireHook } from 'hooks'; +import { confirm } from 'bootbox'; + +// Mock dependencies +jest.mock('hooks', () => ({ + fire: jest.fn(), +})); +jest.mock('bootbox', () => ({ + confirm: jest.fn(), +})); +global.fetch = jest.fn(); + +// Mock config +global.config = { + relative_path: '/test-path', + csrf_token: 'test-csrf-token', +}; + +// Test suite +describe('api.js', () => { + beforeEach(() => { + jest.clearAllMocks(); + }); + + describe('call', () => { + it('should call xhr with correct options and return result', async () => { + const mockResponse = { response: { data: 'test' }, status: { message: 'Success' } }; + global.fetch.mockResolvedValue({ + ok: true, + json: async () => mockResponse, + headers: { + get: () => 'application/json', + }, + }); + fireHook.mockResolvedValue({ url: 'test-login' }); + + const result = await api.get('/test-route', { key: 'value' }); + expect(result).toEqual({ data: 'test' }); + expect(global.fetch).toHaveBeenCalledWith( + '/test-path/api/v3/test-route?key=value', + expect.objectContaining({ + method: 'GET', + }) + ); + }); + + it('should handle reauthentication error', async () => { + const error = new Error('A valid login session was not found. Please log in and try again.'); + global.fetch.mockRejectedValue(error); + + fireHook.mockResolvedValue({ url: 'login' }); + confirm.mockImplementation((_, cb) => cb(true)); + + try { + await api.get('/test-route'); + } catch (err) { + expect(err).toBe(error); + } + expect(confirm).toHaveBeenCalledWith('[[error:api.reauth-required]]', expect.any(Function)); + }); + + it('should throw an error for non-reauthentication issues', async () => { + const error = new Error('Test Error'); + global.fetch.mockRejectedValue(error); + + await expect(api.get('/test-route')).rejects.toThrow('Test Error'); + }); + }); + + describe('xhr', () => { + it('should handle JSON responses correctly', async () => { + const mockResponse = { response: { data: 'test' }, status: { message: 'Success' } }; + global.fetch.mockResolvedValue({ + ok: true, + json: async () => mockResponse, + headers: { + get: () => 'application/json', + }, + }); + + const result = await api.get('/test-route'); + expect(result).toEqual({ data: 'test' }); + }); + + it('should handle non-JSON responses correctly', async () => { + global.fetch.mockResolvedValue({ + ok: true, + text: async () => 'test-response', + headers: { + get: () => 'text/plain', + }, + }); + + const result = await api.get('/test-route'); + expect(result).toEqual('test-response'); + }); + + it('should throw an error for non-OK responses', async () => { + global.fetch.mockResolvedValue({ + ok: false, + statusText: 'Test Error', + headers: { + get: () => 'text/plain', + }, + }); + + await expect(api.get('/test-route')).rejects.toThrow('Test Error'); + }); + }); + + describe('HTTP methods', () => { + it('should make GET requests correctly', async () => { + global.fetch.mockResolvedValue({ + ok: true, + json: async () => ({}), + headers: { + get: () => 'application/json', + }, + }); + + await api.get('/test-route', { key: 'value' }); + expect(global.fetch).toHaveBeenCalledWith( + '/test-path/api/v3/test-route?key=value', + expect.objectContaining({ method: 'GET' }) + ); + }); + + it('should make POST requests correctly', async () => { + global.fetch.mockResolvedValue({ + ok: true, + json: async () => ({}), + headers: { + get: () => 'application/json', + }, + }); + + await api.post('/test-route', { key: 'value' }); + expect(global.fetch).toHaveBeenCalledWith( + '/test-path/api/v3/test-route', + expect.objectContaining({ + method: 'POST', + body: JSON.stringify({ key: 'value' }), + headers: expect.objectContaining({ 'x-csrf-token': 'test-csrf-token' }), + }) + ); + }); + + it('should make DELETE requests correctly', async () => { + global.fetch.mockResolvedValue({ + ok: true, + json: async () => ({}), + headers: { + get: () => 'application/json', + }, + }); + + await api.del('/test-route', { key: 'value' }); + expect(global.fetch).toHaveBeenCalledWith( + '/test-path/api/v3/test-route', + expect.objectContaining({ + method: 'DELETE', + body: JSON.stringify({ key: 'value' }), + headers: expect.objectContaining({ 'x-csrf-token': 'test-csrf-token' }), + }) + ); + }); + }); +}); From 5644b3787f1345bd5654a408017bfeaa7a5174d8 Mon Sep 17 00:00:00 2001 From: Evelyn Lo Date: Thu, 23 Jan 2025 19:11:08 -0500 Subject: [PATCH 5/6] fix syntax issues --- src/posts/votes.js | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/src/posts/votes.js b/src/posts/votes.js index ca5444c17c..13ca1daff0 100644 --- a/src/posts/votes.js +++ b/src/posts/votes.js @@ -164,23 +164,22 @@ module.exports = function (Posts) { console.log('here'); throw new Error(errorMessage); } - console.log('**Evelyn**'); console.log('Voted PIDs Today:', votedPidsToday); - console.log('Votes Today Limit:', votesToday); - console.log('Voter Per User Today:', voterPerUserToday); console.log('Target UID:', targetUid); - - /* if (reputation < meta.config[`min:rep:${type}`]) { throw new Error(`[[error:not-enough-reputation-to-${type}, ${meta.config[`min:rep:${type}`]}]]`); } */ const votesToday = meta.config[`${type}sPerDay`]; + console.log('Votes Today Limit:', votesToday); + if (votesToday && votedPidsToday.length >= votesToday) { throw new Error(`[[error:too-many-${type}s-today, ${votesToday}]]`); } const voterPerUserToday = meta.config[`${type}sPerUserPerDay`]; + console.log('Voter Per User Today:', voterPerUserToday); + if (voterPerUserToday) { const postData = await Posts.getPostsFields(votedPidsToday, ['uid']); const targetUpVotes = postData.filter(p => p.uid === targetUid).length; From 6f0bed450e2a04d5d8fae5d78e043204af7f844c Mon Sep 17 00:00:00 2001 From: Evelyn Lo Date: Thu, 23 Jan 2025 19:14:28 -0500 Subject: [PATCH 6/6] fix syntax issues --- test/modules/api.test.js | 170 --------------------------------------- 1 file changed, 170 deletions(-) delete mode 100644 test/modules/api.test.js diff --git a/test/modules/api.test.js b/test/modules/api.test.js deleted file mode 100644 index e9e76e7b56..0000000000 --- a/test/modules/api.test.js +++ /dev/null @@ -1,170 +0,0 @@ -// Import the functions to be tested -import * as api from '../../public/src/modules/api'; -import { fire as fireHook } from 'hooks'; -import { confirm } from 'bootbox'; - -// Mock dependencies -jest.mock('hooks', () => ({ - fire: jest.fn(), -})); -jest.mock('bootbox', () => ({ - confirm: jest.fn(), -})); -global.fetch = jest.fn(); - -// Mock config -global.config = { - relative_path: '/test-path', - csrf_token: 'test-csrf-token', -}; - -// Test suite -describe('api.js', () => { - beforeEach(() => { - jest.clearAllMocks(); - }); - - describe('call', () => { - it('should call xhr with correct options and return result', async () => { - const mockResponse = { response: { data: 'test' }, status: { message: 'Success' } }; - global.fetch.mockResolvedValue({ - ok: true, - json: async () => mockResponse, - headers: { - get: () => 'application/json', - }, - }); - fireHook.mockResolvedValue({ url: 'test-login' }); - - const result = await api.get('/test-route', { key: 'value' }); - expect(result).toEqual({ data: 'test' }); - expect(global.fetch).toHaveBeenCalledWith( - '/test-path/api/v3/test-route?key=value', - expect.objectContaining({ - method: 'GET', - }) - ); - }); - - it('should handle reauthentication error', async () => { - const error = new Error('A valid login session was not found. Please log in and try again.'); - global.fetch.mockRejectedValue(error); - - fireHook.mockResolvedValue({ url: 'login' }); - confirm.mockImplementation((_, cb) => cb(true)); - - try { - await api.get('/test-route'); - } catch (err) { - expect(err).toBe(error); - } - expect(confirm).toHaveBeenCalledWith('[[error:api.reauth-required]]', expect.any(Function)); - }); - - it('should throw an error for non-reauthentication issues', async () => { - const error = new Error('Test Error'); - global.fetch.mockRejectedValue(error); - - await expect(api.get('/test-route')).rejects.toThrow('Test Error'); - }); - }); - - describe('xhr', () => { - it('should handle JSON responses correctly', async () => { - const mockResponse = { response: { data: 'test' }, status: { message: 'Success' } }; - global.fetch.mockResolvedValue({ - ok: true, - json: async () => mockResponse, - headers: { - get: () => 'application/json', - }, - }); - - const result = await api.get('/test-route'); - expect(result).toEqual({ data: 'test' }); - }); - - it('should handle non-JSON responses correctly', async () => { - global.fetch.mockResolvedValue({ - ok: true, - text: async () => 'test-response', - headers: { - get: () => 'text/plain', - }, - }); - - const result = await api.get('/test-route'); - expect(result).toEqual('test-response'); - }); - - it('should throw an error for non-OK responses', async () => { - global.fetch.mockResolvedValue({ - ok: false, - statusText: 'Test Error', - headers: { - get: () => 'text/plain', - }, - }); - - await expect(api.get('/test-route')).rejects.toThrow('Test Error'); - }); - }); - - describe('HTTP methods', () => { - it('should make GET requests correctly', async () => { - global.fetch.mockResolvedValue({ - ok: true, - json: async () => ({}), - headers: { - get: () => 'application/json', - }, - }); - - await api.get('/test-route', { key: 'value' }); - expect(global.fetch).toHaveBeenCalledWith( - '/test-path/api/v3/test-route?key=value', - expect.objectContaining({ method: 'GET' }) - ); - }); - - it('should make POST requests correctly', async () => { - global.fetch.mockResolvedValue({ - ok: true, - json: async () => ({}), - headers: { - get: () => 'application/json', - }, - }); - - await api.post('/test-route', { key: 'value' }); - expect(global.fetch).toHaveBeenCalledWith( - '/test-path/api/v3/test-route', - expect.objectContaining({ - method: 'POST', - body: JSON.stringify({ key: 'value' }), - headers: expect.objectContaining({ 'x-csrf-token': 'test-csrf-token' }), - }) - ); - }); - - it('should make DELETE requests correctly', async () => { - global.fetch.mockResolvedValue({ - ok: true, - json: async () => ({}), - headers: { - get: () => 'application/json', - }, - }); - - await api.del('/test-route', { key: 'value' }); - expect(global.fetch).toHaveBeenCalledWith( - '/test-path/api/v3/test-route', - expect.objectContaining({ - method: 'DELETE', - body: JSON.stringify({ key: 'value' }), - headers: expect.objectContaining({ 'x-csrf-token': 'test-csrf-token' }), - }) - ); - }); - }); -});