From 09854147d1775b9e4e66b4d1bcb6cfe38cba4bd8 Mon Sep 17 00:00:00 2001 From: Faruk AYDIN Date: Wed, 14 Feb 2024 00:51:16 +0100 Subject: [PATCH 1/5] feat: Extend renderer functionality to work with pagination --- packages/backend/src/helpers/renderer.js | 27 ++++++++++++++++++------ 1 file changed, 20 insertions(+), 7 deletions(-) diff --git a/packages/backend/src/helpers/renderer.js b/packages/backend/src/helpers/renderer.js index fcc6e6a0..50865256 100644 --- a/packages/backend/src/helpers/renderer.js +++ b/packages/backend/src/helpers/renderer.js @@ -1,14 +1,27 @@ +const isPaginated = (object) => + object?.pageInfo && + object?.totalCount !== undefined && + Array.isArray(object?.records); + +const isArray = (object) => + Array.isArray(object) || Array.isArray(object?.records); + +const totalCount = (object) => + isPaginated(object) ? object.totalCount : isArray(object) ? object.length : 1; + const renderObject = (response, object) => { - const isArray = Array.isArray(object); + const data = isPaginated(object) ? object.records : object; const computedPayload = { - data: object, + data, meta: { - type: object.constructor.name, - count: isArray ? object.length : 1, - isArray, - currentPage: null, - totalPages: null, + type: isPaginated(object) + ? object.records[0].constructor.name + : object.constructor.name, + count: totalCount(object), + isArray: isArray(object), + currentPage: isPaginated(object) ? object.pageInfo.currentPage : null, + totalPages: isPaginated(object) ? object.pageInfo.totalPages : null, }, }; From b89a4d58d986721d37c9d22fc0690c5d641b146d Mon Sep 17 00:00:00 2001 From: Faruk AYDIN Date: Wed, 14 Feb 2024 00:51:48 +0100 Subject: [PATCH 2/5] feat: Add pagination for REST endpoints --- .../backend/src/helpers/pagination-rest.js | 25 +++++++++++++++++++ 1 file changed, 25 insertions(+) create mode 100644 packages/backend/src/helpers/pagination-rest.js diff --git a/packages/backend/src/helpers/pagination-rest.js b/packages/backend/src/helpers/pagination-rest.js new file mode 100644 index 00000000..89239d85 --- /dev/null +++ b/packages/backend/src/helpers/pagination-rest.js @@ -0,0 +1,25 @@ +const paginateRest = async (query, page) => { + const pageSize = 10; + + page = parseInt(page, 10); + + if (isNaN(page) || page < 1) { + page = 1; + } + + const [records, count] = await Promise.all([ + query.limit(pageSize).offset((page - 1) * pageSize), + query.resultSize(), + ]); + + return { + pageInfo: { + currentPage: page, + totalPages: Math.ceil(count / pageSize), + }, + totalCount: count, + records, + }; +}; + +export default paginateRest; From da732becb63c3d81efd61f50f3b23e46a051c945 Mon Sep 17 00:00:00 2001 From: Faruk AYDIN Date: Wed, 14 Feb 2024 00:52:17 +0100 Subject: [PATCH 3/5] feat: Implement api/v1/users API endpoint --- .../src/controllers/api/v1/users/get-users.js | 18 ++++++++++++++++++ packages/backend/src/routes/api/v1/users.js | 2 ++ 2 files changed, 20 insertions(+) create mode 100644 packages/backend/src/controllers/api/v1/users/get-users.js diff --git a/packages/backend/src/controllers/api/v1/users/get-users.js b/packages/backend/src/controllers/api/v1/users/get-users.js new file mode 100644 index 00000000..11728f90 --- /dev/null +++ b/packages/backend/src/controllers/api/v1/users/get-users.js @@ -0,0 +1,18 @@ +import { renderObject } from '../../../../helpers/renderer.js'; +import User from '../../../../models/user.js'; +import paginateRest from '../../../../helpers/pagination-rest.js'; + +export default async (request, response) => { + const usersQuery = User.query() + .leftJoinRelated({ + role: true, + }) + .withGraphFetched({ + role: true, + }) + .orderBy('full_name', 'asc'); + + const users = await paginateRest(usersQuery, request.query.page); + + renderObject(response, users); +}; diff --git a/packages/backend/src/routes/api/v1/users.js b/packages/backend/src/routes/api/v1/users.js index 08bc8f65..fb1ece18 100644 --- a/packages/backend/src/routes/api/v1/users.js +++ b/packages/backend/src/routes/api/v1/users.js @@ -3,9 +3,11 @@ import { authenticateUser } from '../../../helpers/authentication.js'; import { authorizeUser } from '../../../helpers/authorization.js'; import getCurrentUserAction from '../../../controllers/api/v1/users/get-current-user.js'; import getUserAction from '../../../controllers/api/v1/users/get-user.js'; +import getUsersAction from '../../../controllers/api/v1/users/get-users.js'; const router = Router(); +router.get('/', authenticateUser, authorizeUser, getUsersAction); router.get('/me', authenticateUser, getCurrentUserAction); router.get('/:userId', authenticateUser, authorizeUser, getUserAction); From d583e4242819e30a6dba87e958123a09a9265628 Mon Sep 17 00:00:00 2001 From: Faruk AYDIN Date: Wed, 14 Feb 2024 00:58:12 +0100 Subject: [PATCH 4/5] refactor: Structure api mock data with folders --- .../src/controllers/api/v1/users/get-current-user.test.js | 4 ++-- .../backend/src/controllers/api/v1/users/get-user.test.js | 4 ++-- .../rest/api/v1/users/get-current-user.js} | 4 ++-- .../{payloads/user.js => mocks/rest/api/v1/users/get-user.js} | 4 ++-- 4 files changed, 8 insertions(+), 8 deletions(-) rename packages/backend/test/{payloads/current-user.js => mocks/rest/api/v1/users/get-current-user.js} (89%) rename packages/backend/test/{payloads/user.js => mocks/rest/api/v1/users/get-user.js} (90%) diff --git a/packages/backend/src/controllers/api/v1/users/get-current-user.test.js b/packages/backend/src/controllers/api/v1/users/get-current-user.test.js index 04b948b3..922dd409 100644 --- a/packages/backend/src/controllers/api/v1/users/get-current-user.test.js +++ b/packages/backend/src/controllers/api/v1/users/get-current-user.test.js @@ -3,7 +3,7 @@ import request from 'supertest'; import app from '../../../../app.js'; import createAuthTokenByUserId from '../../../../helpers/create-auth-token-by-user-id'; import { createUser } from '../../../../../test/factories/user'; -import currentUserPayload from '../../../../../test/payloads/current-user.js'; +import getCurrentUserMock from '../../../../../test/mocks/rest/api/v1/users/get-current-user'; describe('GET /api/v1/users/me', () => { let role, currentUser, token; @@ -20,7 +20,7 @@ describe('GET /api/v1/users/me', () => { .set('Authorization', token) .expect(200); - const expectedPayload = currentUserPayload(currentUser, role); + const expectedPayload = getCurrentUserMock(currentUser, role); expect(response.body).toEqual(expectedPayload); }); }); diff --git a/packages/backend/src/controllers/api/v1/users/get-user.test.js b/packages/backend/src/controllers/api/v1/users/get-user.test.js index 35107de3..4ce64f82 100644 --- a/packages/backend/src/controllers/api/v1/users/get-user.test.js +++ b/packages/backend/src/controllers/api/v1/users/get-user.test.js @@ -4,7 +4,7 @@ import app from '../../../../app.js'; import createAuthTokenByUserId from '../../../../helpers/create-auth-token-by-user-id'; import { createUser } from '../../../../../test/factories/user'; import { createPermission } from '../../../../../test/factories/permission'; -import userPayload from '../../../../../test/payloads/user.js'; +import getUserMock from '../../../../../test/mocks/rest/api/v1/users/get-user'; describe('GET /api/v1/users/:userId', () => { let currentUser, currentUserRole, anotherUser, anotherUserRole, token; @@ -30,7 +30,7 @@ describe('GET /api/v1/users/:userId', () => { .set('Authorization', token) .expect(200); - const expectedPayload = userPayload(anotherUser, anotherUserRole); + const expectedPayload = getUserMock(anotherUser, anotherUserRole); expect(response.body).toEqual(expectedPayload); }); }); diff --git a/packages/backend/test/payloads/current-user.js b/packages/backend/test/mocks/rest/api/v1/users/get-current-user.js similarity index 89% rename from packages/backend/test/payloads/current-user.js rename to packages/backend/test/mocks/rest/api/v1/users/get-current-user.js index 7102f6b4..7bdec2be 100644 --- a/packages/backend/test/payloads/current-user.js +++ b/packages/backend/test/mocks/rest/api/v1/users/get-current-user.js @@ -1,4 +1,4 @@ -const currentUserPayload = (currentUser, role) => { +const getCurrentUserMock = (currentUser, role) => { return { data: { createdAt: currentUser.createdAt.toISOString(), @@ -29,4 +29,4 @@ const currentUserPayload = (currentUser, role) => { }; }; -export default currentUserPayload; +export default getCurrentUserMock; diff --git a/packages/backend/test/payloads/user.js b/packages/backend/test/mocks/rest/api/v1/users/get-user.js similarity index 90% rename from packages/backend/test/payloads/user.js rename to packages/backend/test/mocks/rest/api/v1/users/get-user.js index 02eac3d4..cacfc473 100644 --- a/packages/backend/test/payloads/user.js +++ b/packages/backend/test/mocks/rest/api/v1/users/get-user.js @@ -1,4 +1,4 @@ -const userPayload = (currentUser, role) => { +const getUserMock = (currentUser, role) => { return { data: { createdAt: currentUser.createdAt.toISOString(), @@ -28,4 +28,4 @@ const userPayload = (currentUser, role) => { }; }; -export default userPayload; +export default getUserMock; From a769f7880144049e5693fafda2c9649d68646247 Mon Sep 17 00:00:00 2001 From: Faruk AYDIN Date: Wed, 14 Feb 2024 01:20:29 +0100 Subject: [PATCH 5/5] test: Add tests for api/v1/users API endpoint --- .../api/v1/users/get-users.test.js | 56 +++++++++++++++++++ packages/backend/src/helpers/authorization.js | 4 ++ .../test/mocks/rest/api/v1/users/get-users.js | 38 +++++++++++++ 3 files changed, 98 insertions(+) create mode 100644 packages/backend/src/controllers/api/v1/users/get-users.test.js create mode 100644 packages/backend/test/mocks/rest/api/v1/users/get-users.js diff --git a/packages/backend/src/controllers/api/v1/users/get-users.test.js b/packages/backend/src/controllers/api/v1/users/get-users.test.js new file mode 100644 index 00000000..ed8baeba --- /dev/null +++ b/packages/backend/src/controllers/api/v1/users/get-users.test.js @@ -0,0 +1,56 @@ +import { describe, it, expect, beforeEach } from 'vitest'; +import request from 'supertest'; +import app from '../../../../app'; +import createAuthTokenByUserId from '../../../../helpers/create-auth-token-by-user-id'; +import { createRole } from '../../../../../test/factories/role'; +import { createPermission } from '../../../../../test/factories/permission'; +import { createUser } from '../../../../../test/factories/user'; +import getUsersMock from '../../../../../test/mocks/rest/api/v1/users/get-users'; + +describe('GET /api/v1/users', () => { + let currentUser, currentUserRole, anotherUser, anotherUserRole, token; + + beforeEach(async () => { + currentUserRole = await createRole({ + key: 'currentUser', + name: 'Current user role', + }); + + await createPermission({ + action: 'read', + subject: 'User', + roleId: currentUserRole.id, + }); + + currentUser = await createUser({ + roleId: currentUserRole.id, + fullName: 'Current User', + }); + + anotherUserRole = await createRole({ + key: 'anotherUser', + name: 'Another user role', + }); + + anotherUser = await createUser({ + roleId: anotherUserRole.id, + fullName: 'Another User', + }); + + token = createAuthTokenByUserId(currentUser.id); + }); + + it('should return users data', async () => { + const response = await request(app) + .get('/api/v1/users') + .set('Authorization', token) + .expect(200); + + const expectedResponsePayload = await getUsersMock( + [anotherUser, currentUser], + [anotherUserRole, currentUserRole] + ); + + expect(response.body).toEqual(expectedResponsePayload); + }); +}); diff --git a/packages/backend/src/helpers/authorization.js b/packages/backend/src/helpers/authorization.js index a027dbac..28cefc59 100644 --- a/packages/backend/src/helpers/authorization.js +++ b/packages/backend/src/helpers/authorization.js @@ -3,6 +3,10 @@ const authorizationList = { action: 'read', subject: 'User', }, + '/api/v1/users/': { + action: 'read', + subject: 'User', + }, }; export const authorizeUser = async (request, response, next) => { diff --git a/packages/backend/test/mocks/rest/api/v1/users/get-users.js b/packages/backend/test/mocks/rest/api/v1/users/get-users.js new file mode 100644 index 00000000..73d27474 --- /dev/null +++ b/packages/backend/test/mocks/rest/api/v1/users/get-users.js @@ -0,0 +1,38 @@ +const getUsersMock = async (users, roles) => { + const data = users.map((user) => { + const role = roles.find((r) => r.id === user.roleId); + return { + createdAt: user.createdAt.toISOString(), + email: user.email, + fullName: user.fullName, + id: user.id, + role: role + ? { + createdAt: role.createdAt.toISOString(), + description: role.description, + id: role.id, + isAdmin: role.isAdmin, + key: role.key, + name: role.name, + updatedAt: role.updatedAt.toISOString(), + } + : null, // Fallback to null if role not found + roleId: user.roleId, + trialExpiryDate: user.trialExpiryDate.toISOString(), + updatedAt: user.updatedAt.toISOString(), + }; + }); + + return { + data: data, + meta: { + count: data.length, + currentPage: 1, + isArray: true, + totalPages: 1, + type: 'User', + }, + }; +}; + +export default getUsersMock;