From 56a5984038b195c928e78efe9ec8c373081e947c Mon Sep 17 00:00:00 2001 From: Pranav Kakaraparti Date: Mon, 9 Dec 2024 04:54:15 -0500 Subject: [PATCH 1/3] feat: update trust relationship ordering --- server/handlers/walletHandler/index.js | 40 ++++++++++++++++-------- server/models/Trust.js | 31 +++++++++++++------ server/repositories/TrustRepository.js | 42 +++++++++++++++++++------- server/services/TrustService.js | 40 ++++++++++++++++++------ 4 files changed, 110 insertions(+), 43 deletions(-) diff --git a/server/handlers/walletHandler/index.js b/server/handlers/walletHandler/index.js index d0159500..3d8b0abb 100644 --- a/server/handlers/walletHandler/index.js +++ b/server/handlers/walletHandler/index.js @@ -74,30 +74,44 @@ const walletGetTrustRelationships = async (req, res) => { }); const validatedQuery = await walletGetTrustRelationshipsSchema.validateAsync( req.query, - { abortEarly: false }, + { + abortEarly: false, + }, ); const walletService = new WalletService(); const { wallet_id: walletId } = validatedParams; const { wallet_id: loggedInWalletId } = req; - const sortBy = 'created_at'; - const orderBy = 'desc'; - const { state, type, request_type, limit, offset, sort_by, order, search } = validatedQuery; + const sortBy = 'created_at'; + const orderBy = 'desc'; + const { + state, + type, + request_type, + limit, + offset, + sort_by, + order, + search, + } = validatedQuery; - const { wallets:managedWallets } = await walletService.getAllWallets( + const { wallets: managedWallets } = await walletService.getAllWallets( loggedInWalletId, { - limit:'10', - offset:'0', + limit: '1000', + offset: '0', }, null, sortBy, orderBy, null, - null + null, ); const trustService = new TrustService(); - const {result: trust_relationships, count: total} = await trustService.getTrustRelationships( + const { + result: trust_relationships, + count: total, + } = await trustService.getTrustRelationships( loggedInWalletId, managedWallets, { @@ -105,11 +119,11 @@ const walletGetTrustRelationships = async (req, res) => { state, type, request_type, - limit, - offset, - sort_by, + limit, + offset, + sort_by, order, - search + search, }, ); res.status(200).json({ diff --git a/server/models/Trust.js b/server/models/Trust.js index 632573e4..821de118 100644 --- a/server/models/Trust.js +++ b/server/models/Trust.js @@ -45,7 +45,7 @@ class Trust { */ async getTrustRelationships({ walletId, - managedWallets=[], + managedWallets = [], state, type, request_type, @@ -55,13 +55,14 @@ class Trust { search, order, }) { - const managedWalletIds = managedWallets.map(wallet => wallet.id); + const managedWalletIds = managedWallets.map((wallet) => wallet.id); + const orConditions = [ { actor_wallet_id: walletId }, { target_wallet_id: walletId }, { originator_wallet_id: walletId }, ]; - + managedWalletIds.forEach((managedWalletId) => { orConditions.push({ actor_wallet_id: managedWalletId }); orConditions.push({ target_wallet_id: managedWalletId }); @@ -92,9 +93,19 @@ class Trust { ], }); } - return this._trustRepository.getByFilter(filter, { offset, limit, sort_by, order }); + return this._trustRepository.getByFilter( + filter, + { + offset, + limit, + sort_by, + order, + }, + walletId, + managedWalletIds, + ); } - + async getTrustRelationshipsCount({ walletId, state, type, request_type }) { const filter = Trust.getTrustRelationshipFilter({ walletId, @@ -235,8 +246,8 @@ class Trust { // check if I (current wallet) can add a new trust like this async checkDuplicateRequest({ walletId, trustRelationship }) { let trustRelationships = await this.getTrustRelationships({ walletId }); - if (trustRelationships.result) { - trustRelationships = trustRelationships.result; + if (trustRelationships.result) { + trustRelationships = trustRelationships.result; } if ( trustRelationship.type === @@ -380,8 +391,10 @@ class Trust { async updateTrustState(trustRelationship, state) { const trustRelationshipToUpdate = { ...trustRelationship }; - const now = new Date(); - const formattedDate = `${(now.getMonth() + 1).toString().padStart(2, '0')}/${now + const now = new Date(); + const formattedDate = `${(now.getMonth() + 1) + .toString() + .padStart(2, '0')}/${now .getDate() .toString() .padStart(2, '0')}/${now.getFullYear()}`; diff --git a/server/repositories/TrustRepository.js b/server/repositories/TrustRepository.js index 24614392..9202de87 100644 --- a/server/repositories/TrustRepository.js +++ b/server/repositories/TrustRepository.js @@ -37,7 +37,12 @@ class TrustRepository extends BaseRepository { return list; } - async getByFilter(filter, limitOptions) { + async getByFilter( + filter, + limitOptions = {}, + loggedInWalletId = '', + managedWalletIds = [], + ) { let promise = this._session .getDB() .select( @@ -68,15 +73,7 @@ class TrustRepository extends BaseRepository { ) .where((builder) => this.whereBuilder(filter, builder)); - const count = await this._session.getDB().from(promise.as('p')).count('*'); - - if (limitOptions && limitOptions.limit) { - promise = promise.limit(limitOptions.limit); - } - - if (limitOptions && limitOptions.offset) { - promise = promise.offset(limitOptions.offset); - } + const count = await this._session.getDB().from(promise.as('p')).count('*'); let order = 'desc'; let column = 'created_at'; @@ -98,7 +95,30 @@ class TrustRepository extends BaseRepository { promise = promise.offset(limitOptions.offset); } } - promise = promise.orderBy(column, order); + + // order by priority (which is requested state) + // target is current wallet or one of its managed wallets + if (managedWalletIds.length > 0 && loggedInWalletId) { + promise = promise.select( + this._session.getDB().raw( + `CASE + WHEN wallet_trust.state = 'requested' AND + (wallet_trust.target_wallet_id = ? OR wallet_trust.target_wallet_id IN (${managedWalletIds + .map(() => '?') + .join(',')})) THEN 1 + ELSE 0 + END AS priority`, + [loggedInWalletId, ...managedWalletIds], + ), + ); + promise = promise.orderBy([ + { column: 'priority', order: 'desc' }, + { column, order }, // secondary + ]); + } else { + // normal ordering for other endpoints + promise = promise.orderBy(column, order); + } const result = await promise; diff --git a/server/services/TrustService.js b/server/services/TrustService.js index 9d913701..18512fd1 100644 --- a/server/services/TrustService.js +++ b/server/services/TrustService.js @@ -16,7 +16,17 @@ class TrustService { async getTrustRelationships( loggedInWalletId, managedWallets, - { walletId, state, type, request_type, offset, limit,sort_by, order, search }, + { + walletId, + state, + type, + request_type, + offset, + limit, + sort_by, + order, + search, + }, ) { // check if wallet exists first // throws error if no wallet matching walletId exists @@ -43,7 +53,7 @@ class TrustService { limit, sort_by, order, - search + search, }); // const count = await this._trust.getTrustRelationshipsCount({ @@ -58,7 +68,13 @@ class TrustService { // limit and offset not feasible using the current implementation // except if done manually or coming up with a single query - async getAllTrustRelationships({ walletId, state, type, request_type, search }) { + async getAllTrustRelationships({ + walletId, + state, + type, + request_type, + search, + }) { const walletModel = new Wallet(this._session); const { wallets } = await walletModel.getAllWallets( walletId, @@ -72,13 +88,17 @@ class TrustService { const managedWallets = []; await Promise.all( wallets.map(async (w) => { - const trustRelationships = await this.getTrustRelationships(walletId, managedWallets,{ - walletId: w.id, - state, - type, - request_type, - search - }); + const trustRelationships = await this.getTrustRelationships( + walletId, + managedWallets, + { + walletId: w.id, + state, + type, + request_type, + search, + }, + ); alltrustRelationships.push(...trustRelationships.result); }), ); From 3fadb9559755268f6d2c8ae1b6752dcf99a63247 Mon Sep 17 00:00:00 2001 From: Pranav Kakaraparti Date: Mon, 9 Dec 2024 04:54:24 -0500 Subject: [PATCH 2/3] fix: update tests --- server/models/Trust.spec.js | 79 ++++++++++++++++++++++--------------- 1 file changed, 47 insertions(+), 32 deletions(-) diff --git a/server/models/Trust.spec.js b/server/models/Trust.spec.js index 9b804e6f..3ef90d10 100644 --- a/server/models/Trust.spec.js +++ b/server/models/Trust.spec.js @@ -27,19 +27,19 @@ describe('Trust Model', () => { describe('getTrustRelationships', () => { const walletId = uuid(); - const managedWallets = [{id: '90f8b2ab-c101-405d-922a-0a64dbe64ab6'}]; - const managedWalletIds = managedWallets.map(wallet => wallet.id); - const orConditions = [ - { actor_wallet_id: walletId }, - { target_wallet_id: walletId }, - { originator_wallet_id: walletId }, - ]; - - managedWalletIds.forEach((managedWalletId) => { - orConditions.push({ actor_wallet_id: managedWalletId }); - orConditions.push({ target_wallet_id: managedWalletId }); - orConditions.push({ originator_wallet_id: managedWalletId }); - }); + const managedWallets = [{ id: '90f8b2ab-c101-405d-922a-0a64dbe64ab6' }]; + const managedWalletIds = managedWallets.map((wallet) => wallet.id); + const orConditions = [ + { actor_wallet_id: walletId }, + { target_wallet_id: walletId }, + { originator_wallet_id: walletId }, + ]; + + managedWalletIds.forEach((managedWalletId) => { + orConditions.push({ actor_wallet_id: managedWalletId }); + orConditions.push({ target_wallet_id: managedWalletId }); + orConditions.push({ originator_wallet_id: managedWalletId }); + }); const filter = { and: [ { @@ -50,7 +50,7 @@ describe('Trust Model', () => { it('should get relationships', async () => { trustRepositoryStub.getByFilter.resolves(['relationship1']); - + const result = await trustModel.getTrustRelationships({ managedWallets, walletId, @@ -59,12 +59,17 @@ describe('Trust Model', () => { offset: 1, }); expect(result).eql(['relationship1']); - expect(trustRepositoryStub.getByFilter).calledOnceWithExactly(filter, { - limit: 10, - offset: 1, - order: undefined, - sort_by: undefined - }); + expect(trustRepositoryStub.getByFilter).calledOnceWithExactly( + filter, + { + limit: 10, + offset: 1, + order: undefined, + sort_by: undefined, + }, + walletId, + managedWalletIds, + ); }); it('should get relationships -- state', async () => { @@ -85,8 +90,10 @@ describe('Trust Model', () => { limit: 10, offset: 1, order: undefined, - sort_by: undefined + sort_by: undefined, }, + walletId, + managedWalletIds, ); }); @@ -109,8 +116,10 @@ describe('Trust Model', () => { limit: 10, offset: 11, order: undefined, - sort_by: undefined + sort_by: undefined, }, + walletId, + managedWalletIds, ); }); @@ -133,8 +142,10 @@ describe('Trust Model', () => { limit: 101, offset: 1, order: undefined, - sort_by: undefined + sort_by: undefined, }, + walletId, + managedWalletIds, ); }); @@ -161,8 +172,10 @@ describe('Trust Model', () => { limit: 100, offset: 0, order: undefined, - sort_by: undefined + sort_by: undefined, }, + walletId, + managedWalletIds, ); }); }); @@ -739,13 +752,14 @@ describe('Trust Model', () => { it('updateTrustState', async () => { trustRepositoryStub.update.resolves({ status: 'updated' }); - const now = new Date(); - const formattedDate = `${(now.getMonth() + 1).toString().padStart(2, '0')}/${now + const now = new Date(); + const formattedDate = `${(now.getMonth() + 1) + .toString() + .padStart(2, '0')}/${now .getDate() .toString() .padStart(2, '0')}/${now.getFullYear()}`; - const result = await trustModel.updateTrustState( { id: 'trustId', @@ -767,7 +781,7 @@ describe('Trust Model', () => { expect(trustRepositoryStub.update).calledOnceWithExactly({ id: 'trustId', state: 'new state', - updated_at: formattedDate + updated_at: formattedDate, }); }); @@ -910,7 +924,7 @@ describe('Trust Model', () => { }); it('should error out -- no permission to accept', async () => { - trustRepositoryStub.getByFilter.resolves({count: 0, result: []}); + trustRepositoryStub.getByFilter.resolves({ count: 0, result: [] }); const trustRelationshipId = uuid(); const walletId = uuid(); @@ -938,9 +952,10 @@ describe('Trust Model', () => { const trustRelationshipId = uuid(); const walletId = uuid(); - trustRepositoryStub.getByFilter.resolves({count: 1, result:[ - { originator_wallet_id: walletId, id: trustRelationshipId }, - ]}); + trustRepositoryStub.getByFilter.resolves({ + count: 1, + result: [{ originator_wallet_id: walletId, id: trustRelationshipId }], + }); updateTrustStateStub.resolves('state cancelled'); const result = await trustModel.cancelTrustRequest({ trustRelationshipId, From c123015937e2df2ef7be65149dca49e45c78e799 Mon Sep 17 00:00:00 2001 From: Pranav Kakaraparti Date: Mon, 9 Dec 2024 19:12:09 -0500 Subject: [PATCH 3/3] fix: record duplication issue, priority column --- server/repositories/TrustRepository.js | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/server/repositories/TrustRepository.js b/server/repositories/TrustRepository.js index 9202de87..d96ccb83 100644 --- a/server/repositories/TrustRepository.js +++ b/server/repositories/TrustRepository.js @@ -96,7 +96,8 @@ class TrustRepository extends BaseRepository { } } - // order by priority (which is requested state) + // order by new column priority + // priority is 1 when state is requested and // target is current wallet or one of its managed wallets if (managedWalletIds.length > 0 && loggedInWalletId) { promise = promise.select( @@ -114,13 +115,18 @@ class TrustRepository extends BaseRepository { promise = promise.orderBy([ { column: 'priority', order: 'desc' }, { column, order }, // secondary + { column: 'id', order }, // tertiary sort, prevent duplicate records among pages ]); } else { // normal ordering for other endpoints promise = promise.orderBy(column, order); } - const result = await promise; + let result = await promise; + + // remove priority column from result + // eslint-disable-next-line no-unused-vars + result = result.map(({ priority, ...rest }) => rest); return { result, count: +count[0].count }; }