From a9a77872d2ad0fe19caaba717d229e15f7903579 Mon Sep 17 00:00:00 2001 From: Dimitrie Stefanescu Date: Tue, 21 Jul 2020 23:40:42 +0100 Subject: [PATCH] feat(gql): stream users now returns a custom type - no longer polluting the original & many other fixes & squashed bugs (stream counts, other users' streams, etc.) --- .gitignore | 3 +- modules/core/graph/resolvers/commits.js | 14 ++- modules/core/graph/resolvers/streams.js | 16 ++- modules/core/graph/resolvers/user.js | 10 ++ .../graph/schemas/branchesAndCommits.graphql | 12 +- modules/core/graph/schemas/streams.graphql | 12 +- modules/core/graph/schemas/user.graphql | 5 +- modules/core/services/commits.js | 19 +++- modules/core/services/streams.js | 28 +++-- modules/core/tests/graph.spec.js | 107 ++++++++++-------- 10 files changed, 142 insertions(+), 84 deletions(-) diff --git a/.gitignore b/.gitignore index 419b7b442..47792c5f1 100644 --- a/.gitignore +++ b/.gitignore @@ -3,4 +3,5 @@ frontend/node_modules frontend/dist .nyc_output coverage/ -.env \ No newline at end of file +.env +.vscode \ No newline at end of file diff --git a/modules/core/graph/resolvers/commits.js b/modules/core/graph/resolvers/commits.js index 2fff5870b..b8441697e 100644 --- a/modules/core/graph/resolvers/commits.js +++ b/modules/core/graph/resolvers/commits.js @@ -36,15 +36,21 @@ module.exports = { User: { async commits( parent, args, context, info ) { - // TODO - throw new ApolloError( 'not implemented' ) + + let publicOnly = context.userId !== parent.id + let totalCount = await getCommitsTotalCountByUserId( { userId: parent.id } ) + let { commits: items, cursor } = await getCommitsByUserId( { userId: parent.id, limit: args.limit, cursor: args.cursor, publicOnly } ) + + return { items, cursor, totalCount } } }, Branch: { async commits( parent, args, context, info ) { + throw new ApolloError( 'not implemented' ) + } }, @@ -62,7 +68,7 @@ module.exports = { await validateServerRole( context, 'server:user' ) await validateScopes( context.scopes, 'streams:write' ) await authorizeResolver( context.userId, args.commit.streamId, 'stream:contributor' ) - + let commit = await getCommitById( { id: args.commit.id } ) if ( commit.author !== context.userId ) throw new AuthorizationError( 'Only the author of a commit may update it.' ) @@ -82,4 +88,4 @@ module.exports = { return await deleteCommit( { id: args.commit.id } ) } } -} \ No newline at end of file +} diff --git a/modules/core/graph/resolvers/streams.js b/modules/core/graph/resolvers/streams.js index adf71d0fb..a3e353f27 100644 --- a/modules/core/graph/resolvers/streams.js +++ b/modules/core/graph/resolvers/streams.js @@ -25,22 +25,24 @@ module.exports = { } }, Stream: { - async users( parent, args, context, info ) { - let users = await getStreamUsers( parent.id ) + + async collaborators( parent, args, context, info ) { + let users = await getStreamUsers( { streamId: parent.id } ) return users } + }, User: { + async streams( parent, args, context, info ) { // Return only the user's public streams if parent.id !== context.userId let publicOnly = parent.id !== context.userId - - let totalCount = await getUserStreamsCount( { userId: parent.id } ) + let totalCount = await getUserStreamsCount( { userId: parent.id, publicOnly } ) let { cursor, streams } = await getUserStreams( { userId: parent.id, limit: args.limit, cursor: args.cursor, publicOnly: publicOnly } ) - console.log( cursor ) return { totalCount, cursor: cursor, items: streams } } + }, Mutation: { async streamCreate( parent, args, context, info ) { @@ -50,6 +52,7 @@ module.exports = { let id = await createStream( { ...args.stream, ownerId: context.userId } ) return id }, + async streamUpdate( parent, args, context, info ) { await validateServerRole( context, 'server:user' ) await validateScopes( context.scopes, 'streams:write' ) @@ -58,6 +61,7 @@ module.exports = { await updateStream( { streamId: args.stream.id, name: args.stream.name, description: args.stream.description } ) return true }, + async streamDelete( parent, args, context, info ) { await validateServerRole( context, 'server:user' ) await validateScopes( context.scopes, 'streams:write' ) @@ -66,6 +70,7 @@ module.exports = { await deleteStream( { streamId: args.id } ) return true }, + async streamGrantPermission( parent, args, context, info ) { await validateServerRole( context, 'server:user' ) await validateScopes( context.scopes, 'streams:write' ) @@ -75,6 +80,7 @@ module.exports = { return await grantPermissionsStream( { streamId: args.streamId, userId: args.userId, role: args.role.toLowerCase( ) || 'read' } ) }, + async streamRevokePermission( parent, args, context, info ) { await validateServerRole( context, 'server:user' ) await validateScopes( context.scopes, 'streams:write' ) diff --git a/modules/core/graph/resolvers/user.js b/modules/core/graph/resolvers/user.js index e6808373a..a0f225ddb 100644 --- a/modules/core/graph/resolvers/user.js +++ b/modules/core/graph/resolvers/user.js @@ -6,11 +6,14 @@ const { createPersonalAccessToken, createAppToken, revokeToken, revokeTokenById, const { validateServerRole, validateScopes, authorizeResolver } = require( `${appRoot}/modules/shared` ) const setupCheck = require( `${appRoot}/setupcheck` ) const zxcvbn = require( 'zxcvbn' ) + module.exports = { Query: { + async _( ) { return `Ph'nglui mglw'nafh Cthulhu R'lyeh wgah'nagl fhtagn.` }, + async user( parent, args, context, info ) { await validateServerRole( context, 'server:user' ) @@ -25,12 +28,16 @@ module.exports = { return await getUser( args.id || context.userId ) }, + async userPwdStrength( parent, args, context, info ) { let res = zxcvbn( args.pwd ) return { score: res.score, feedback: res.feedback } } + }, + User: { + async email( parent, args, context, info ) { // NOTE: we're redacting the field (returning null) rather than throwing a full error which would invalidate the request. if ( context.userId === parent.id ) { @@ -49,10 +56,13 @@ module.exports = { return null } }, + async role( parent, args, context, info ) { return await getUserRole( parent.id ) } + }, + Mutation: { async userEdit( parent, args, context, info ) { await validateServerRole( context, 'server:user' ) diff --git a/modules/core/graph/schemas/branchesAndCommits.graphql b/modules/core/graph/schemas/branchesAndCommits.graphql index e95347e17..5a62f1c3c 100644 --- a/modules/core/graph/schemas/branchesAndCommits.graphql +++ b/modules/core/graph/schemas/branchesAndCommits.graphql @@ -27,7 +27,7 @@ type Commit { } type CommitCollectionUserNode { - id: String! + commitId: String! referencedObject: String! realObject: Object message: String @@ -44,13 +44,13 @@ type BranchCollection { type CommitCollection { totalCount: Int! cursor: String - commits: [Commit] + items: [Commit] } type CommitCollectionUser { totalCount: Int! cursor: String - commits: [CommitCollectionUserNode] + items: [CommitCollectionUserNode] } extend type Mutation { @@ -88,13 +88,13 @@ input CommitCreateInput { previousCommitIds: [String] } -input CommitUpdateInput { +input CommitUpdateInput { streamId: String! id: String! message: String! } -input CommitDeleteInput { +input CommitDeleteInput { streamId: String! id: String! -} \ No newline at end of file +} diff --git a/modules/core/graph/schemas/streams.graphql b/modules/core/graph/schemas/streams.graphql index 7c9c7c7b1..5354d2fc7 100644 --- a/modules/core/graph/schemas/streams.graphql +++ b/modules/core/graph/schemas/streams.graphql @@ -9,7 +9,7 @@ type Stream { isPublic: Boolean! createdAt: String! updatedAt: String! - users: [ User ]! + collaborators: [ StreamCollaborator ]! } extend type User { @@ -17,10 +17,12 @@ extend type User { All the streams that a user has access to. """ streams( limit: Int! = 20, cursor: String ): StreamCollectionUser - """ - The role this user has on a specific stream (can be populated when accessing a stream's users). - """ - role: String +} + +type StreamCollaborator { + id: String! + name: String! + role: String! } type StreamCollectionUser { diff --git a/modules/core/graph/schemas/user.graphql b/modules/core/graph/schemas/user.graphql index 82c5db1bb..355ec20fd 100644 --- a/modules/core/graph/schemas/user.graphql +++ b/modules/core/graph/schemas/user.graphql @@ -19,9 +19,10 @@ type User { avatar: String verified: Boolean profiles: JSONObject + role: String } -extend type Mutation { +extend type Mutation { """ Edits a user's profile. """ @@ -32,4 +33,4 @@ input UserEditInput { name: String company: String bio: String -} \ No newline at end of file +} diff --git a/modules/core/services/commits.js b/modules/core/services/commits.js index b87c5f15a..8d147d517 100644 --- a/modules/core/services/commits.js +++ b/modules/core/services/commits.js @@ -46,7 +46,7 @@ module.exports = { }, async createCommitByBranchName( { streamId, branchName, objectId, authorId, message, previousCommitIds } ) { - branchName = branchName.toLowerCase() + branchName = branchName.toLowerCase( ) let branches = await getBranchesByStreamId( { streamId: streamId } ) let myBranch = branches.find( b => b.name === branchName ) @@ -75,7 +75,7 @@ module.exports = { }, async getCommitsTotalCountByBranchName( { streamId, branchName } ) { - branchName = branchName.toLowerCase() + branchName = branchName.toLowerCase( ) let branches = await getBranchesByStreamId( { streamId: streamId } ) let myBranch = branches.find( b => b.name === branchName ) @@ -100,11 +100,11 @@ module.exports = { let rows = await query - return { commits: rows, cursor: rows.length > 0 ? rows[ rows.length - 1 ].createdAt : null } + return { commits: rows, cursor: rows.length > 0 ? rows[ rows.length - 1 ].createdAt.toISOString( ) : null } }, async getCommitsByBranchName( { streamId, branchName, limit, cursor } ) { - branchName = branchName.toLowerCase() + branchName = branchName.toLowerCase( ) let branches = await getBranchesByStreamId( { streamId: streamId } ) let myBranch = branches.find( b => b.name === branchName ) @@ -119,6 +119,13 @@ module.exports = { return parseInt( res.count ) }, + /** + * Gets all the commits of a stream. + * @param {[type]} options.streamId [description] + * @param {[type]} options.limit [description] + * @param {[type]} options.cursor [description] + * @return {[type]} [description] + */ async getCommitsByStreamId( { streamId, limit, cursor } ) { limit = limit || 20 let query = StreamCommits( ) @@ -134,7 +141,7 @@ module.exports = { query.orderBy( 'commits.createdAt', 'desc' ).limit( limit ) let rows = await query - return { commits: rows, cursor: rows.length > 0 ? rows[ rows.length - 1 ].createdAt : null } + return { commits: rows, cursor: rows.length > 0 ? rows[ rows.length - 1 ].createdAt.toISOString( ) : null } }, async getCommitsByUserId( { userId, limit, cursor, publicOnly } ) { @@ -157,7 +164,7 @@ module.exports = { query.orderBy( 'commits.createdAt', 'desc' ).limit( limit ) let rows = await query - return { commits: rows, cursor: rows.length > 0 ? rows[ rows.length - 1 ].createdAt : null } + return { commits: rows, cursor: rows.length > 0 ? rows[ rows.length - 1 ].createdAt.toISOString( ) : null } }, async getCommitsTotalCountByUserId( { userId } ) { diff --git a/modules/core/services/streams.js b/modules/core/services/streams.js index 5d85c360c..548f7d734 100644 --- a/modules/core/services/streams.js +++ b/modules/core/services/streams.js @@ -93,9 +93,6 @@ module.exports = { limit = limit || 100 publicOnly = publicOnly !== false //defaults to true if not provided - - if ( publicOnly ) query.isPublic = true - let query = Acl( ) .columns( [ { id: 'streams.id' }, 'name', 'description', 'isPublic', 'createdAt', 'updatedAt' ] ).select( ) .join( 'streams', 'stream_acl.resourceId', 'streams.id' ) @@ -110,17 +107,32 @@ module.exports = { query.orderBy( 'streams.updatedAt', 'desc' ).limit( limit ) let rows = await query - return { streams: rows, cursor: rows.length > 0 ? rows[ rows.length - 1 ].updatedAt.toISOString() : null } + return { streams: rows, cursor: rows.length > 0 ? rows[ rows.length - 1 ].updatedAt.toISOString( ) : null } }, - async getUserStreamsCount( { userId } ) { - let [ res ] = await Acl( ).count( ).where( { userId: userId } ) + async getUserStreamsCount( { userId, publicOnly } ) { + + publicOnly = publicOnly !== false //defaults to true if not provided + + let query = Acl( ).count( ) + .join( 'streams', 'stream_acl.resourceId', 'streams.id' ) + .where( { userId: userId } ) + + if ( publicOnly ) + query.andWhere( 'streams.isPublic', true ) + + let [ res ] = await query return parseInt( res.count ) }, async getStreamUsers( { streamId } ) { - return Acl( ).where( { resourceId: streamId } ) + let query = + Acl( ).columns( { role: 'stream_acl.role' }, 'id', 'name' ).select( ) + .where( { resourceId: streamId } ) .rightJoin( 'users', { 'users.id': 'stream_acl.userId' } ) - .select( 'role', 'username', 'name', 'id' ) + .select( 'stream_acl.role', 'username', 'name', 'id' ) + .orderBy( 'users.id' ) + + return await query } } diff --git a/modules/core/tests/graph.spec.js b/modules/core/tests/graph.spec.js index 4df5d7e59..405727058 100644 --- a/modules/core/tests/graph.spec.js +++ b/modules/core/tests/graph.spec.js @@ -438,18 +438,43 @@ describe( 'GraphQL API Core', ( ) => { expect( res.body.errors ).to.not.exist expect( res.body.data.user.streams.items.length ).to.equal( 3 ) - console.log( res.body.data.user.streams.cursor ) const res2 = await sendRequest( userA.token, { query: `{ user { streams( limit: 3, cursor: "${res.body.data.user.streams.cursor}" ) { totalCount cursor items { id name } } } }` } ) - - console.log( res2.body.errors ) - console.log( res2.body.data ) + expect( res2 ).to.be.json + expect( res2.body.errors ).to.not.exist + expect( res2.body.data.user.streams.items.length ).to.equal( 3 ) let streams = res2.body.data.user.streams.items let s1 = streams.find( s => s.name === 'TS1 (u A) Private UPDATED' ) expect( s1 ).to.exist } ) + it( 'Should retrieve my commits (across all streams)', async ( ) => { + + for ( let i = 10; i < 20; i++ ) { + let c1 = { + message: 'what a message for a first commit', + streamId: ts1, + objectId: objIds[ i ], + branchName: 'master', + } + let res = await sendRequest( userA.token, { query: `mutation( $myCommit: CommitCreateInput! ) { commitCreate( commit: $myCommit ) }`, variables: { myCommit: c1 } } ) + } + + const res = await sendRequest( userA.token, { query: `{ user { commits( limit: 3 ) { totalCount cursor items { commitId message referencedObject } } } }` } ) + expect( res ).to.be.json + expect( res.body.errors ).to.not.exist + expect( res.body.data.user.commits.totalCount ).to.equal( 11 ) + expect( res.body.data.user.commits.cursor ).to.exist + expect( res.body.data.user.commits.items.length ).to.equal( 3 ) + + const res2 = await sendRequest( userA.token, { query: `{ user { commits( limit: 3, cursor: "${res.body.data.user.commits.cursor}") { totalCount cursor items { commitId message referencedObject } } } }` } ) + expect( res2 ).to.be.json + expect( res2.body.errors ).to.not.exist + expect( res2.body.data.user.commits.totalCount ).to.equal( 11 ) + expect( res2.body.data.user.commits.items.length ).to.equal( 3 ) + + } ) } ) describe( 'Different Users` Profile', ( ) => { @@ -479,10 +504,11 @@ describe( 'GraphQL API Core', ( ) => { } ) it( 'Should only retrieve public streams from a different user profile ', async ( ) => { - const res = await sendRequest( token1, { query: `query { user(id:"${userB.id}") { streamCollection { totalCount streams { id name isPublic role }} } }` } ) + const res = await sendRequest( token1, { query: `query { user( id:"${userB.id}" ) { streams { totalCount items { id name isPublic } } } }` } ) + expect( res ).to.be.json expect( res.body.errors ).to.not.exist - expect( res.body.data.user.streamCollection.totalCount ).to.equal( 1 ) + expect( res.body.data.user.streams.totalCount ).to.equal( 1 ) } ) } ) @@ -491,48 +517,24 @@ describe( 'GraphQL API Core', ( ) => { let retrievedStream - it( 'Should fully retrieve a stream', async ( ) => { - const res = await sendRequest( userA.token, { query: `query { - stream(id:"${ts1}") { - id - name - createdAt - updatedAt - clonedFrom { - id - } - role - commits(offset:0 limit:100) { - totalCount - commits { - id - description - } - } - tags(offset:0 limit: 10) { - totalCount - tags { + it( 'Should retrieve a stream', async ( ) => { + const res = await sendRequest( userA.token, { query: ` + query { + stream(id:"${ts1}") { id name - commit { + createdAt + updatedAt + collaborators { id - description + name + role } } - } - branches(offset:0 limit: 10 ) { - totalCount - branches { - id - name - } - } - users { - name - role - } - } - }` } ) + }` } ) + + console.log( res.body.errors ) + console.log( res.body.data.stream.collaborators ) expect( res ).to.be.json expect( res.body.errors ).to.not.exist @@ -541,10 +543,13 @@ describe( 'GraphQL API Core', ( ) => { retrievedStream = stream expect( stream.name ).to.equal( 'TS1 (u A) Private UPDATED' ) - expect( stream.tags.totalCount ).to.equal( 2 ) - expect( stream.branches.totalCount ).to.equal( 1 ) - expect( stream.commits.totalCount ).to.equal( 2 ) - expect( stream.users ).to.have.lengthOf( 2 ) + expect( stream.collaborators ).to.have.lengthOf( 2 ) + expect( stream.collaborators[ 0 ].role ).to.equal( 'stream:owner' ) + expect( stream.collaborators[ 1 ].role ).to.equal( 'stream:contributor' ) + } ) + + it( 'should retrieve all stream branches', async ( ) => { + assert.fail( 'todo' ) } ) it( 'should retrieve a stream branch', async ( ) => { @@ -559,6 +564,14 @@ describe( 'GraphQL API Core', ( ) => { } ) + it( 'should retrieve a branch`s commits', async ( ) => { + assert.fail( 'todo' ) + } ) + + it( 'should retrieve all stream commits', async ( ) => { + assert.fail( 'todo' ) + } ) + it( 'should retrieve a stream commit', async ( ) => { const res = await sendRequest( userA.token, { query: `query { stream(id:"${ts1}") { commit(id:"${c2.id}") { id description data } } }` } ) expect( res ).to.be.json