Merge branch 'dev'

CHANGELOG.md

@@ -15,6 +15,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 ### Changed
 - When inviting someone to a group that has Join Questions new members are now asked to answer the join questions before being let in to the group even when invited by join link or email address.
 
+### Fixed
+- Bug that allowed people to message someone they don't share a group with.
+
 ## [5.6.2] - 2023-12-29
 
 ### Changed

api/graphql/index.js

@@ -376,7 +376,7 @@ export function makeMutations (expressContext, userId, isAdmin, fetchOne) {
     expireInvitation: (root, { invitationId }) =>
       expireInvitation(userId, invitationId),
 
-    findOrCreateThread: (root, { data }) => findOrCreateThread(userId, data),
+    findOrCreateThread: (root, { data }) => findOrCreateThread(userId, data.participantIds),
 
     findOrCreateLinkPreviewByUrl: (root, { data }) => findOrCreateLinkPreviewByUrl(data),
 

api/graphql/mutations/index.js

@@ -1,8 +1,5 @@
 const { GraphQLYogaError } = require('@graphql-yoga/node')
 import { isEmpty, mapKeys, pick, snakeCase, size, trim } from 'lodash'
-import underlyingFindOrCreateThread, {
-  validateThreadData
-} from '../../models/post/findOrCreateThread'
 import convertGraphqlData from './convertGraphqlData'
 
 export {
@@ -110,6 +107,7 @@ export {
   createZapierTrigger,
   deleteZapierTrigger
 } from './zapier'
+export { default as findOrCreateThread } from '../../models/post/findOrCreateThread'
 
 export async function updateMe (sessionId, userId, changes) {
   const user = await User.find(userId)
@@ -129,11 +127,6 @@ export async function leaveGroup (userId, groupId) {
   return groupId
 }
 
-export function findOrCreateThread (userId, data) {
-  return validateThreadData(userId, data)
-  .then(() => underlyingFindOrCreateThread(userId, data.participantIds))
-}
-
 export async function findOrCreateLinkPreviewByUrl ({ url }) {
   const preview = await LinkPreview.find(url)
 

api/models/GroupMembership.js

@@ -3,7 +3,6 @@ import { isEmpty } from 'lodash'
 import {
   whereId
 } from './group/queryUtils'
-import GroupJoinQuestionAnswer from './GroupJoinQuestionAnswer'
 
 module.exports = bookshelf.Model.extend(Object.assign({
   tableName: 'group_memberships',

api/models/User.js

@@ -1,7 +1,6 @@
 /* globals RedisClient */
 import bcrypt from 'bcrypt'
 import crypto from 'crypto'
-const { GraphQLYogaError } = require('@graphql-yoga/node')
 import { has, isEmpty, merge, omit, pick, intersectionBy } from 'lodash'
 import fetch from 'node-fetch'
 import { v4 as uuidv4 } from 'uuid'
@@ -10,7 +9,7 @@ import { Validators } from 'hylo-shared'
 import HasSettings from './mixins/HasSettings'
 import { findThread } from './post/findOrCreateThread'
 import { generateHyloJWT } from '../../lib/HyloJWT'
-import GroupJoinQuestionAnswer from './GroupJoinQuestionAnswer'
+const { GraphQLYogaError } = require('@graphql-yoga/node')
 
 module.exports = bookshelf.Model.extend(merge({
   tableName: 'users',

api/models/group/queryUtils.js

@@ -1,4 +1,3 @@
-import { getDataTypeForModel } from './DataType'
 import { castArray, has } from 'lodash'
 
 // handle a single entity or a list of entity or ids; do nothing if no ids or

api/models/post/findOrCreateThread.js

@@ -1,40 +1,41 @@
-const { GraphQLYogaError } = require('@graphql-yoga/node')
 import { pick } from 'lodash'
-import { map, uniq } from 'lodash/fp'
-import { isFollowing } from '../group/queryUtils'
+import { uniq } from 'lodash/fp'
+import { personFilter } from '../../graphql/filters'
+const { GraphQLYogaError } = require('@graphql-yoga/node')
 
 export function findThread (userIds) {
   return Post.havingExactFollowers(userIds)
-    .query(q => q.where("posts_users.following", true).where({ type: Post.Type.THREAD }))
+    .query(q => q.where('posts_users.following', true).where({ type: Post.Type.THREAD }))
     .fetch()
 }
 
-export default function findOrCreateThread (userId, participantIds) {
-  return findThread(uniq([userId].concat(participantIds)))
-  .then(post => post || createThread(userId, uniq(participantIds)))
+export default async function findOrCreateThread (userId, participantIds, skipValidation = false) {
+  const post = await findThread(uniq([userId].concat(participantIds)))
+  if (post) return post
+  if (skipValidation || await validateThreadData(userId, participantIds)) {
+    return createThread(userId, uniq(participantIds))
+  }
 }
 
 export async function createThread (userId, participantIds) {
   const attrs = await setupNewThreadAttrs(userId)
   let thread
   await bookshelf.transaction(async trx => {
-    thread = await Post.create(attrs, {transacting: trx})
-    await afterSavingThread(thread, {participantIds, transacting: trx})
+    thread = await Post.create(attrs, { transacting: trx })
+    await afterSavingThread(thread, { participantIds, transacting: trx })
   })
   return thread
 }
 
-export function validateThreadData (userId, data) {
-  const { participantIds } = data
+export async function validateThreadData (userId, participantIds) {
   if (!(participantIds && participantIds.length)) {
     throw new GraphQLYogaError("participantIds can't be empty")
   }
-  const checkForSharedGroup = id =>
-    Group.inSameGroup([userId, id])
-    .then(doesShare => {
-      if (!doesShare) throw new GraphQLYogaError(`no shared groups with user ${id}`)
-    })
-  return Promise.all(map(checkForSharedGroup, participantIds))
+  const validParticipantIds = await personFilter(userId)(User.where('id', 'in', uniq(participantIds))).fetchAll()
+  if (validParticipantIds.length !== participantIds.length) {
+    throw new GraphQLYogaError("Cannot message a participant who doesn't share a group")
+  }
+  return true
 }
 
 function setupNewThreadAttrs (userId) {

api/models/post/findOrCreateThread.test.js

@@ -2,29 +2,29 @@ import findOrCreateThread, { validateThreadData } from './findOrCreateThread'
 import factories from '../../../test/setup/factories'
 
 describe('findOrCreateThread', () => {
-  var u1, u2, u3
+  let u1, u2, u3
   before(async () => {
     u1 = await factories.user().save()
     u2 = await factories.user().save()
     u3 = await factories.user().save()
   })
 
   it('finds or creates a thread', async () => {
-    let thread = await findOrCreateThread(u1.id, [u1.id, u2.id, u3.id])
+    let thread = await findOrCreateThread(u1.id, [u1.id, u2.id, u3.id], true)
     thread = await Post.find(thread.id)
     expect(await thread.followers().fetch().then(x => x.length)).to.equal(3)
 
-    let thread2 = await findOrCreateThread(u2.id, [u1.id, u2.id, u3.id])
+    const thread2 = await findOrCreateThread(u2.id, [u1.id, u2.id, u3.id], true)
     expect(thread2.id).to.equal(thread.id)
 
-    let thread3 = await findOrCreateThread(u2.id, [u2.id, u3.id])
+    const thread3 = await findOrCreateThread(u2.id, [u2.id, u3.id], true)
     expect(thread3.id).not.to.equal(thread.id)
     expect(await thread3.followers().fetch().then(x => x.length)).to.equal(2)
   })
 })
 
 describe('validateThreadData', () => {
-  var user, userSharingGroup, userNotInGroup, group
+  let user, userSharingGroup, userNotInGroup, group
 
   before(async () => {
     group = await factories.group().save()
@@ -35,19 +35,29 @@ describe('validateThreadData', () => {
     await userSharingGroup.joinGroup(group)
   })
 
-  it('fails if no participantIds are provided', () => {
-    const fn = () => validateThreadData(user.id, [])
-    expect(fn).to.throw(/participantIds can't be empty/)
+  it('fails if no participantIds are provided', async () => {
+    let err
+    try {
+      await validateThreadData(user.id, [])
+    } catch (error) {
+      err = error
+    }
+    expect(err.message).to.equal("participantIds can't be empty")
   })
-  it('fails if there is a participantId for a user the creator shares no communities with', () => {
-    const data = {participantIds: [userSharingGroup.id, userNotInGroup.id]}
-    return validateThreadData(user.id, data)
-    .catch(function (e) {
-      expect(e.message).to.equal(`no shared communities with user ${userNotInGroup.id}`)
-    })
+
+  it('fails if there is a participantId for a user the creator shares no groups with', async () => {
+    const participantIds = [userSharingGroup.id, userNotInGroup.id]
+    let err
+    try {
+      await validateThreadData(user.id, participantIds)
+    } catch (error) {
+      err = error
+    }
+    expect(err.message).to.equal("Cannot message a participant who doesn't share a group")
   })
-  it('continue the promise chain if user shares group with all participants', () => {
-    const data = {participantIds: [userSharingGroup.id]}
-    expect(validateThreadData(user.id, data)).to.respondTo('then')
+
+  it('returns true if user shares group with all participants', async () => {
+    const participantIds = [userSharingGroup.id]
+    expect(await validateThreadData(user.id, participantIds)).to.equal(true)
   })
 })