From f1276b03fedb537ee7d5a216cc2a416e7ee480f0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gerg=C5=91=20Jedlicska?= Date: Wed, 9 Mar 2022 12:36:10 +0100 Subject: [PATCH] feat(server): add screenshot to comments --- .../comments/graph/resolvers/comments.js | 2 - .../comments/graph/schemas/comments.gql | 2 + .../migrations/20220222173000_comments.js | 1 + .../server/modules/comments/services/index.js | 12 +- .../comments/tests/comments.graph.spec.js | 0 .../modules/comments/tests/comments.spec.js | 227 ++++++++++++++++++ 6 files changed, 241 insertions(+), 3 deletions(-) create mode 100644 packages/server/modules/comments/tests/comments.graph.spec.js create mode 100644 packages/server/modules/comments/tests/comments.spec.js diff --git a/packages/server/modules/comments/graph/resolvers/comments.js b/packages/server/modules/comments/graph/resolvers/comments.js index c36c2e6ab..78c79f7bc 100644 --- a/packages/server/modules/comments/graph/resolvers/comments.js +++ b/packages/server/modules/comments/graph/resolvers/comments.js @@ -35,7 +35,6 @@ module.exports = { const streamId = parent.resources.filter( r => r.resourceType === 'stream' )[0].resourceId const resources = [ { resourceId: parent.id, resourceType: 'comment' } ] return await getComments( { streamId, resources, limit: args.limit, cursor: args.cursor } ) - // TODO } }, Mutation: { @@ -53,7 +52,6 @@ module.exports = { async commentCreate( parent, args, context, info ) { // TODO: check perms, persist comment - // console.log( '---', args ) await authorizeResolver( context.userId, args.input.streamId, 'stream:reviewer' ) let id = await createComment( { userId: context.userId, input: args.input } ) await pubsub.publish( 'COMMENT_CREATED', { diff --git a/packages/server/modules/comments/graph/schemas/comments.gql b/packages/server/modules/comments/graph/schemas/comments.gql index b9a99b143..39a1c96a1 100644 --- a/packages/server/modules/comments/graph/schemas/comments.gql +++ b/packages/server/modules/comments/graph/schemas/comments.gql @@ -8,6 +8,7 @@ type Comment { id: String! authorId: String! archived: Boolean! + screenshot: String text: String! data: JSONObject! resources: [ResourceIdentifier]! @@ -54,6 +55,7 @@ input CommentCreateInput { resources: [ResourceIdentifierInput]! text: String! data: JSONObject! + screenshot: String } input ReplyCreateInput { diff --git a/packages/server/modules/comments/migrations/20220222173000_comments.js b/packages/server/modules/comments/migrations/20220222173000_comments.js index decce18ed..5dfd63047 100644 --- a/packages/server/modules/comments/migrations/20220222173000_comments.js +++ b/packages/server/modules/comments/migrations/20220222173000_comments.js @@ -6,6 +6,7 @@ exports.up = async ( knex ) => { table.timestamp( 'createdAt' ).defaultTo( knex.fn.now( ) ) table.timestamp( 'updatedAt' ).defaultTo( knex.fn.now( ) ) table.string( 'text' ) + table.text( 'screenshot' ) table.jsonb( 'data' ) table.boolean( 'archived' ).defaultTo( false ).notNullable() table.string( 'parentComment', 10 ).references( 'id' ).inTable( 'comments' ).defaultTo( null ) diff --git a/packages/server/modules/comments/services/index.js b/packages/server/modules/comments/services/index.js index 56b3c7d6d..4264d5579 100644 --- a/packages/server/modules/comments/services/index.js +++ b/packages/server/modules/comments/services/index.js @@ -5,14 +5,16 @@ const knex = require( `${appRoot}/db/knex` ) const Streams = () => knex( 'streams' ) const Objects = () => knex( 'objects' ) +const Branches = ( ) => knex( 'branches' ) const Commits = () => knex( 'commits' ) const Comments = () => knex( 'comments' ) const CommentLinks = () => knex( 'comment_links' ) const persistResourceLinks = async ( commentId, resources ) => - Promise.all( resources.map( res => persistResourceLink( commentId, res ) ) ) + await Promise.all( resources.map( res => persistResourceLink( commentId, res ) ) ) const persistResourceLink = async ( commentId, { resourceId, resourceType } ) => { + //should the resource belonging to the stream stuff be validated here? let query switch ( resourceType ) { case 'stream': @@ -51,6 +53,14 @@ const getCommentLinksForResources = async ( streamId, resources ) => { module.exports = { async createComment( { userId, input } ) { + 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 } delete comment.resources diff --git a/packages/server/modules/comments/tests/comments.graph.spec.js b/packages/server/modules/comments/tests/comments.graph.spec.js new file mode 100644 index 000000000..e69de29bb diff --git a/packages/server/modules/comments/tests/comments.spec.js b/packages/server/modules/comments/tests/comments.spec.js new file mode 100644 index 000000000..4f4401114 --- /dev/null +++ b/packages/server/modules/comments/tests/comments.spec.js @@ -0,0 +1,227 @@ +/* istanbul ignore file */ +const expect = require( 'chai' ).expect +const crs = require( 'crypto-random-string' ) + +const appRoot = require( 'app-root-path' ) +const { beforeEachContext } = require( `${appRoot}/test/hooks` ) +const { createUser } = require( `${appRoot}/modules/core/services/users` ) +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' ) + +describe( 'Comments @comments', () => { + let user = { + name: 'The comment wizard', + email: 'comment@wizard.ry', + password: 'i did not like Rivendel wine :(' + } + + let stream = { + name: 'Commented stream', + description: 'Chit chats over here' + } + + let testObject1 = { + foo: 'bar' + } + + let testObject2 = { + foo: 'barbar', + baz: 123 + } + let commitId1, commitId2 + + before( async () => { + await beforeEachContext() + + user.id = await createUser( user ) + stream.id = await createStream( { ...stream, ownerId: user.id } ) + + testObject1.id = await createObject( stream.id, testObject1 ) + testObject2.id = await createObject( stream.id, testObject2 ) + + commitId1 = await createCommitByBranchName( { streamId: stream.id, branchName: 'main', message: 'first commit', sourceApplication: 'tests', objectId: testObject1.id, authorId: user.id } ) + 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 ( ) => { + return await createComment( { + userId: user.id, + input: { + streamId: stream.id, + resources: [ + { resourceId: commitId1, resourceType: 'commit' }, + { resourceId: testObject1.id, resourceType: 'object' } + ], + 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' ) ) + } ) + it( 'Should not be allowed to comment with mismatching streamId and stream resourceId', async ( ) => { + return await createComment( { + userId: user.id, + input: { + streamId: stream.id, + resources: [ + { resourceId: 'almost the stream.id', resourceType: 'stream' }, + { resourceId: commitId1, resourceType: 'commit' }, + { resourceId: testObject1.id, resourceType: 'object' } + ], + 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( 'Input streamId doesn\'t match the stream resource.resourceId' ) ) + } ) + it( 'Should not be allowed to comment targeting multiple streams as a resource', async ( ) => { + return await createComment( { + userId: user.id, + input: { + streamId: stream.id, + resources: [ + { resourceId: stream.id, resourceType: 'stream' }, + { resourceId: commitId1, resourceType: 'commit' }, + { resourceId: stream.id, resourceType: 'stream' }, + { resourceId: testObject1.id, resourceType: 'object' } + ], + 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( 'Commenting on multiple streams is not supported' ) ) + } ) + it( 'Should not be allowed to comment on non existing resources', async () => { + const nonExistentResources = [ + { + streamId: 'this doesnt exist dummy', + resources: [ + { resourceId: 'this doesnt exist dummy', resourceType: 'stream' }, + ], + text: null, + data: null + }, + { + streamId: stream.id, + resources: [ + { resourceId: stream.id, resourceType: 'stream' }, + { resourceId: 'this doesnt exist dummy', resourceType: 'commit' }, + ], + text: null, + data: null + }, + { + streamId: stream.id, + resources: [ + { resourceId: stream.id, resourceType: 'stream' }, + { resourceId: 'this doesnt exist dummy', resourceType: 'object' }, + ], + text: null, + data: null + }, + { + streamId: stream.id, + resources: [ + { resourceId: stream.id, resourceType: 'stream' }, + { resourceId: 'this doesnt exist dummy', resourceType: 'comment' }, + ], + text: null, + data: null + }, + ] + for ( const input of nonExistentResources ) { + await createComment( { userId: user.id, input } ) + .then( () => { throw new Error( 'This should have been rejected' ) } ) + .catch( error => expect( error.message ).to.contain( ': this doesnt exist dummy doesn\'t exist, you cannot comment on it' ) ) + } + } ) + it( 'Should not be allowed to comment on an non supported resource type', async () => { + await createComment( { + userId: user.id, + input: { + streamId: stream.id, + resources: [ + { resourceId: stream.id, resourceType: 'stream' }, + { resourceId: 'jubbjabb', resourceType: 'flux capacitor' }, + ], + text: crs( { length: 10 } ), + data: { justSome: crs( { length: 10 } ) } + } } ) + .then( () => { throw new Error( 'This should have been rejected' ) } ) + .catch( error => expect( error.message ).to.equal( 'resource type flux capacitor is not supported as a comment target' ) ) + } ) + + it( 'Should be able to comment on valid resources in any permutation', async () => { + //comment on branches too!!! + const resourceCombinations = [ + [ + { resourceId: stream.id, resourceType: 'stream' } + ], + [ + { resourceId: stream.id, resourceType: 'stream' }, + { resourceId: commitId1, resourceType: 'commit' } + ], + [ + { resourceId: stream.id, resourceType: 'stream' }, + { resourceId: commitId1, resourceType: 'commit' }, + { resourceId: testObject1.id, resourceType: 'object' } + ], + [ + // object overlay on object + { resourceId: stream.id, resourceType: 'stream' }, + { resourceId: testObject1.id, resourceType: 'object' }, + { resourceId: testObject2.id, resourceType: 'object' } + ], + [ + // an object overlayed on a commit + { resourceId: stream.id, resourceType: 'stream' }, + { resourceId: commitId1, resourceType: 'commit' }, + { resourceId: testObject2.id, resourceType: 'object' } + ], + [ + // an object overlayed on a commit + { resourceId: stream.id, resourceType: 'stream' }, + { resourceId: commitId1, resourceType: 'commit' }, + { resourceId: testObject1.id, resourceType: 'object' }, + { resourceId: testObject2.id, resourceType: 'object' } + ], + [ + { resourceId: stream.id, resourceType: 'stream' }, + { resourceId: commitId1, resourceType: 'commit' }, + { resourceId: commitId2, resourceType: 'commit' }, + { resourceId: testObject1.id, resourceType: 'object' } + ] + ] + + // yeah i know, Promise.all, but this is easier to debug... + for ( const resources of resourceCombinations ) { + const commentId = await createComment( { + userId: user.id, + input: { + streamId: stream.id, + resources, + text: crs( { length: 10 } ), + data: { justSome: crs( { length: 10 } ) } + } + } ) + expect( commentId ).to.exist + } + } ) + it( 'Should not return the same comment multiple times for multi resource comments' ) + it( 'Should handle cursor 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' ) + it( 'Should be able to archive a comment' ) + it( 'Should not be allowed to archive a not existing comment' ) + it( 'Should not return archived comments in plain queries' ) + 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' ) +} ) \ No newline at end of file