From a9e545ae60a4b4e08194614b20cf03f1ca5e97cf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gerg=C5=91=20Jedlicska?= Date: Thu, 10 Mar 2022 11:33:33 +0100 Subject: [PATCH] fix(server comments): no mandatory stream as comment target resource --- .../server/modules/comments/services/index.js | 10 ++-- .../modules/comments/tests/comments.spec.js | 46 +++++++++++++++---- 2 files changed, 45 insertions(+), 11 deletions(-) diff --git a/packages/server/modules/comments/services/index.js b/packages/server/modules/comments/services/index.js index 4264d5579..97fa72c63 100644 --- a/packages/server/modules/comments/services/index.js +++ b/packages/server/modules/comments/services/index.js @@ -46,19 +46,23 @@ const getCommentLinksForResources = async ( streamId, resources ) => { const objectIds = resources.filter( res => res.resourceType === 'object' ).map( r => r.resourceId ) if ( objectIds.length ) { const streamObjectIds = ( await Objects().where( { streamId } ).whereIn( 'id', objectIds ) ).map( o => o.id ) - commentLinks = commentLinks.filter( link => streamObjectIds.includes( link.resourceId ) ) + // if a comment link is of type object, check if the object belongs to the stream, other types do not need filtering + // since all other types are directly linked to a stream + commentLinks = commentLinks.filter( link => link.resourceType === 'object' ? streamObjectIds.includes( link.resourceId ) : true ) } + // let commentGroups = {} + // for (const link of commentLinks) return commentLinks } module.exports = { async createComment( { userId, input } ) { + if ( input.resources.length < 1 ) throw Error( 'Must specify atleast one resource as the comment target' ) + const streamResources = input.resources.filter( r => r.resourceType === 'stream' ) - if ( streamResources.length < 1 ) throw Error( 'Must specify atleast a stream as the comment target' ) if ( streamResources.length > 1 ) throw Error( 'Commenting on multiple streams is not supported' ) const [ stream ] = streamResources - if ( stream.resourceId !== input.streamId ) throw Error( 'Input streamId doesn\'t match the stream resource.resourceId' ) let comment = { ...input } diff --git a/packages/server/modules/comments/tests/comments.spec.js b/packages/server/modules/comments/tests/comments.spec.js index 4f4401114..4eca10240 100644 --- a/packages/server/modules/comments/tests/comments.spec.js +++ b/packages/server/modules/comments/tests/comments.spec.js @@ -9,7 +9,7 @@ const { createStream } = require( `${appRoot}/modules/core/services/streams` ) const { createCommitByBranchName } = require( `${appRoot}/modules/core/services/commits` ) const { createObject } = require( `${appRoot}/modules/core/services/objects` ) -const { createComment } = require( '../services' ) +const { createComment, getComments } = require( '../services' ) describe( 'Comments @comments', () => { let user = { @@ -46,21 +46,18 @@ describe( 'Comments @comments', () => { commitId2 = await createCommitByBranchName( { streamId: stream.id, branchName: 'main', message: 'first commit', sourceApplication: 'tests', objectId: testObject2.id, authorId: user.id } ) } ) - it( 'Should not be allowed to comment without specifying the stream as a resource', async ( ) => { + it( 'Should not be allowed to comment without specifying atleast one target resource', async ( ) => { return await createComment( { userId: user.id, input: { streamId: stream.id, - resources: [ - { resourceId: commitId1, resourceType: 'commit' }, - { resourceId: testObject1.id, resourceType: 'object' } - ], + resources: [], text: crs( { length: 10 } ), data: { justSome: crs( { length: 10 } ) } } } ) .then( () => { throw new Error( 'This should have been rejected' ) } ) - .catch( error => expect( error.message ).to.be.equal( 'Must specify atleast a stream as the comment target' ) ) + .catch( error => expect( error.message ).to.be.equal( 'Must specify atleast one resource as the comment target' ) ) } ) it( 'Should not be allowed to comment with mismatching streamId and stream resourceId', async ( ) => { return await createComment( { @@ -157,6 +154,12 @@ describe( 'Comments @comments', () => { .catch( error => expect( error.message ).to.equal( 'resource type flux capacitor is not supported as a comment target' ) ) } ) + it( 'Should not be able to comment resources that do not belong to the input streamId', async () => { + // need to check streamId - commit link + // need to check streamId - object link + // need to check streamId - stream match + // need to check comment reply recursively? that sounds too much of an effort + } ) it( 'Should be able to comment on valid resources in any permutation', async () => { //comment on branches too!!! const resourceCombinations = [ @@ -213,8 +216,34 @@ describe( 'Comments @comments', () => { expect( commentId ).to.exist } } ) - it( 'Should not return the same comment multiple times for multi resource comments' ) + it( 'Should not return the same comment multiple times for multi resource comments', async () => { + const localObjectId = await createObject( stream.id, { testObject: 1 } ) + + const commentCount = 3 + for ( let i = 0; i <= commentCount; i++ ) { + await createComment( { + userId: user.id, + input: { + streamId: stream.id, + resources: [ + { resourceId: stream.id, resourceType: 'stream' }, + { resourceId: commitId1, resourceType: 'commit' }, + { resourceId: localObjectId, resourceType: 'object' } + ], + text: crs( { length: 10 } ), + data: { justSome: crs( { length: 10 } ) } + } + } ) + } + + const comments = await getComments( { streamId: stream.id, resources: [ + { resourceId: commitId1, resourceType: 'commit' }, + { resourceId: localObjectId, resourceType: 'object' } + ] } ) + expect( comments.items ).to.have.lengthOf( commentCount ) + } ) it( 'Should handle cursor and limit for queries' ) + it( 'Should handle very and limit for queries' ) it( 'Should return all the referenced resources for a comment' ) it( 'Should be able to edit a comment text and its context???' ) it( 'Should not be allowed to edit a not existing comment' ) @@ -224,4 +253,5 @@ describe( 'Comments @comments', () => { it( 'Should return archived comments if explicitly asked for them' ) it( 'Return value from get comments and get comment should match in data' ) it( 'Should publish events to pubsub, test it by registering a subscriber' ) + it( 'Should be able to write a short novel as comment text' ) } ) \ No newline at end of file