huanld/speckle-server
1
0
Fork 0
Code Issues Pull Requests Actions 1 Packages Projects Releases Wiki Activity

fix(server comments): fix pagination time confilct js <-> postgres

Browse Source
This commit is contained in:
Gergő Jedlicska
2022-03-11 10:31:59 +01:00
parent a9e545ae60
commit 519ea2ec2c
4 changed files with 82 additions and 19 deletions
Download Patch File Download Diff File Expand all files Collapse all files
packages/server/modules/comments/graph/resolvers/comments.js
+2 -2
View File
@@ -69,8 +69,8 @@ module.exports = {
await authorizeResolver( context.userId, args.input.streamId, 'stream:reviewer' )
// the reply also has to be linked to the stream, for the recursive reply lookup to work
let input = { ...args.input, resources: [
{ id: args.input.parentComment, type: 'comment' },
{ id: args.input.streamId, type: 'stream' }
{ resourceId: args.input.parentComment, resourceType: 'comment' },
{ resourceId: args.input.streamId, resourceType: 'stream' }
] }
let id = await createComment( { userId: context.userId, input } )
await pubsub.publish( 'COMMENT_REPLY_CREATED', {
packages/server/modules/comments/migrations/20220222173000_comments.js
+3 -2
View File
@@ -3,8 +3,9 @@ exports.up = async ( knex ) => {
await knex.schema.createTable( 'comments', table => {
table.string( 'id', 10 ).primary( )
table.string( 'authorId', 10 ).references( 'id' ).inTable( 'users' ).notNullable().index( )
table.timestamp( 'createdAt' ).defaultTo( knex.fn.now( ) )
table.timestamp( 'updatedAt' ).defaultTo( knex.fn.now( ) )
// table.timestamp( 'createdAt' ).defaultTo( knex.fn.now( ) )
table.specificType( 'createdAt', 'TIMESTAMPTZ(3)' ).defaultTo( knex.fn.now( ) )
table.specificType( 'updatedAt', 'TIMESTAMPTZ(3)' ).defaultTo( knex.fn.now( ) )
table.string( 'text' )
table.text( 'screenshot' )
table.jsonb( 'data' )
packages/server/modules/comments/services/index.js
+19 -5
View File
@@ -50,9 +50,21 @@ const getCommentLinksForResources = async ( streamId, resources ) => {
// 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
// group comment links by comment ids, so that the resources can be filtered below
let commentGroups = {}
for ( const link of commentLinks ) {
if ( !( link.commentId in commentGroups ) ) commentGroups[link.commentId] = []
commentGroups[link.commentId].push( link.resourceId )
}
const relevantCommentIds = Object
.keys( commentGroups )
.filter(
// make sure, that the given comment targets exactly the same set of resources, as the input requested
commentId => commentGroups[commentId].length === resourceIds.length && resourceIds.every( resId => commentGroups[commentId].includes( resId ) )
)
return commentLinks.filter( l => relevantCommentIds.includes( l.commentId ) )
}
module.exports = {
@@ -97,8 +109,10 @@ module.exports = {
const commentLinks = await getCommentLinksForResources( streamId, resources )
const relevantComments = [ ...new Set( commentLinks.map( l => l.commentId ) ) ]
let query = Comments().whereIn( 'id', relevantComments ).orderBy( 'createdAt' )
if ( cursor ) query = query.where( 'createdAt', '>', cursor )
let items = await query.limit( limit )
if ( cursor ) query = query.where( 'createdAt', '>', cursor.toISOString() )
const defaultLimit = 100
let items = await query.limit( limit ?? defaultLimit )
if ( items.length ) {
cursor = items[items.length - 1].createdAt
} else {
packages/server/modules/comments/tests/comments.spec.js
+58 -10
View File
@@ -59,8 +59,12 @@ describe( 'Comments @comments', () => {
.then( () => { throw new Error( 'This should have been rejected' ) } )
.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( {
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
await createComment( {
userId: user.id,
input: {
streamId: stream.id,
@@ -75,6 +79,9 @@ describe( 'Comments @comments', () => {
} )
.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' ) )
//add the checks from above
expect( 1 ).to.equal( 2 )
} )
it( 'Should not be allowed to comment targeting multiple streams as a resource', async ( ) => {
return await createComment( {
@@ -154,12 +161,6 @@ 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 = [
@@ -220,7 +221,7 @@ describe( 'Comments @comments', () => {
const localObjectId = await createObject( stream.id, { testObject: 1 } )
const commentCount = 3
for ( let i = 0; i <= commentCount; i++ ) {
for ( let i = 0; i < commentCount; i++ ) {
await createComment( {
userId: user.id,
input: {
@@ -242,12 +243,59 @@ describe( 'Comments @comments', () => {
] } )
expect( comments.items ).to.have.lengthOf( commentCount )
} )
it( 'Should handle cursor and limit for queries' )
it( 'Should handle cursor and limit for queries', async ( ) => {
const localObjectId = await createObject( stream.id, { testObject: 'something completely different' } )
let createdComments = []
const commentCount = 10
for ( let i = 0; i < commentCount; i++ ) {
createdComments.push( 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 } ) }
}
} ) )
await new Promise( resolve => setTimeout( resolve, 500 ) )
}
let comments = await getComments( {
streamId: stream.id,
resources: [
{ resourceId: commitId1, resourceType: 'commit' },
{ resourceId: localObjectId, resourceType: 'object' }
],
limit : 2
} )
expect( comments.items ).to.have.lengthOf( 2 )
expect( createdComments.slice( 0, 2 ) ).deep.to.equal( comments.items.map( c => c.id ) )
const cursor = comments.items[1].createdAt
comments = await getComments( {
streamId: stream.id,
resources: [
{ resourceId: commitId1, resourceType: 'commit' },
{ resourceId: localObjectId, resourceType: 'object' }
],
limit : 2,
cursor
} )
expect( comments.items ).to.have.lengthOf( 2 )
expect( createdComments.slice( 2, 4 ) ).deep.to.equal( comments.items.map( c => c.id ) )
} )
it( 'Should handle very and limit for queries' )
it( 'Should properly return replies for a comment' )
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( 'Replies to archived comment should be archived, when a parent is archived' )
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' )