From 67ab1607dcf71eb0ac6c1cfa0749c5f82dea5d72 Mon Sep 17 00:00:00 2001 From: Fabis Date: Tue, 1 Oct 2024 15:28:02 +0100 Subject: [PATCH 01/12] chore(server): core IoC 18 - updateCommitAndNotifyFactory --- .../modules/core/domain/commits/operations.ts | 27 ++- .../modules/core/domain/commits/types.ts | 3 + .../modules/core/graph/resolvers/commits.js | 31 ++- .../modules/core/graph/resolvers/versions.ts | 33 ++- packages/server/modules/core/loaders.ts | 3 +- .../modules/core/repositories/commits.ts | 85 ++++---- .../core/services/commit/management.ts | 203 +++++++++--------- .../server/modules/core/services/commits.js | 9 - .../server/modules/core/tests/commits.spec.js | 45 +++- 9 files changed, 273 insertions(+), 166 deletions(-) diff --git a/packages/server/modules/core/domain/commits/operations.ts b/packages/server/modules/core/domain/commits/operations.ts index eae40d20b..7d31c47ce 100644 --- a/packages/server/modules/core/domain/commits/operations.ts +++ b/packages/server/modules/core/domain/commits/operations.ts @@ -1,8 +1,13 @@ import { BranchCommit, CommitWithStreamBranchMetadata, - Commit + Commit, + CommitBranch } from '@/modules/core/domain/commits/types' +import { + CommitUpdateInput, + UpdateVersionInput +} from '@/modules/core/graph/generated/graphql' import { BranchCommitRecord, StreamCommitRecord } from '@/modules/core/helpers/types' import { MaybeNullOrUndefined, Nullable, Optional } from '@speckle/shared' import { Knex } from 'knex' @@ -86,3 +91,23 @@ export type InsertStreamCommits = ( trx: Knex.Transaction }> ) => Promise + +export type UpdateCommitAndNotify = ( + params: CommitUpdateInput | UpdateVersionInput, + userId: string +) => Promise + +export type GetCommitBranches = (commitIds: string[]) => Promise + +export type GetCommitBranch = (commitId: string) => Promise> + +export type SwitchCommitBranch = ( + commitId: string, + newBranchId: string, + oldBranchId?: string +) => Promise + +export type UpdateCommit = ( + commitId: string, + commit: Partial +) => Promise diff --git a/packages/server/modules/core/domain/commits/types.ts b/packages/server/modules/core/domain/commits/types.ts index f0dc3d5e5..c3bcdd7ab 100644 --- a/packages/server/modules/core/domain/commits/types.ts +++ b/packages/server/modules/core/domain/commits/types.ts @@ -1,3 +1,4 @@ +import { Branch } from '@/modules/core/domain/branches/types' import { CommitRecord } from '@/modules/core/helpers/types' export type Commit = CommitRecord @@ -11,3 +12,5 @@ export type CommitWithStreamBranchMetadata = Commit & { branchId: string branchName: string } + +export type CommitBranch = Branch & { commitId: string } diff --git a/packages/server/modules/core/graph/resolvers/commits.js b/packages/server/modules/core/graph/resolvers/commits.js index 51084315c..000e61b32 100644 --- a/packages/server/modules/core/graph/resolvers/commits.js +++ b/packages/server/modules/core/graph/resolvers/commits.js @@ -16,11 +16,11 @@ const { getPaginatedBranchCommits } = require('@/modules/core/services/commit/retrieval') const { - updateCommitAndNotify, markCommitReceivedAndNotify, deleteCommitAndNotifyFactory, createCommitByBranchIdFactory, - createCommitByBranchNameFactory + createCommitByBranchNameFactory, + updateCommitAndNotifyFactory } = require('@/modules/core/services/commit/management') const { RateLimitError } = require('@/modules/core/errors/ratelimit') @@ -44,10 +44,17 @@ const { deleteCommitFactory, createCommitFactory, insertStreamCommitsFactory, - insertBranchCommitsFactory + insertBranchCommitsFactory, + getCommitBranchFactory, + switchCommitBranchFactory, + updateCommitFactory } = require('@/modules/core/repositories/commits') const { db } = require('@/db/knex') -const { markCommitStreamUpdated } = require('@/modules/core/repositories/streams') +const { + markCommitStreamUpdated, + getStream, + getCommitStream +} = require('@/modules/core/repositories/streams') const { markCommitBranchUpdatedFactory, getBranchByIdFactory, @@ -55,7 +62,8 @@ const { } = require('@/modules/core/repositories/branches') const { addCommitDeletedActivity, - addCommitCreatedActivity + addCommitCreatedActivity, + addCommitUpdatedActivity } = require('@/modules/activitystream/services/commitActivity') const { getObject } = require('@/modules/core/repositories/objects') const { VersionsEmitter } = require('@/modules/core/events/versionsEmitter') @@ -91,6 +99,19 @@ const createCommitByBranchName = createCommitByBranchNameFactory({ getBranchById: getBranchByIdFactory({ db }) }) +const updateCommitAndNotify = updateCommitAndNotifyFactory({ + getCommit: getCommitFactory({ db }), + getStream, + getCommitStream, + getStreamBranchByName: getStreamBranchByNameFactory({ db }), + getCommitBranch: getCommitBranchFactory({ db }), + switchCommitBranch: switchCommitBranchFactory({ db }), + updateCommit: updateCommitFactory({ db }), + addCommitUpdatedActivity, + markCommitStreamUpdated, + markCommitBranchUpdated: markCommitBranchUpdatedFactory({ db }) +}) + /** * @param {boolean} publicOnly * @param {string} userId diff --git a/packages/server/modules/core/graph/resolvers/versions.ts b/packages/server/modules/core/graph/resolvers/versions.ts index 697912c67..bed6b0da8 100644 --- a/packages/server/modules/core/graph/resolvers/versions.ts +++ b/packages/server/modules/core/graph/resolvers/versions.ts @@ -14,7 +14,7 @@ import { CommitUpdateError } from '@/modules/core/errors/commit' import { createCommitByBranchIdFactory, markCommitReceivedAndNotify, - updateCommitAndNotify + updateCommitAndNotifyFactory } from '@/modules/core/services/commit/management' import { getRateLimitResult, @@ -23,18 +23,30 @@ import { import { RateLimitError } from '@/modules/core/errors/ratelimit' import { createCommitFactory, + getCommitBranchFactory, + getCommitFactory, insertBranchCommitsFactory, - insertStreamCommitsFactory + insertStreamCommitsFactory, + switchCommitBranchFactory, + updateCommitFactory } from '@/modules/core/repositories/commits' import { db } from '@/db/knex' import { getObject } from '@/modules/core/repositories/objects' import { getBranchByIdFactory, + getStreamBranchByNameFactory, markCommitBranchUpdatedFactory } from '@/modules/core/repositories/branches' -import { markCommitStreamUpdated } from '@/modules/core/repositories/streams' +import { + getCommitStream, + getStream, + markCommitStreamUpdated +} from '@/modules/core/repositories/streams' import { VersionsEmitter } from '@/modules/core/events/versionsEmitter' -import { addCommitCreatedActivity } from '@/modules/activitystream/services/commitActivity' +import { + addCommitCreatedActivity, + addCommitUpdatedActivity +} from '@/modules/activitystream/services/commitActivity' const createCommitByBranchId = createCommitByBranchIdFactory({ createCommit: createCommitFactory({ db }), @@ -48,6 +60,19 @@ const createCommitByBranchId = createCommitByBranchIdFactory({ addCommitCreatedActivity }) +const updateCommitAndNotify = updateCommitAndNotifyFactory({ + getCommit: getCommitFactory({ db }), + getStream, + getCommitStream, + getStreamBranchByName: getStreamBranchByNameFactory({ db }), + getCommitBranch: getCommitBranchFactory({ db }), + switchCommitBranch: switchCommitBranchFactory({ db }), + updateCommit: updateCommitFactory({ db }), + addCommitUpdatedActivity, + markCommitStreamUpdated, + markCommitBranchUpdated: markCommitBranchUpdatedFactory({ db }) +}) + export = { Project: { async version(parent, args, ctx) { diff --git a/packages/server/modules/core/loaders.ts b/packages/server/modules/core/loaders.ts index b285b2feb..6d099a731 100644 --- a/packages/server/modules/core/loaders.ts +++ b/packages/server/modules/core/loaders.ts @@ -23,7 +23,7 @@ import { import { Nullable } from '@/modules/shared/helpers/typeHelper' import { ServerInviteRecord } from '@/modules/serverinvites/domain/types' import { - getCommitBranches, + getCommitBranchesFactory, getCommitsFactory, getSpecificBranchCommitsFactory, getStreamCommitCounts, @@ -112,6 +112,7 @@ const getStreamBranchCounts = getStreamBranchCountsFactory({ db }) const getBranchCommitCounts = getBranchCommitCountsFactory({ db }) const getCommits = getCommitsFactory({ db }) const getSpecificBranchCommits = getSpecificBranchCommitsFactory({ db }) +const getCommitBranches = getCommitBranchesFactory({ db }) /** * TODO: Lazy load DataLoaders to reduce memory usage diff --git a/packages/server/modules/core/repositories/commits.ts b/packages/server/modules/core/repositories/commits.ts index 53b56879b..7081aa080 100644 --- a/packages/server/modules/core/repositories/commits.ts +++ b/packages/server/modules/core/repositories/commits.ts @@ -30,7 +30,11 @@ import { GetCommits, GetSpecificBranchCommits, InsertBranchCommits, - InsertStreamCommits + InsertStreamCommits, + GetCommitBranches, + GetCommitBranch, + SwitchCommitBranch, + UpdateCommit } from '@/modules/core/domain/commits/operations' const tables = { @@ -321,47 +325,54 @@ export async function getBranchCommitsTotalCount( return parseInt(res?.count || '0') } -export async function getCommitBranches(commitIds: string[]) { - if (!commitIds?.length) return [] +export const getCommitBranchesFactory = + (deps: { db: Knex }): GetCommitBranches => + async (commitIds: string[]) => { + if (!commitIds?.length) return [] - const q = BranchCommits.knex() - .select>([ - ...Branches.cols, - knex.raw(`?? as "commitId"`, [BranchCommits.col.commitId]) - ]) - .innerJoin(Branches.name, Branches.col.id, BranchCommits.col.branchId) - .whereIn(BranchCommits.col.commitId, commitIds) + const q = tables + .branchCommits(deps.db) + .select>([ + ...Branches.cols, + knex.raw(`?? as "commitId"`, [BranchCommits.col.commitId]) + ]) + .innerJoin(Branches.name, Branches.col.id, BranchCommits.col.branchId) + .whereIn(BranchCommits.col.commitId, commitIds) - return await q -} - -export async function getCommitBranch(commitId: string) { - const [commit] = await getCommitBranches([commitId]) - return commit as Optional -} - -export async function switchCommitBranch( - commitId: string, - newBranchId: string, - oldBranchId?: string -) { - const q = BranchCommits.knex() - .where(BranchCommits.col.commitId, commitId) - .update(BranchCommits.withoutTablePrefix.col.branchId, newBranchId) - - if (oldBranchId) { - q.andWhere(BranchCommits.col.branchId, oldBranchId) + return await q } - await q -} +export const getCommitBranchFactory = + (deps: { db: Knex }): GetCommitBranch => + async (commitId: string) => { + const [commit] = await getCommitBranchesFactory(deps)([commitId]) + return commit as Optional + } -export async function updateCommit(commitId: string, commit: Partial) { - const [newCommit] = (await Commits.knex() - .where(Commits.col.id, commitId) - .update(commit, '*')) as CommitRecord[] - return newCommit -} +export const switchCommitBranchFactory = + (deps: { db: Knex }): SwitchCommitBranch => + async (commitId: string, newBranchId: string, oldBranchId?: string) => { + const q = tables + .branchCommits(deps.db) + .where(BranchCommits.col.commitId, commitId) + .update(BranchCommits.withoutTablePrefix.col.branchId, newBranchId) + + if (oldBranchId) { + q.andWhere(BranchCommits.col.branchId, oldBranchId) + } + + await q + } + +export const updateCommitFactory = + (deps: { db: Knex }): UpdateCommit => + async (commitId: string, commit: Partial) => { + const [newCommit] = (await tables + .commits(deps.db) + .where(Commits.col.id, commitId) + .update(commit, '*')) as CommitRecord[] + return newCommit + } export const createCommitFactory = (deps: { db: Knex }): StoreCommit => diff --git a/packages/server/modules/core/services/commit/management.ts b/packages/server/modules/core/services/commit/management.ts index 6a5205868..79f0c528c 100644 --- a/packages/server/modules/core/services/commit/management.ts +++ b/packages/server/modules/core/services/commit/management.ts @@ -16,9 +16,13 @@ import { DeleteCommit, DeleteCommitAndNotify, GetCommit, + GetCommitBranch, InsertBranchCommits, InsertStreamCommits, - StoreCommit + StoreCommit, + SwitchCommitBranch, + UpdateCommit, + UpdateCommitAndNotify } from '@/modules/core/domain/commits/operations' import { CommitCreateError, @@ -37,16 +41,7 @@ import { UpdateVersionInput } from '@/modules/core/graph/generated/graphql' import { CommitRecord } from '@/modules/core/helpers/types' -import { - getStreamBranchByNameFactory, - markCommitBranchUpdatedFactory -} from '@/modules/core/repositories/branches' -import { - getCommitBranch, - getCommitFactory, - switchCommitBranch, - updateCommit -} from '@/modules/core/repositories/commits' +import { getCommitFactory } from '@/modules/core/repositories/commits' import { getObject } from '@/modules/core/repositories/objects' import { getCommitStream, @@ -255,99 +250,111 @@ const isOldVersionUpdateInput = ( i: CommitUpdateInput | UpdateVersionInput ): i is CommitUpdateInput => has(i, 'streamId') -export async function updateCommitAndNotify( - params: CommitUpdateInput | UpdateVersionInput, - userId: string -) { - const { - message, - newBranchName, - streamId, - id: commitId - } = isOldVersionUpdateInput(params) - ? params - : { - message: params.message, - id: params.versionId, - streamId: null, - newBranchName: null - } +export const updateCommitAndNotifyFactory = + (deps: { + getCommit: GetCommit + getStream: typeof getStream + getCommitStream: typeof getCommitStream + getStreamBranchByName: GetStreamBranchByName + getCommitBranch: GetCommitBranch + switchCommitBranch: SwitchCommitBranch + updateCommit: UpdateCommit + addCommitUpdatedActivity: typeof addCommitUpdatedActivity + markCommitStreamUpdated: typeof markCommitStreamUpdated + markCommitBranchUpdated: MarkCommitBranchUpdated + }): UpdateCommitAndNotify => + async (params: CommitUpdateInput | UpdateVersionInput, userId: string) => { + const { + message, + newBranchName, + streamId, + id: commitId + } = isOldVersionUpdateInput(params) + ? params + : { + message: params.message, + id: params.versionId, + streamId: null, + newBranchName: null + } - if (!message && !newBranchName) { - throw new CommitUpdateError('Nothing to update', { - info: { ...params, userId } - }) - } - - const [commit, stream] = await Promise.all([ - getCommitFactory({ db })(commitId), - streamId ? getStream({ streamId, userId }) : getCommitStream({ commitId, userId }) - ]) - if (!commit) { - throw new CommitUpdateError("Can't update nonexistant commit", { - info: { ...params, userId } - }) - } - if (!stream) { - throw new CommitUpdateError("Can't resolve commit stream", { - info: { ...params, userId } - }) - } - if (commit.author !== userId && stream.role !== Roles.Stream.Owner) { - throw new CommitUpdateError( - 'Only the author of a commit or a stream owner may update it', - { - info: { ...params, userId, streamRole: stream.role } - } - ) - } - - if (newBranchName) { - try { - const [newBranch, oldBranch] = await Promise.all([ - getStreamBranchByNameFactory({ db })(streamId, newBranchName), - getCommitBranch(commitId) - ]) - - if (!newBranch || !oldBranch) { - throw new Error("Couldn't resolve branch") - } - if (!commit) { - throw new Error("Couldn't find commit") - } - - await switchCommitBranch(commitId, newBranch.id, oldBranch.id) - } catch (e) { - throw new CommitUpdateError('Failed to update commit branch', { - cause: ensureError(e), - info: params + if (!message && !newBranchName) { + throw new CommitUpdateError('Nothing to update', { + info: { ...params, userId } }) } - } - let newCommit: CommitRecord = commit - if (message) { - newCommit = await updateCommit(commitId, { message }) - } - - if (commit) { - await addCommitUpdatedActivity({ - commitId, - streamId: stream.id, - userId, - originalCommit: commit, - update: params, - newCommit - }) - - await Promise.all([ - markCommitStreamUpdated(commit.id), - markCommitBranchUpdatedFactory({ db })(commit.id) + const [commit, stream] = await Promise.all([ + deps.getCommit(commitId), + streamId + ? deps.getStream({ streamId, userId }) + : deps.getCommitStream({ commitId, userId }) ]) - } + if (!commit) { + throw new CommitUpdateError("Can't update nonexistant commit", { + info: { ...params, userId } + }) + } + if (!stream) { + throw new CommitUpdateError("Can't resolve commit stream", { + info: { ...params, userId } + }) + } + if (commit.author !== userId && stream.role !== Roles.Stream.Owner) { + throw new CommitUpdateError( + 'Only the author of a commit or a stream owner may update it', + { + info: { ...params, userId, streamRole: stream.role } + } + ) + } - return newCommit -} + if (newBranchName) { + try { + const [newBranch, oldBranch] = await Promise.all([ + deps.getStreamBranchByName(streamId, newBranchName), + deps.getCommitBranch(commitId) + ]) + + if (!newBranch || !oldBranch) { + throw new Error("Couldn't resolve branch") + } + if (!commit) { + throw new Error("Couldn't find commit") + } + + await deps.switchCommitBranch(commitId, newBranch.id, oldBranch.id) + } catch (e) { + throw new CommitUpdateError('Failed to update commit branch', { + cause: ensureError(e), + info: params + }) + } + } + + let newCommit: CommitRecord = commit + if (message) { + newCommit = await deps.updateCommit(commitId, { message }) + } + + if (commit) { + await deps.addCommitUpdatedActivity({ + commitId, + streamId: stream.id, + userId, + originalCommit: commit, + update: params, + newCommit + }) + + await Promise.all([ + deps.markCommitStreamUpdated(commit.id), + deps.markCommitBranchUpdated(commit.id) + ]) + } + + return newCommit + } export const deleteCommitAndNotifyFactory = (deps: { diff --git a/packages/server/modules/core/services/commits.js b/packages/server/modules/core/services/commits.js index faf975d3b..8044c6281 100644 --- a/packages/server/modules/core/services/commits.js +++ b/packages/server/modules/core/services/commits.js @@ -10,7 +10,6 @@ const { getPaginatedBranchCommits, getBranchCommitsTotalCount } = require('@/modules/core/repositories/commits') -const { updateCommitAndNotify } = require('@/modules/core/services/commit/management') const { clamp } = require('lodash') const getCommitsByUserIdBase = ({ userId, publicOnly, streamIdWhitelist }) => { @@ -47,14 +46,6 @@ const getCommitsByUserIdBase = ({ userId, publicOnly, streamIdWhitelist }) => { } module.exports = { - /** - * @deprecated Use 'updateCommitAndNotify()' - */ - async updateCommit({ streamId, id, message, newBranchName, userId }) { - await updateCommitAndNotify({ streamId, id, message, newBranchName }, userId) - return true - }, - /** * @deprecated Use `getBranchCommitsTotalCount()` instead */ diff --git a/packages/server/modules/core/tests/commits.spec.js b/packages/server/modules/core/tests/commits.spec.js index 9935be106..db97fd808 100644 --- a/packages/server/modules/core/tests/commits.spec.js +++ b/packages/server/modules/core/tests/commits.spec.js @@ -8,7 +8,6 @@ const { createStream } = require('../services/streams') const { createObject } = require('../services/objects') const { - updateCommit, getCommitsTotalCountByBranchName, getCommitsByBranchName, getCommitsByStreamId, @@ -34,17 +33,26 @@ const { deleteCommitFactory, createCommitFactory, insertStreamCommitsFactory, - insertBranchCommitsFactory + insertBranchCommitsFactory, + getCommitBranchFactory, + switchCommitBranchFactory, + updateCommitFactory } = require('@/modules/core/repositories/commits') const { deleteCommitAndNotifyFactory, createCommitByBranchIdFactory, - createCommitByBranchNameFactory + createCommitByBranchNameFactory, + updateCommitAndNotifyFactory } = require('@/modules/core/services/commit/management') -const { markCommitStreamUpdated } = require('@/modules/core/repositories/streams') +const { + markCommitStreamUpdated, + getCommitStream, + getStream +} = require('@/modules/core/repositories/streams') const { addCommitDeletedActivity, - addCommitCreatedActivity + addCommitCreatedActivity, + addCommitUpdatedActivity } = require('@/modules/activitystream/services/commitActivity') const { getObject } = require('@/modules/core/repositories/objects') const { VersionsEmitter } = require('@/modules/core/events/versionsEmitter') @@ -82,6 +90,19 @@ const createCommitByBranchName = createCommitByBranchNameFactory({ getBranchById: getBranchByIdFactory({ db }) }) +const updateCommitAndNotify = updateCommitAndNotifyFactory({ + getCommit: getCommitFactory({ db }), + getStream, + getCommitStream, + getStreamBranchByName: getStreamBranchByNameFactory({ db }), + getCommitBranch: getCommitBranchFactory({ db }), + switchCommitBranch: switchCommitBranchFactory({ db }), + updateCommit: updateCommitFactory({ db }), + addCommitUpdatedActivity, + markCommitStreamUpdated, + markCommitBranchUpdated: markCommitBranchUpdatedFactory({ db }) +}) + describe('Commits @core-commits', () => { const user = { name: 'Dimitrie Stefanescu', @@ -255,12 +276,14 @@ describe('Commits @core-commits', () => { }) it('Should update a commit', async () => { - const res = await updateCommit({ - id: commitId1, - message: 'FIRST COMMIT YOOOOOO', - userId: user.id, - streamId: stream.id - }) + const res = await updateCommitAndNotify( + { + id: commitId1, + message: 'FIRST COMMIT YOOOOOO', + streamId: stream.id + }, + user.id + ) expect(res).to.equal(true) }) From 8ebd4cd47d49f387517dc583e12b55c613ddce8d Mon Sep 17 00:00:00 2001 From: Fabis Date: Tue, 1 Oct 2024 16:41:17 +0100 Subject: [PATCH 02/12] testfix --- packages/server/modules/core/tests/commits.spec.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/server/modules/core/tests/commits.spec.js b/packages/server/modules/core/tests/commits.spec.js index db97fd808..cdd971340 100644 --- a/packages/server/modules/core/tests/commits.spec.js +++ b/packages/server/modules/core/tests/commits.spec.js @@ -284,7 +284,7 @@ describe('Commits @core-commits', () => { }, user.id ) - expect(res).to.equal(true) + expect(res).to.be.ok }) it('Should delete a commit', async () => { From c6486b4ef4c29a35dee5f5f8ba9780f86054f9c0 Mon Sep 17 00:00:00 2001 From: Fabis Date: Tue, 1 Oct 2024 16:49:06 +0100 Subject: [PATCH 03/12] chore(server): core IoC 19 - getAllBranchCommitsFactory --- .../services/commentActivity.ts | 4 +- .../modules/cli/commands/download/commit.ts | 4 +- .../modules/cli/commands/download/project.ts | 4 +- .../comments/graph/resolvers/comments.ts | 4 +- .../modules/core/domain/commits/operations.ts | 5 ++ .../modules/core/graph/resolvers/models.ts | 4 +- .../modules/core/repositories/commits.ts | 68 ++++++++++--------- .../core/services/commit/viewerResources.ts | 12 ++-- .../server/modules/cross-server-sync/index.ts | 4 +- 9 files changed, 59 insertions(+), 50 deletions(-) diff --git a/packages/server/modules/activitystream/services/commentActivity.ts b/packages/server/modules/activitystream/services/commentActivity.ts index e0c04d4a3..99bfe1935 100644 --- a/packages/server/modules/activitystream/services/commentActivity.ts +++ b/packages/server/modules/activitystream/services/commentActivity.ts @@ -16,7 +16,7 @@ import { getStreamBranchesByNameFactory } from '@/modules/core/repositories/branches' import { - getAllBranchCommits, + getAllBranchCommitsFactory, getCommitsAndTheirBranchIds, getSpecificBranchCommitsFactory } from '@/modules/core/repositories/commits' @@ -69,7 +69,7 @@ export async function addCommentCreatedActivity(params: { getBranchLatestCommits: getBranchLatestCommitsFactory({ db }), getStreamBranchesByName: getStreamBranchesByNameFactory({ db }), getSpecificBranchCommits: getSpecificBranchCommitsFactory({ db }), - getAllBranchCommits + getAllBranchCommits: getAllBranchCommitsFactory({ db }) }) }) diff --git a/packages/server/modules/cli/commands/download/commit.ts b/packages/server/modules/cli/commands/download/commit.ts index bef091c6a..447019dbc 100644 --- a/packages/server/modules/cli/commands/download/commit.ts +++ b/packages/server/modules/cli/commands/download/commit.ts @@ -26,7 +26,7 @@ import { } from '@/modules/core/services/commit/viewerResources' import { createCommitFactory, - getAllBranchCommits, + getAllBranchCommitsFactory, getSpecificBranchCommitsFactory, insertBranchCommitsFactory, insertStreamCommitsFactory @@ -102,7 +102,7 @@ const command: CommandModule< getBranchLatestCommits, getStreamBranchesByName: getStreamBranchesByNameFactory({ db }), getSpecificBranchCommits: getSpecificBranchCommitsFactory({ db }), - getAllBranchCommits + getAllBranchCommits: getAllBranchCommitsFactory({ db }) }) }) const createCommentThreadAndNotify = createCommentThreadAndNotifyFactory({ diff --git a/packages/server/modules/cli/commands/download/project.ts b/packages/server/modules/cli/commands/download/project.ts index 3151e5b6d..34b321bf5 100644 --- a/packages/server/modules/cli/commands/download/project.ts +++ b/packages/server/modules/cli/commands/download/project.ts @@ -32,7 +32,7 @@ import { } from '@/modules/activitystream/services/commentActivity' import { createCommitFactory, - getAllBranchCommits, + getAllBranchCommitsFactory, getSpecificBranchCommitsFactory, insertBranchCommitsFactory, insertStreamCommitsFactory @@ -99,7 +99,7 @@ const command: CommandModule< getBranchLatestCommits: getBranchLatestCommitsFactory({ db }), getStreamBranchesByName: getStreamBranchesByNameFactory({ db }), getSpecificBranchCommits: getSpecificBranchCommitsFactory({ db }), - getAllBranchCommits + getAllBranchCommits: getAllBranchCommitsFactory({ db }) }) }) const createCommentThreadAndNotify = createCommentThreadAndNotifyFactory({ diff --git a/packages/server/modules/comments/graph/resolvers/comments.ts b/packages/server/modules/comments/graph/resolvers/comments.ts index cbd3977fa..0426a54ef 100644 --- a/packages/server/modules/comments/graph/resolvers/comments.ts +++ b/packages/server/modules/comments/graph/resolvers/comments.ts @@ -86,7 +86,7 @@ import { CommentsEmitter } from '@/modules/comments/events/emitter' import { getBlobsFactory } from '@/modules/blobstorage/repositories' import { ResourceIdentifier } from '@/modules/comments/domain/types' import { - getAllBranchCommits, + getAllBranchCommitsFactory, getCommitsAndTheirBranchIds, getSpecificBranchCommitsFactory } from '@/modules/core/repositories/commits' @@ -179,7 +179,7 @@ const getViewerResourceItemsUngrouped = getViewerResourceItemsUngroupedFactory({ getBranchLatestCommits: getBranchLatestCommitsFactory({ db }), getStreamBranchesByName: getStreamBranchesByNameFactory({ db }), getSpecificBranchCommits: getSpecificBranchCommitsFactory({ db }), - getAllBranchCommits + getAllBranchCommits: getAllBranchCommitsFactory({ db }) }) }) const createCommentThreadAndNotify = createCommentThreadAndNotifyFactory({ diff --git a/packages/server/modules/core/domain/commits/operations.ts b/packages/server/modules/core/domain/commits/operations.ts index 7d31c47ce..7c53cc85c 100644 --- a/packages/server/modules/core/domain/commits/operations.ts +++ b/packages/server/modules/core/domain/commits/operations.ts @@ -111,3 +111,8 @@ export type UpdateCommit = ( commitId: string, commit: Partial ) => Promise + +export type GetAllBranchCommits = (params: { + branchIds?: string[] + projectId?: string +}) => Promise<{ [branchId: string]: Commit[] }> diff --git a/packages/server/modules/core/graph/resolvers/models.ts b/packages/server/modules/core/graph/resolvers/models.ts index e39840bd6..895d5d2a3 100644 --- a/packages/server/modules/core/graph/resolvers/models.ts +++ b/packages/server/modules/core/graph/resolvers/models.ts @@ -41,7 +41,7 @@ import { BranchNotFoundError } from '@/modules/core/errors/branch' import { CommitNotFoundError } from '@/modules/core/errors/commit' import { getStreamObjects } from '@/modules/core/repositories/objects' import { - getAllBranchCommits, + getAllBranchCommitsFactory, getSpecificBranchCommitsFactory } from '@/modules/core/repositories/commits' import { db } from '@/db/knex' @@ -58,7 +58,7 @@ const getViewerResourceGroups = getViewerResourceGroupsFactory({ getBranchLatestCommits: getBranchLatestCommitsFactory({ db }), getStreamBranchesByName: getStreamBranchesByNameFactory({ db }), getSpecificBranchCommits: getSpecificBranchCommitsFactory({ db }), - getAllBranchCommits + getAllBranchCommits: getAllBranchCommitsFactory({ db }) }) const getPaginatedProjectModels = getPaginatedProjectModelsFactory({ diff --git a/packages/server/modules/core/repositories/commits.ts b/packages/server/modules/core/repositories/commits.ts index 7081aa080..8d1bccbce 100644 --- a/packages/server/modules/core/repositories/commits.ts +++ b/packages/server/modules/core/repositories/commits.ts @@ -34,7 +34,8 @@ import { GetCommitBranches, GetCommitBranch, SwitchCommitBranch, - UpdateCommit + UpdateCommit, + GetAllBranchCommits } from '@/modules/core/domain/commits/operations' const tables = { @@ -418,41 +419,44 @@ export async function getObjectCommitsWithStreamIds( return await q } -export async function getAllBranchCommits(params: { - branchIds?: string[] - projectId?: string -}): Promise> { - const { branchIds, projectId } = params - if (!branchIds?.length && !projectId) return {} +export const getAllBranchCommitsFactory = + (deps: { db: Knex }): GetAllBranchCommits => + async (params: { + branchIds?: string[] + projectId?: string + }): Promise> => { + const { branchIds, projectId } = params + if (!branchIds?.length && !projectId) return {} - const q = BranchCommits.knex() - .select>([ - ...Commits.cols, - BranchCommits.col.branchId - ]) - .innerJoin(Commits.name, Commits.col.id, BranchCommits.col.commitId) + const q = tables + .branchCommits(deps.db) + .select>([ + ...Commits.cols, + BranchCommits.col.branchId + ]) + .innerJoin(Commits.name, Commits.col.id, BranchCommits.col.commitId) - if (branchIds?.length) { - q.whereIn(BranchCommits.col.branchId, branchIds) + if (branchIds?.length) { + q.whereIn(BranchCommits.col.branchId, branchIds) + } + + if (projectId) { + q.innerJoin(Branches.name, Branches.col.id, BranchCommits.col.branchId) + q.andWhere(Branches.col.streamId, projectId) + } + + const res = await q + return reduce( + res, + (res, item) => { + const branchId = item.branchId + ;(res[branchId] = res[branchId] || []).push(item) + return res + }, + {} as Record + ) } - if (projectId) { - q.innerJoin(Branches.name, Branches.col.id, BranchCommits.col.branchId) - q.andWhere(Branches.col.streamId, projectId) - } - - const res = await q - return reduce( - res, - (res, item) => { - const branchId = item.branchId - ;(res[branchId] = res[branchId] || []).push(item) - return res - }, - {} as Record - ) -} - export async function getUserStreamCommitCounts(params: { userIds: string[] /** diff --git a/packages/server/modules/core/services/commit/viewerResources.ts b/packages/server/modules/core/services/commit/viewerResources.ts index 55be2ac16..a899f2cfa 100644 --- a/packages/server/modules/core/services/commit/viewerResources.ts +++ b/packages/server/modules/core/services/commit/viewerResources.ts @@ -10,7 +10,10 @@ import { GetBranchLatestCommits, GetStreamBranchesByName } from '@/modules/core/domain/branches/operations' -import { GetSpecificBranchCommits } from '@/modules/core/domain/commits/operations' +import { + GetAllBranchCommits, + GetSpecificBranchCommits +} from '@/modules/core/domain/commits/operations' import { ResourceIdentifier, ResourceIdentifierInput, @@ -20,10 +23,7 @@ import { ViewerUpdateTrackingTarget } from '@/modules/core/graph/generated/graphql' import { CommitRecord } from '@/modules/core/helpers/types' -import { - getAllBranchCommits, - getCommitsAndTheirBranchIds -} from '@/modules/core/repositories/commits' +import { getCommitsAndTheirBranchIds } from '@/modules/core/repositories/commits' import { getStreamObjects } from '@/modules/core/repositories/objects' import { Optional, SpeckleViewer } from '@speckle/shared' import { flatten, keyBy, reduce, uniq, uniqWith } from 'lodash' @@ -77,7 +77,7 @@ const getObjectResourceGroupsFactory = type GetVersionResourceGroupsIncludingAllVersionsFactoryDeps = { getStreamBranchesByName: GetStreamBranchesByName - getAllBranchCommits: typeof getAllBranchCommits + getAllBranchCommits: GetAllBranchCommits } const getVersionResourceGroupsIncludingAllVersionsFactory = diff --git a/packages/server/modules/cross-server-sync/index.ts b/packages/server/modules/cross-server-sync/index.ts index 9db68e63c..5c5d839d9 100644 --- a/packages/server/modules/cross-server-sync/index.ts +++ b/packages/server/modules/cross-server-sync/index.ts @@ -31,7 +31,7 @@ import { } from '@/modules/core/repositories/branches' import { createCommitFactory, - getAllBranchCommits, + getAllBranchCommitsFactory, getSpecificBranchCommitsFactory, insertBranchCommitsFactory, insertStreamCommitsFactory @@ -76,7 +76,7 @@ const crossServerSyncModule: SpeckleModule = { getBranchLatestCommits: getBranchLatestCommitsFactory({ db }), getStreamBranchesByName: getStreamBranchesByNameFactory({ db }), getSpecificBranchCommits: getSpecificBranchCommitsFactory({ db }), - getAllBranchCommits + getAllBranchCommits: getAllBranchCommitsFactory({ db }) }) }) const createCommentThreadAndNotify = createCommentThreadAndNotifyFactory({ From a8ed344fb86c4101dacc22fd5613df7175312356 Mon Sep 17 00:00:00 2001 From: Mike Date: Mon, 7 Oct 2024 13:59:41 +0200 Subject: [PATCH 04/12] Feat: Add custom modal for user feedback (#3179) --- .../components/dashboard/Sidebar.vue | 18 +++-- .../frontend-2/components/feedback/Dialog.vue | 80 +++++++++++++++++++ .../frontend-2/lib/core/composables/zapier.ts | 18 +++++ 3 files changed, 109 insertions(+), 7 deletions(-) create mode 100644 packages/frontend-2/components/feedback/Dialog.vue create mode 100644 packages/frontend-2/lib/core/composables/zapier.ts diff --git a/packages/frontend-2/components/dashboard/Sidebar.vue b/packages/frontend-2/components/dashboard/Sidebar.vue index d1c1a6400..9078713ee 100644 --- a/packages/frontend-2/components/dashboard/Sidebar.vue +++ b/packages/frontend-2/components/dashboard/Sidebar.vue @@ -123,17 +123,13 @@ - - +
+ - +
+ + { } } }) + +const openFeedbackDialog = () => { + showFeedbackDialog.value = true + isOpenMobile.value = false +} diff --git a/packages/frontend-2/components/feedback/Dialog.vue b/packages/frontend-2/components/feedback/Dialog.vue new file mode 100644 index 000000000..dc9f3792c --- /dev/null +++ b/packages/frontend-2/components/feedback/Dialog.vue @@ -0,0 +1,80 @@ + + + diff --git a/packages/frontend-2/lib/core/composables/zapier.ts b/packages/frontend-2/lib/core/composables/zapier.ts new file mode 100644 index 000000000..413b28931 --- /dev/null +++ b/packages/frontend-2/lib/core/composables/zapier.ts @@ -0,0 +1,18 @@ +export function useZapier() { + const sendWebhook = async (webhookUrl: string, data: Record) => { + const response = await fetch(webhookUrl, { + method: 'POST', + headers: { + 'Content-Type': 'application/json' + }, + mode: 'no-cors', + body: JSON.stringify(data) + }) + + return response + } + + return { + sendWebhook + } +} From 428f1efe03816c5ebaf29b3334524392d5f950ca Mon Sep 17 00:00:00 2001 From: Kristaps Fabians Geikins Date: Mon, 7 Oct 2024 13:51:09 +0100 Subject: [PATCH 05/12] chore(server): core IoC 18 - updateCommitAndNotifyFactory (#3169) * chore(server): core IoC 18 - updateCommitAndNotifyFactory * testfix --- .../modules/core/domain/commits/operations.ts | 27 ++- .../modules/core/domain/commits/types.ts | 3 + .../modules/core/graph/resolvers/commits.js | 31 ++- .../modules/core/graph/resolvers/versions.ts | 33 ++- packages/server/modules/core/loaders.ts | 3 +- .../modules/core/repositories/commits.ts | 85 ++++---- .../core/services/commit/management.ts | 203 +++++++++--------- .../server/modules/core/services/commits.js | 9 - .../server/modules/core/tests/commits.spec.js | 47 ++-- 9 files changed, 274 insertions(+), 167 deletions(-) diff --git a/packages/server/modules/core/domain/commits/operations.ts b/packages/server/modules/core/domain/commits/operations.ts index eae40d20b..7d31c47ce 100644 --- a/packages/server/modules/core/domain/commits/operations.ts +++ b/packages/server/modules/core/domain/commits/operations.ts @@ -1,8 +1,13 @@ import { BranchCommit, CommitWithStreamBranchMetadata, - Commit + Commit, + CommitBranch } from '@/modules/core/domain/commits/types' +import { + CommitUpdateInput, + UpdateVersionInput +} from '@/modules/core/graph/generated/graphql' import { BranchCommitRecord, StreamCommitRecord } from '@/modules/core/helpers/types' import { MaybeNullOrUndefined, Nullable, Optional } from '@speckle/shared' import { Knex } from 'knex' @@ -86,3 +91,23 @@ export type InsertStreamCommits = ( trx: Knex.Transaction }> ) => Promise + +export type UpdateCommitAndNotify = ( + params: CommitUpdateInput | UpdateVersionInput, + userId: string +) => Promise + +export type GetCommitBranches = (commitIds: string[]) => Promise + +export type GetCommitBranch = (commitId: string) => Promise> + +export type SwitchCommitBranch = ( + commitId: string, + newBranchId: string, + oldBranchId?: string +) => Promise + +export type UpdateCommit = ( + commitId: string, + commit: Partial +) => Promise diff --git a/packages/server/modules/core/domain/commits/types.ts b/packages/server/modules/core/domain/commits/types.ts index f0dc3d5e5..c3bcdd7ab 100644 --- a/packages/server/modules/core/domain/commits/types.ts +++ b/packages/server/modules/core/domain/commits/types.ts @@ -1,3 +1,4 @@ +import { Branch } from '@/modules/core/domain/branches/types' import { CommitRecord } from '@/modules/core/helpers/types' export type Commit = CommitRecord @@ -11,3 +12,5 @@ export type CommitWithStreamBranchMetadata = Commit & { branchId: string branchName: string } + +export type CommitBranch = Branch & { commitId: string } diff --git a/packages/server/modules/core/graph/resolvers/commits.js b/packages/server/modules/core/graph/resolvers/commits.js index 51084315c..000e61b32 100644 --- a/packages/server/modules/core/graph/resolvers/commits.js +++ b/packages/server/modules/core/graph/resolvers/commits.js @@ -16,11 +16,11 @@ const { getPaginatedBranchCommits } = require('@/modules/core/services/commit/retrieval') const { - updateCommitAndNotify, markCommitReceivedAndNotify, deleteCommitAndNotifyFactory, createCommitByBranchIdFactory, - createCommitByBranchNameFactory + createCommitByBranchNameFactory, + updateCommitAndNotifyFactory } = require('@/modules/core/services/commit/management') const { RateLimitError } = require('@/modules/core/errors/ratelimit') @@ -44,10 +44,17 @@ const { deleteCommitFactory, createCommitFactory, insertStreamCommitsFactory, - insertBranchCommitsFactory + insertBranchCommitsFactory, + getCommitBranchFactory, + switchCommitBranchFactory, + updateCommitFactory } = require('@/modules/core/repositories/commits') const { db } = require('@/db/knex') -const { markCommitStreamUpdated } = require('@/modules/core/repositories/streams') +const { + markCommitStreamUpdated, + getStream, + getCommitStream +} = require('@/modules/core/repositories/streams') const { markCommitBranchUpdatedFactory, getBranchByIdFactory, @@ -55,7 +62,8 @@ const { } = require('@/modules/core/repositories/branches') const { addCommitDeletedActivity, - addCommitCreatedActivity + addCommitCreatedActivity, + addCommitUpdatedActivity } = require('@/modules/activitystream/services/commitActivity') const { getObject } = require('@/modules/core/repositories/objects') const { VersionsEmitter } = require('@/modules/core/events/versionsEmitter') @@ -91,6 +99,19 @@ const createCommitByBranchName = createCommitByBranchNameFactory({ getBranchById: getBranchByIdFactory({ db }) }) +const updateCommitAndNotify = updateCommitAndNotifyFactory({ + getCommit: getCommitFactory({ db }), + getStream, + getCommitStream, + getStreamBranchByName: getStreamBranchByNameFactory({ db }), + getCommitBranch: getCommitBranchFactory({ db }), + switchCommitBranch: switchCommitBranchFactory({ db }), + updateCommit: updateCommitFactory({ db }), + addCommitUpdatedActivity, + markCommitStreamUpdated, + markCommitBranchUpdated: markCommitBranchUpdatedFactory({ db }) +}) + /** * @param {boolean} publicOnly * @param {string} userId diff --git a/packages/server/modules/core/graph/resolvers/versions.ts b/packages/server/modules/core/graph/resolvers/versions.ts index 697912c67..bed6b0da8 100644 --- a/packages/server/modules/core/graph/resolvers/versions.ts +++ b/packages/server/modules/core/graph/resolvers/versions.ts @@ -14,7 +14,7 @@ import { CommitUpdateError } from '@/modules/core/errors/commit' import { createCommitByBranchIdFactory, markCommitReceivedAndNotify, - updateCommitAndNotify + updateCommitAndNotifyFactory } from '@/modules/core/services/commit/management' import { getRateLimitResult, @@ -23,18 +23,30 @@ import { import { RateLimitError } from '@/modules/core/errors/ratelimit' import { createCommitFactory, + getCommitBranchFactory, + getCommitFactory, insertBranchCommitsFactory, - insertStreamCommitsFactory + insertStreamCommitsFactory, + switchCommitBranchFactory, + updateCommitFactory } from '@/modules/core/repositories/commits' import { db } from '@/db/knex' import { getObject } from '@/modules/core/repositories/objects' import { getBranchByIdFactory, + getStreamBranchByNameFactory, markCommitBranchUpdatedFactory } from '@/modules/core/repositories/branches' -import { markCommitStreamUpdated } from '@/modules/core/repositories/streams' +import { + getCommitStream, + getStream, + markCommitStreamUpdated +} from '@/modules/core/repositories/streams' import { VersionsEmitter } from '@/modules/core/events/versionsEmitter' -import { addCommitCreatedActivity } from '@/modules/activitystream/services/commitActivity' +import { + addCommitCreatedActivity, + addCommitUpdatedActivity +} from '@/modules/activitystream/services/commitActivity' const createCommitByBranchId = createCommitByBranchIdFactory({ createCommit: createCommitFactory({ db }), @@ -48,6 +60,19 @@ const createCommitByBranchId = createCommitByBranchIdFactory({ addCommitCreatedActivity }) +const updateCommitAndNotify = updateCommitAndNotifyFactory({ + getCommit: getCommitFactory({ db }), + getStream, + getCommitStream, + getStreamBranchByName: getStreamBranchByNameFactory({ db }), + getCommitBranch: getCommitBranchFactory({ db }), + switchCommitBranch: switchCommitBranchFactory({ db }), + updateCommit: updateCommitFactory({ db }), + addCommitUpdatedActivity, + markCommitStreamUpdated, + markCommitBranchUpdated: markCommitBranchUpdatedFactory({ db }) +}) + export = { Project: { async version(parent, args, ctx) { diff --git a/packages/server/modules/core/loaders.ts b/packages/server/modules/core/loaders.ts index b285b2feb..6d099a731 100644 --- a/packages/server/modules/core/loaders.ts +++ b/packages/server/modules/core/loaders.ts @@ -23,7 +23,7 @@ import { import { Nullable } from '@/modules/shared/helpers/typeHelper' import { ServerInviteRecord } from '@/modules/serverinvites/domain/types' import { - getCommitBranches, + getCommitBranchesFactory, getCommitsFactory, getSpecificBranchCommitsFactory, getStreamCommitCounts, @@ -112,6 +112,7 @@ const getStreamBranchCounts = getStreamBranchCountsFactory({ db }) const getBranchCommitCounts = getBranchCommitCountsFactory({ db }) const getCommits = getCommitsFactory({ db }) const getSpecificBranchCommits = getSpecificBranchCommitsFactory({ db }) +const getCommitBranches = getCommitBranchesFactory({ db }) /** * TODO: Lazy load DataLoaders to reduce memory usage diff --git a/packages/server/modules/core/repositories/commits.ts b/packages/server/modules/core/repositories/commits.ts index 53b56879b..7081aa080 100644 --- a/packages/server/modules/core/repositories/commits.ts +++ b/packages/server/modules/core/repositories/commits.ts @@ -30,7 +30,11 @@ import { GetCommits, GetSpecificBranchCommits, InsertBranchCommits, - InsertStreamCommits + InsertStreamCommits, + GetCommitBranches, + GetCommitBranch, + SwitchCommitBranch, + UpdateCommit } from '@/modules/core/domain/commits/operations' const tables = { @@ -321,47 +325,54 @@ export async function getBranchCommitsTotalCount( return parseInt(res?.count || '0') } -export async function getCommitBranches(commitIds: string[]) { - if (!commitIds?.length) return [] +export const getCommitBranchesFactory = + (deps: { db: Knex }): GetCommitBranches => + async (commitIds: string[]) => { + if (!commitIds?.length) return [] - const q = BranchCommits.knex() - .select>([ - ...Branches.cols, - knex.raw(`?? as "commitId"`, [BranchCommits.col.commitId]) - ]) - .innerJoin(Branches.name, Branches.col.id, BranchCommits.col.branchId) - .whereIn(BranchCommits.col.commitId, commitIds) + const q = tables + .branchCommits(deps.db) + .select>([ + ...Branches.cols, + knex.raw(`?? as "commitId"`, [BranchCommits.col.commitId]) + ]) + .innerJoin(Branches.name, Branches.col.id, BranchCommits.col.branchId) + .whereIn(BranchCommits.col.commitId, commitIds) - return await q -} - -export async function getCommitBranch(commitId: string) { - const [commit] = await getCommitBranches([commitId]) - return commit as Optional -} - -export async function switchCommitBranch( - commitId: string, - newBranchId: string, - oldBranchId?: string -) { - const q = BranchCommits.knex() - .where(BranchCommits.col.commitId, commitId) - .update(BranchCommits.withoutTablePrefix.col.branchId, newBranchId) - - if (oldBranchId) { - q.andWhere(BranchCommits.col.branchId, oldBranchId) + return await q } - await q -} +export const getCommitBranchFactory = + (deps: { db: Knex }): GetCommitBranch => + async (commitId: string) => { + const [commit] = await getCommitBranchesFactory(deps)([commitId]) + return commit as Optional + } -export async function updateCommit(commitId: string, commit: Partial) { - const [newCommit] = (await Commits.knex() - .where(Commits.col.id, commitId) - .update(commit, '*')) as CommitRecord[] - return newCommit -} +export const switchCommitBranchFactory = + (deps: { db: Knex }): SwitchCommitBranch => + async (commitId: string, newBranchId: string, oldBranchId?: string) => { + const q = tables + .branchCommits(deps.db) + .where(BranchCommits.col.commitId, commitId) + .update(BranchCommits.withoutTablePrefix.col.branchId, newBranchId) + + if (oldBranchId) { + q.andWhere(BranchCommits.col.branchId, oldBranchId) + } + + await q + } + +export const updateCommitFactory = + (deps: { db: Knex }): UpdateCommit => + async (commitId: string, commit: Partial) => { + const [newCommit] = (await tables + .commits(deps.db) + .where(Commits.col.id, commitId) + .update(commit, '*')) as CommitRecord[] + return newCommit + } export const createCommitFactory = (deps: { db: Knex }): StoreCommit => diff --git a/packages/server/modules/core/services/commit/management.ts b/packages/server/modules/core/services/commit/management.ts index 6a5205868..79f0c528c 100644 --- a/packages/server/modules/core/services/commit/management.ts +++ b/packages/server/modules/core/services/commit/management.ts @@ -16,9 +16,13 @@ import { DeleteCommit, DeleteCommitAndNotify, GetCommit, + GetCommitBranch, InsertBranchCommits, InsertStreamCommits, - StoreCommit + StoreCommit, + SwitchCommitBranch, + UpdateCommit, + UpdateCommitAndNotify } from '@/modules/core/domain/commits/operations' import { CommitCreateError, @@ -37,16 +41,7 @@ import { UpdateVersionInput } from '@/modules/core/graph/generated/graphql' import { CommitRecord } from '@/modules/core/helpers/types' -import { - getStreamBranchByNameFactory, - markCommitBranchUpdatedFactory -} from '@/modules/core/repositories/branches' -import { - getCommitBranch, - getCommitFactory, - switchCommitBranch, - updateCommit -} from '@/modules/core/repositories/commits' +import { getCommitFactory } from '@/modules/core/repositories/commits' import { getObject } from '@/modules/core/repositories/objects' import { getCommitStream, @@ -255,99 +250,111 @@ const isOldVersionUpdateInput = ( i: CommitUpdateInput | UpdateVersionInput ): i is CommitUpdateInput => has(i, 'streamId') -export async function updateCommitAndNotify( - params: CommitUpdateInput | UpdateVersionInput, - userId: string -) { - const { - message, - newBranchName, - streamId, - id: commitId - } = isOldVersionUpdateInput(params) - ? params - : { - message: params.message, - id: params.versionId, - streamId: null, - newBranchName: null - } +export const updateCommitAndNotifyFactory = + (deps: { + getCommit: GetCommit + getStream: typeof getStream + getCommitStream: typeof getCommitStream + getStreamBranchByName: GetStreamBranchByName + getCommitBranch: GetCommitBranch + switchCommitBranch: SwitchCommitBranch + updateCommit: UpdateCommit + addCommitUpdatedActivity: typeof addCommitUpdatedActivity + markCommitStreamUpdated: typeof markCommitStreamUpdated + markCommitBranchUpdated: MarkCommitBranchUpdated + }): UpdateCommitAndNotify => + async (params: CommitUpdateInput | UpdateVersionInput, userId: string) => { + const { + message, + newBranchName, + streamId, + id: commitId + } = isOldVersionUpdateInput(params) + ? params + : { + message: params.message, + id: params.versionId, + streamId: null, + newBranchName: null + } - if (!message && !newBranchName) { - throw new CommitUpdateError('Nothing to update', { - info: { ...params, userId } - }) - } - - const [commit, stream] = await Promise.all([ - getCommitFactory({ db })(commitId), - streamId ? getStream({ streamId, userId }) : getCommitStream({ commitId, userId }) - ]) - if (!commit) { - throw new CommitUpdateError("Can't update nonexistant commit", { - info: { ...params, userId } - }) - } - if (!stream) { - throw new CommitUpdateError("Can't resolve commit stream", { - info: { ...params, userId } - }) - } - if (commit.author !== userId && stream.role !== Roles.Stream.Owner) { - throw new CommitUpdateError( - 'Only the author of a commit or a stream owner may update it', - { - info: { ...params, userId, streamRole: stream.role } - } - ) - } - - if (newBranchName) { - try { - const [newBranch, oldBranch] = await Promise.all([ - getStreamBranchByNameFactory({ db })(streamId, newBranchName), - getCommitBranch(commitId) - ]) - - if (!newBranch || !oldBranch) { - throw new Error("Couldn't resolve branch") - } - if (!commit) { - throw new Error("Couldn't find commit") - } - - await switchCommitBranch(commitId, newBranch.id, oldBranch.id) - } catch (e) { - throw new CommitUpdateError('Failed to update commit branch', { - cause: ensureError(e), - info: params + if (!message && !newBranchName) { + throw new CommitUpdateError('Nothing to update', { + info: { ...params, userId } }) } - } - let newCommit: CommitRecord = commit - if (message) { - newCommit = await updateCommit(commitId, { message }) - } - - if (commit) { - await addCommitUpdatedActivity({ - commitId, - streamId: stream.id, - userId, - originalCommit: commit, - update: params, - newCommit - }) - - await Promise.all([ - markCommitStreamUpdated(commit.id), - markCommitBranchUpdatedFactory({ db })(commit.id) + const [commit, stream] = await Promise.all([ + deps.getCommit(commitId), + streamId + ? deps.getStream({ streamId, userId }) + : deps.getCommitStream({ commitId, userId }) ]) - } + if (!commit) { + throw new CommitUpdateError("Can't update nonexistant commit", { + info: { ...params, userId } + }) + } + if (!stream) { + throw new CommitUpdateError("Can't resolve commit stream", { + info: { ...params, userId } + }) + } + if (commit.author !== userId && stream.role !== Roles.Stream.Owner) { + throw new CommitUpdateError( + 'Only the author of a commit or a stream owner may update it', + { + info: { ...params, userId, streamRole: stream.role } + } + ) + } - return newCommit -} + if (newBranchName) { + try { + const [newBranch, oldBranch] = await Promise.all([ + deps.getStreamBranchByName(streamId, newBranchName), + deps.getCommitBranch(commitId) + ]) + + if (!newBranch || !oldBranch) { + throw new Error("Couldn't resolve branch") + } + if (!commit) { + throw new Error("Couldn't find commit") + } + + await deps.switchCommitBranch(commitId, newBranch.id, oldBranch.id) + } catch (e) { + throw new CommitUpdateError('Failed to update commit branch', { + cause: ensureError(e), + info: params + }) + } + } + + let newCommit: CommitRecord = commit + if (message) { + newCommit = await deps.updateCommit(commitId, { message }) + } + + if (commit) { + await deps.addCommitUpdatedActivity({ + commitId, + streamId: stream.id, + userId, + originalCommit: commit, + update: params, + newCommit + }) + + await Promise.all([ + deps.markCommitStreamUpdated(commit.id), + deps.markCommitBranchUpdated(commit.id) + ]) + } + + return newCommit + } export const deleteCommitAndNotifyFactory = (deps: { diff --git a/packages/server/modules/core/services/commits.js b/packages/server/modules/core/services/commits.js index faf975d3b..8044c6281 100644 --- a/packages/server/modules/core/services/commits.js +++ b/packages/server/modules/core/services/commits.js @@ -10,7 +10,6 @@ const { getPaginatedBranchCommits, getBranchCommitsTotalCount } = require('@/modules/core/repositories/commits') -const { updateCommitAndNotify } = require('@/modules/core/services/commit/management') const { clamp } = require('lodash') const getCommitsByUserIdBase = ({ userId, publicOnly, streamIdWhitelist }) => { @@ -47,14 +46,6 @@ const getCommitsByUserIdBase = ({ userId, publicOnly, streamIdWhitelist }) => { } module.exports = { - /** - * @deprecated Use 'updateCommitAndNotify()' - */ - async updateCommit({ streamId, id, message, newBranchName, userId }) { - await updateCommitAndNotify({ streamId, id, message, newBranchName }, userId) - return true - }, - /** * @deprecated Use `getBranchCommitsTotalCount()` instead */ diff --git a/packages/server/modules/core/tests/commits.spec.js b/packages/server/modules/core/tests/commits.spec.js index 9935be106..cdd971340 100644 --- a/packages/server/modules/core/tests/commits.spec.js +++ b/packages/server/modules/core/tests/commits.spec.js @@ -8,7 +8,6 @@ const { createStream } = require('../services/streams') const { createObject } = require('../services/objects') const { - updateCommit, getCommitsTotalCountByBranchName, getCommitsByBranchName, getCommitsByStreamId, @@ -34,17 +33,26 @@ const { deleteCommitFactory, createCommitFactory, insertStreamCommitsFactory, - insertBranchCommitsFactory + insertBranchCommitsFactory, + getCommitBranchFactory, + switchCommitBranchFactory, + updateCommitFactory } = require('@/modules/core/repositories/commits') const { deleteCommitAndNotifyFactory, createCommitByBranchIdFactory, - createCommitByBranchNameFactory + createCommitByBranchNameFactory, + updateCommitAndNotifyFactory } = require('@/modules/core/services/commit/management') -const { markCommitStreamUpdated } = require('@/modules/core/repositories/streams') +const { + markCommitStreamUpdated, + getCommitStream, + getStream +} = require('@/modules/core/repositories/streams') const { addCommitDeletedActivity, - addCommitCreatedActivity + addCommitCreatedActivity, + addCommitUpdatedActivity } = require('@/modules/activitystream/services/commitActivity') const { getObject } = require('@/modules/core/repositories/objects') const { VersionsEmitter } = require('@/modules/core/events/versionsEmitter') @@ -82,6 +90,19 @@ const createCommitByBranchName = createCommitByBranchNameFactory({ getBranchById: getBranchByIdFactory({ db }) }) +const updateCommitAndNotify = updateCommitAndNotifyFactory({ + getCommit: getCommitFactory({ db }), + getStream, + getCommitStream, + getStreamBranchByName: getStreamBranchByNameFactory({ db }), + getCommitBranch: getCommitBranchFactory({ db }), + switchCommitBranch: switchCommitBranchFactory({ db }), + updateCommit: updateCommitFactory({ db }), + addCommitUpdatedActivity, + markCommitStreamUpdated, + markCommitBranchUpdated: markCommitBranchUpdatedFactory({ db }) +}) + describe('Commits @core-commits', () => { const user = { name: 'Dimitrie Stefanescu', @@ -255,13 +276,15 @@ describe('Commits @core-commits', () => { }) it('Should update a commit', async () => { - const res = await updateCommit({ - id: commitId1, - message: 'FIRST COMMIT YOOOOOO', - userId: user.id, - streamId: stream.id - }) - expect(res).to.equal(true) + const res = await updateCommitAndNotify( + { + id: commitId1, + message: 'FIRST COMMIT YOOOOOO', + streamId: stream.id + }, + user.id + ) + expect(res).to.be.ok }) it('Should delete a commit', async () => { From 66ecafb2e550d627263861450e15e9a6351a3500 Mon Sep 17 00:00:00 2001 From: Fabis Date: Tue, 1 Oct 2024 17:06:49 +0100 Subject: [PATCH 06/12] chore(server): core IoC 20 - commits repo dataloders --- .../modules/core/domain/commits/operations.ts | 33 ++++ packages/server/modules/core/loaders.ts | 9 +- .../modules/core/repositories/commits.ts | 177 ++++++++++-------- .../modules/core/services/commit/retrieval.ts | 9 +- .../server/modules/core/services/commits.js | 5 - .../server/modules/core/tests/commits.spec.js | 7 +- 6 files changed, 140 insertions(+), 100 deletions(-) diff --git a/packages/server/modules/core/domain/commits/operations.ts b/packages/server/modules/core/domain/commits/operations.ts index 7c53cc85c..1d460776a 100644 --- a/packages/server/modules/core/domain/commits/operations.ts +++ b/packages/server/modules/core/domain/commits/operations.ts @@ -116,3 +116,36 @@ export type GetAllBranchCommits = (params: { branchIds?: string[] projectId?: string }) => Promise<{ [branchId: string]: Commit[] }> + +export type GetStreamCommitCounts = ( + streamIds: string[], + options?: Partial<{ + ignoreGlobalsBranch: boolean + }> +) => Promise< + { + count: number + streamId: string + }[] +> + +export type GetStreamCommitCount = ( + streamId: string, + options?: Partial<{ + ignoreGlobalsBranch: boolean + }> +) => Promise + +export type GetUserStreamCommitCounts = (params: { + userIds: string[] + publicOnly?: boolean +}) => Promise<{ + [userId: string]: number +}> + +export type GetUserAuthoredCommitCounts = (params: { + userIds: string[] + publicOnly?: boolean +}) => Promise<{ + [userId: string]: number +}> diff --git a/packages/server/modules/core/loaders.ts b/packages/server/modules/core/loaders.ts index 6d099a731..387a31ac5 100644 --- a/packages/server/modules/core/loaders.ts +++ b/packages/server/modules/core/loaders.ts @@ -26,9 +26,9 @@ import { getCommitBranchesFactory, getCommitsFactory, getSpecificBranchCommitsFactory, - getStreamCommitCounts, - getUserAuthoredCommitCounts, - getUserStreamCommitCounts + getStreamCommitCountsFactory, + getUserAuthoredCommitCountsFactory, + getUserStreamCommitCountsFactory } from '@/modules/core/repositories/commits' import { ResourceIdentifier, Scope } from '@/modules/core/graph/generated/graphql' import { @@ -113,6 +113,9 @@ const getBranchCommitCounts = getBranchCommitCountsFactory({ db }) const getCommits = getCommitsFactory({ db }) const getSpecificBranchCommits = getSpecificBranchCommitsFactory({ db }) const getCommitBranches = getCommitBranchesFactory({ db }) +const getStreamCommitCounts = getStreamCommitCountsFactory({ db }) +const getUserStreamCommitCounts = getUserStreamCommitCountsFactory({ db }) +const getUserAuthoredCommitCounts = getUserAuthoredCommitCountsFactory({ db }) /** * TODO: Lazy load DataLoaders to reduce memory usage diff --git a/packages/server/modules/core/repositories/commits.ts b/packages/server/modules/core/repositories/commits.ts index 8d1bccbce..542031c60 100644 --- a/packages/server/modules/core/repositories/commits.ts +++ b/packages/server/modules/core/repositories/commits.ts @@ -11,6 +11,7 @@ import { BranchCommitRecord, BranchRecord, CommitRecord, + StreamAclRecord, StreamCommitRecord } from '@/modules/core/helpers/types' import { clamp, uniq, uniqBy, reduce, keyBy, mapValues } from 'lodash' @@ -35,13 +36,18 @@ import { GetCommitBranch, SwitchCommitBranch, UpdateCommit, - GetAllBranchCommits + GetAllBranchCommits, + GetStreamCommitCounts, + GetStreamCommitCount, + GetUserStreamCommitCounts, + GetUserAuthoredCommitCounts } from '@/modules/core/domain/commits/operations' const tables = { commits: (db: Knex) => db(Commits.name), branchCommits: (db: Knex) => db(BranchCommits.name), - streamCommits: (db: Knex) => db(StreamCommits.name) + streamCommits: (db: Knex) => db(StreamCommits.name), + streamAcl: (db: Knex) => db(StreamAcl.name) } export const generateCommitId = () => crs({ length: 10 }) @@ -181,41 +187,40 @@ export const insertBranchCommitsFactory = return await q } -export async function getStreamCommitCounts( - streamIds: string[], - options?: Partial<{ ignoreGlobalsBranch: boolean }> -) { - if (!streamIds?.length) return [] +export const getStreamCommitCountsFactory = + (deps: { db: Knex }): GetStreamCommitCounts => + async (streamIds: string[], options?: Partial<{ ignoreGlobalsBranch: boolean }>) => { + if (!streamIds?.length) return [] - const { ignoreGlobalsBranch } = options || {} + const { ignoreGlobalsBranch } = options || {} - const q = StreamCommits.knex() - .select(StreamCommits.col.streamId) - .whereIn(StreamCommits.col.streamId, streamIds) - .count() - .groupBy(StreamCommits.col.streamId) + const q = tables + .streamCommits(deps.db) + .select(StreamCommits.col.streamId) + .whereIn(StreamCommits.col.streamId, streamIds) + .count() + .groupBy(StreamCommits.col.streamId) - if (ignoreGlobalsBranch) { - q.innerJoin( - BranchCommits.name, - StreamCommits.col.commitId, - BranchCommits.col.commitId - ) - .innerJoin(Branches.name, Branches.col.id, BranchCommits.col.branchId) - .andWhereNot(Branches.col.name, 'globals') + if (ignoreGlobalsBranch) { + q.innerJoin( + BranchCommits.name, + StreamCommits.col.commitId, + BranchCommits.col.commitId + ) + .innerJoin(Branches.name, Branches.col.id, BranchCommits.col.branchId) + .andWhereNot(Branches.col.name, 'globals') + } + + const results = (await q) as { streamId: string; count: string }[] + return results.map((r) => ({ ...r, count: parseInt(r.count) })) } - const results = (await q) as { streamId: string; count: string }[] - return results.map((r) => ({ ...r, count: parseInt(r.count) })) -} - -export async function getStreamCommitCount( - streamId: string, - options?: Partial<{ ignoreGlobalsBranch: boolean }> -) { - const [res] = await getStreamCommitCounts([streamId], options) - return res?.count || 0 -} +export const getStreamCommitCountFactory = + (deps: { db: Knex }): GetStreamCommitCount => + async (streamId: string, options?: Partial<{ ignoreGlobalsBranch: boolean }>) => { + const [res] = await getStreamCommitCountsFactory(deps)([streamId], options) + return res?.count || 0 + } export async function getCommitsAndTheirBranchIds(commitIds: string[]) { if (!commitIds.length) return [] @@ -457,62 +462,68 @@ export const getAllBranchCommitsFactory = ) } -export async function getUserStreamCommitCounts(params: { - userIds: string[] - /** - * Only include commits from public/discoverable streams - */ - publicOnly?: boolean -}) { - const { userIds, publicOnly } = params - if (!userIds?.length) return {} +export const getUserStreamCommitCountsFactory = + (deps: { db: Knex }): GetUserStreamCommitCounts => + async (params: { + userIds: string[] + /** + * Only include commits from public/discoverable streams + */ + publicOnly?: boolean + }) => { + const { userIds, publicOnly } = params + if (!userIds?.length) return {} - const q = StreamAcl.knex() - .select<{ userId: string; count: string }[]>([ - StreamAcl.col.userId, - knex.raw('COUNT(*)') - ]) - .join(StreamCommits.name, StreamCommits.col.streamId, StreamAcl.col.resourceId) - .whereIn(StreamAcl.col.userId, userIds) - .groupBy(StreamAcl.col.userId) + const q = tables + .streamAcl(deps.db) + .select<{ userId: string; count: string }[]>([ + StreamAcl.col.userId, + knex.raw('COUNT(*)') + ]) + .join(StreamCommits.name, StreamCommits.col.streamId, StreamAcl.col.resourceId) + .whereIn(StreamAcl.col.userId, userIds) + .groupBy(StreamAcl.col.userId) - if (publicOnly) { - q.join(Streams.name, Streams.col.id, StreamAcl.col.resourceId) - q.andWhere((q1) => { - q1.where(Streams.col.isPublic, true).orWhere(Streams.col.isDiscoverable, true) - }) + if (publicOnly) { + q.join(Streams.name, Streams.col.id, StreamAcl.col.resourceId) + q.andWhere((q1) => { + q1.where(Streams.col.isPublic, true).orWhere(Streams.col.isDiscoverable, true) + }) + } + + const res = await q + return mapValues(keyBy(res, 'userId'), (r) => parseInt(r.count)) } - const res = await q - return mapValues(keyBy(res, 'userId'), (r) => parseInt(r.count)) -} +export const getUserAuthoredCommitCountsFactory = + (deps: { db: Knex }): GetUserAuthoredCommitCounts => + async (params: { + userIds: string[] + /** + * Only include commits from public/discoverable streams + */ + publicOnly?: boolean + }) => { + const { userIds, publicOnly } = params + if (!userIds?.length) return {} -export async function getUserAuthoredCommitCounts(params: { - userIds: string[] - /** - * Only include commits from public/discoverable streams - */ - publicOnly?: boolean -}) { - const { userIds, publicOnly } = params - if (!userIds?.length) return {} + const q = tables + .commits(deps.db) + .select<{ authorId: string; count: string }[]>([ + Commits.col.author, + knex.raw('COUNT(*)') + ]) + .whereIn(Commits.col.author, userIds) + .groupBy(Commits.col.author) - const q = Commits.knex() - .select<{ authorId: string; count: string }[]>([ - Commits.col.author, - knex.raw('COUNT(*)') - ]) - .whereIn(Commits.col.author, userIds) - .groupBy(Commits.col.author) + if (publicOnly) { + q.join(StreamCommits.name, StreamCommits.col.commitId, Commits.col.id) + q.join(Streams.name, Streams.col.id, StreamCommits.col.streamId) + q.andWhere((q1) => { + q1.where(Streams.col.isPublic, true).orWhere(Streams.col.isDiscoverable, true) + }) + } - if (publicOnly) { - q.join(StreamCommits.name, StreamCommits.col.commitId, Commits.col.id) - q.join(Streams.name, Streams.col.id, StreamCommits.col.streamId) - q.andWhere((q1) => { - q1.where(Streams.col.isPublic, true).orWhere(Streams.col.isDiscoverable, true) - }) + const res = await q + return mapValues(keyBy(res, 'author'), (r) => parseInt(r.count)) } - - const res = await q - return mapValues(keyBy(res, 'author'), (r) => parseInt(r.count)) -} diff --git a/packages/server/modules/core/services/commit/retrieval.ts b/packages/server/modules/core/services/commit/retrieval.ts index d5c1a1b15..8a2feb22a 100644 --- a/packages/server/modules/core/services/commit/retrieval.ts +++ b/packages/server/modules/core/services/commit/retrieval.ts @@ -7,12 +7,10 @@ import { getBranchCommitsTotalCount, getPaginatedBranchCommits as getPaginatedBranchCommitsDb, getSpecificBranchCommitsFactory, + getStreamCommitCountFactory, PaginatedBranchCommitsParams } from '@/modules/core/repositories/commits' -import { - getCommitsByStreamId, - getCommitsTotalCountByStreamId -} from '@/modules/core/services/commits' +import { getCommitsByStreamId } from '@/modules/core/services/commits' import { BadRequestError } from '@/modules/shared/errors' import { db } from '@/db/knex' @@ -30,8 +28,7 @@ export async function getPaginatedStreamCommits( cursor: params.cursor, ignoreGlobalsBranch: true }) - const totalCount = await getCommitsTotalCountByStreamId({ - streamId, + const totalCount = await getStreamCommitCountFactory({ db })(streamId, { ignoreGlobalsBranch: true }) diff --git a/packages/server/modules/core/services/commits.js b/packages/server/modules/core/services/commits.js index 8044c6281..0847bd6ef 100644 --- a/packages/server/modules/core/services/commits.js +++ b/packages/server/modules/core/services/commits.js @@ -6,7 +6,6 @@ const Commits = () => knex('commits') const StreamCommits = () => knex('stream_commits') const { - getStreamCommitCount, getPaginatedBranchCommits, getBranchCommitsTotalCount } = require('@/modules/core/repositories/commits') @@ -80,10 +79,6 @@ module.exports = { return module.exports.getCommitsByBranchId({ branchId: myBranch.id, limit, cursor }) }, - async getCommitsTotalCountByStreamId({ streamId, ignoreGlobalsBranch }) { - return await getStreamCommitCount(streamId, { ignoreGlobalsBranch }) - }, - /** * @returns {Promise<{ * commits: import('@/modules/core/helpers/types').CommitRecord[], diff --git a/packages/server/modules/core/tests/commits.spec.js b/packages/server/modules/core/tests/commits.spec.js index cdd971340..717ad5506 100644 --- a/packages/server/modules/core/tests/commits.spec.js +++ b/packages/server/modules/core/tests/commits.spec.js @@ -11,7 +11,6 @@ const { getCommitsTotalCountByBranchName, getCommitsByBranchName, getCommitsByStreamId, - getCommitsTotalCountByStreamId, getCommitsByUserId } = require('../services/commits') const { @@ -36,7 +35,8 @@ const { insertBranchCommitsFactory, getCommitBranchFactory, switchCommitBranchFactory, - updateCommitFactory + updateCommitFactory, + getStreamCommitCountFactory } = require('@/modules/core/repositories/commits') const { deleteCommitAndNotifyFactory, @@ -102,6 +102,7 @@ const updateCommitAndNotify = updateCommitAndNotifyFactory({ markCommitStreamUpdated, markCommitBranchUpdated: markCommitBranchUpdatedFactory({ db }) }) +const getStreamCommitCount = getStreamCommitCountFactory({ db }) describe('Commits @core-commits', () => { const user = { @@ -379,7 +380,7 @@ describe('Commits @core-commits', () => { expect(commits.length).to.equal(10) expect(commits2.length).to.equal(5) - const c = await getCommitsTotalCountByStreamId({ streamId }) + const c = await getStreamCommitCount(streamId) expect(c).to.equal(15) }) From ab6daf714bd9f004d60d9769636a40aa9e30ce1c Mon Sep 17 00:00:00 2001 From: Fabis Date: Tue, 1 Oct 2024 17:13:47 +0100 Subject: [PATCH 07/12] chore(server): core IoC 21 - getCommitsAndTheirBranchIdsFactory --- .../services/commentActivity.ts | 8 +++--- .../comments/graph/resolvers/comments.ts | 4 +-- .../modules/core/domain/commits/operations.ts | 4 +++ .../modules/core/repositories/commits.ts | 26 +++++++++++-------- .../core/services/commit/viewerResources.ts | 4 +-- 5 files changed, 27 insertions(+), 19 deletions(-) diff --git a/packages/server/modules/activitystream/services/commentActivity.ts b/packages/server/modules/activitystream/services/commentActivity.ts index 99bfe1935..978ef1290 100644 --- a/packages/server/modules/activitystream/services/commentActivity.ts +++ b/packages/server/modules/activitystream/services/commentActivity.ts @@ -17,7 +17,7 @@ import { } from '@/modules/core/repositories/branches' import { getAllBranchCommitsFactory, - getCommitsAndTheirBranchIds, + getCommitsAndTheirBranchIdsFactory, getSpecificBranchCommitsFactory } from '@/modules/core/repositories/commits' import { getStreamObjects } from '@/modules/core/repositories/objects' @@ -60,7 +60,7 @@ export async function addCommentCreatedActivity(params: { getViewerResourcesFromLegacyIdentifiers: (...args) => getViewerResourcesFromLegacyIdentifiers(...args) // recursive dep }), - getCommitsAndTheirBranchIds, + getCommitsAndTheirBranchIds: getCommitsAndTheirBranchIdsFactory({ db }), getStreamObjects }) const getViewerResourceItemsUngrouped = getViewerResourceItemsUngroupedFactory({ @@ -146,7 +146,7 @@ export async function addCommentArchivedActivity(params: { getViewerResourcesFromLegacyIdentifiers: (...args) => getViewerResourcesFromLegacyIdentifiers(...args) // recursive dep }), - getCommitsAndTheirBranchIds, + getCommitsAndTheirBranchIds: getCommitsAndTheirBranchIdsFactory({ db }), getStreamObjects }) @@ -208,7 +208,7 @@ export async function addReplyAddedActivity(params: { getViewerResourcesFromLegacyIdentifiers: (...args) => getViewerResourcesFromLegacyIdentifiers(...args) // recursive dep }), - getCommitsAndTheirBranchIds, + getCommitsAndTheirBranchIds: getCommitsAndTheirBranchIdsFactory({ db }), getStreamObjects }) diff --git a/packages/server/modules/comments/graph/resolvers/comments.ts b/packages/server/modules/comments/graph/resolvers/comments.ts index 0426a54ef..72b7cc2d3 100644 --- a/packages/server/modules/comments/graph/resolvers/comments.ts +++ b/packages/server/modules/comments/graph/resolvers/comments.ts @@ -87,7 +87,7 @@ import { getBlobsFactory } from '@/modules/blobstorage/repositories' import { ResourceIdentifier } from '@/modules/comments/domain/types' import { getAllBranchCommitsFactory, - getCommitsAndTheirBranchIds, + getCommitsAndTheirBranchIdsFactory, getSpecificBranchCommitsFactory } from '@/modules/core/repositories/commits' import { getStreamObjects } from '@/modules/core/repositories/objects' @@ -148,7 +148,7 @@ const getViewerResourcesFromLegacyIdentifiers = getViewerResourcesFromLegacyIdentifiers: (...args) => getViewerResourcesFromLegacyIdentifiers(...args) // recursive dep }), - getCommitsAndTheirBranchIds, + getCommitsAndTheirBranchIds: getCommitsAndTheirBranchIdsFactory({ db }), getStreamObjects }) diff --git a/packages/server/modules/core/domain/commits/operations.ts b/packages/server/modules/core/domain/commits/operations.ts index 1d460776a..b815039be 100644 --- a/packages/server/modules/core/domain/commits/operations.ts +++ b/packages/server/modules/core/domain/commits/operations.ts @@ -149,3 +149,7 @@ export type GetUserAuthoredCommitCounts = (params: { }) => Promise<{ [userId: string]: number }> + +export type GetCommitsAndTheirBranchIds = ( + commitIds: string[] +) => Promise diff --git a/packages/server/modules/core/repositories/commits.ts b/packages/server/modules/core/repositories/commits.ts index 542031c60..0b5afdb16 100644 --- a/packages/server/modules/core/repositories/commits.ts +++ b/packages/server/modules/core/repositories/commits.ts @@ -40,7 +40,8 @@ import { GetStreamCommitCounts, GetStreamCommitCount, GetUserStreamCommitCounts, - GetUserAuthoredCommitCounts + GetUserAuthoredCommitCounts, + GetCommitsAndTheirBranchIds } from '@/modules/core/domain/commits/operations' const tables = { @@ -222,17 +223,20 @@ export const getStreamCommitCountFactory = return res?.count || 0 } -export async function getCommitsAndTheirBranchIds(commitIds: string[]) { - if (!commitIds.length) return [] +export const getCommitsAndTheirBranchIdsFactory = + (deps: { db: Knex }): GetCommitsAndTheirBranchIds => + async (commitIds: string[]) => { + if (!commitIds.length) return [] - return await Commits.knex() - .select>([ - ...Commits.cols, - BranchCommits.col.branchId - ]) - .innerJoin(BranchCommits.name, BranchCommits.col.commitId, Commits.col.id) - .whereIn(Commits.col.id, commitIds) -} + return await tables + .commits(deps.db) + .select>([ + ...Commits.cols, + BranchCommits.col.branchId + ]) + .innerJoin(BranchCommits.name, BranchCommits.col.commitId, Commits.col.id) + .whereIn(Commits.col.id, commitIds) + } export const getSpecificBranchCommitsFactory = (deps: { db: Knex }): GetSpecificBranchCommits => diff --git a/packages/server/modules/core/services/commit/viewerResources.ts b/packages/server/modules/core/services/commit/viewerResources.ts index a899f2cfa..4ceda4b7a 100644 --- a/packages/server/modules/core/services/commit/viewerResources.ts +++ b/packages/server/modules/core/services/commit/viewerResources.ts @@ -12,6 +12,7 @@ import { } from '@/modules/core/domain/branches/operations' import { GetAllBranchCommits, + GetCommitsAndTheirBranchIds, GetSpecificBranchCommits } from '@/modules/core/domain/commits/operations' import { @@ -23,7 +24,6 @@ import { ViewerUpdateTrackingTarget } from '@/modules/core/graph/generated/graphql' import { CommitRecord } from '@/modules/core/helpers/types' -import { getCommitsAndTheirBranchIds } from '@/modules/core/repositories/commits' import { getStreamObjects } from '@/modules/core/repositories/objects' import { Optional, SpeckleViewer } from '@speckle/shared' import { flatten, keyBy, reduce, uniq, uniqWith } from 'lodash' @@ -370,7 +370,7 @@ export const getViewerResourcesFromLegacyIdentifiersFactory = ( deps: { getViewerResourcesForComments: GetViewerResourcesForComments - getCommitsAndTheirBranchIds: typeof getCommitsAndTheirBranchIds + getCommitsAndTheirBranchIds: GetCommitsAndTheirBranchIds } & GetObjectResourceGroupsDeps ): GetViewerResourcesFromLegacyIdentifiers => async ( From a4bb99ec45f46d15fb2c822fd656afc63ac2890d Mon Sep 17 00:00:00 2001 From: Fabis Date: Tue, 1 Oct 2024 17:26:45 +0100 Subject: [PATCH 08/12] chore(server): core IoC 22 - cloning related commit repo fns --- .../modules/core/domain/commits/operations.ts | 18 ++++++ .../modules/core/repositories/commits.ts | 61 ++++++++++--------- .../modules/core/services/streams/clone.ts | 12 ++-- 3 files changed, 58 insertions(+), 33 deletions(-) diff --git a/packages/server/modules/core/domain/commits/operations.ts b/packages/server/modules/core/domain/commits/operations.ts index b815039be..549ab4f63 100644 --- a/packages/server/modules/core/domain/commits/operations.ts +++ b/packages/server/modules/core/domain/commits/operations.ts @@ -9,6 +9,7 @@ import { UpdateVersionInput } from '@/modules/core/graph/generated/graphql' import { BranchCommitRecord, StreamCommitRecord } from '@/modules/core/helpers/types' +import { BatchedSelectOptions } from '@/modules/shared/helpers/dbHelper' import { MaybeNullOrUndefined, Nullable, Optional } from '@speckle/shared' import { Knex } from 'knex' @@ -153,3 +154,20 @@ export type GetUserAuthoredCommitCounts = (params: { export type GetCommitsAndTheirBranchIds = ( commitIds: string[] ) => Promise + +export type GetBatchedStreamCommits = ( + streamId: string, + options?: Partial +) => AsyncGenerator + +export type GetBatchedBranchCommits = ( + branchIds: string[], + options?: Partial +) => AsyncGenerator + +export type InsertCommits = ( + commits: Commit[], + options?: Partial<{ + trx: Knex.Transaction + }> +) => Promise diff --git a/packages/server/modules/core/repositories/commits.ts b/packages/server/modules/core/repositories/commits.ts index 0b5afdb16..edae68a5b 100644 --- a/packages/server/modules/core/repositories/commits.ts +++ b/packages/server/modules/core/repositories/commits.ts @@ -41,7 +41,10 @@ import { GetStreamCommitCount, GetUserStreamCommitCounts, GetUserAuthoredCommitCounts, - GetCommitsAndTheirBranchIds + GetCommitsAndTheirBranchIds, + GetBatchedStreamCommits, + GetBatchedBranchCommits, + InsertCommits } from '@/modules/core/domain/commits/operations' const tables = { @@ -133,38 +136,38 @@ export const deleteCommitFactory = return !!delCount } -export function getBatchedStreamCommits( - streamId: string, - options?: Partial -) { - const baseQuery = Commits.knex() - .select(Commits.cols) - .innerJoin(StreamCommits.name, StreamCommits.col.commitId, Commits.col.id) - .where(StreamCommits.col.streamId, streamId) - .orderBy(Commits.col.id) +export const getBatchedStreamCommitsFactory = + (deps: { db: Knex }): GetBatchedStreamCommits => + (streamId: string, options?: Partial) => { + const baseQuery = tables + .commits(deps.db) + .select(Commits.cols) + .innerJoin(StreamCommits.name, StreamCommits.col.commitId, Commits.col.id) + .where(StreamCommits.col.streamId, streamId) + .orderBy(Commits.col.id) - return executeBatchedSelect(baseQuery, options) -} + return executeBatchedSelect(baseQuery, options) + } -export function getBatchedBranchCommits( - branchIds: string[], - options?: Partial -) { - const baseQuery = BranchCommits.knex() - .whereIn(BranchCommits.col.branchId, branchIds) - .orderBy(BranchCommits.col.branchId) +export const getBatchedBranchCommitsFactory = + (deps: { db: Knex }): GetBatchedBranchCommits => + (branchIds: string[], options?: Partial) => { + const baseQuery = tables + .branchCommits(deps.db) + .select('*') + .whereIn(BranchCommits.col.branchId, branchIds) + .orderBy(BranchCommits.col.branchId) - return executeBatchedSelect(baseQuery, options) -} + return executeBatchedSelect(baseQuery, options) + } -export async function insertCommits( - commits: CommitRecord[], - options?: Partial<{ trx: Knex.Transaction }> -) { - const q = Commits.knex().insert(commits) - if (options?.trx) q.transacting(options.trx) - return await q -} +export const insertCommitsFactory = + (deps: { db: Knex }): InsertCommits => + async (commits: CommitRecord[], options?: Partial<{ trx: Knex.Transaction }>) => { + const q = tables.commits(deps.db).insert(commits) + if (options?.trx) q.transacting(options.trx) + return await q + } export const insertStreamCommitsFactory = (deps: { db: Knex }): InsertStreamCommits => diff --git a/packages/server/modules/core/services/streams/clone.ts b/packages/server/modules/core/services/streams/clone.ts index c17352ab6..72bf21301 100644 --- a/packages/server/modules/core/services/streams/clone.ts +++ b/packages/server/modules/core/services/streams/clone.ts @@ -15,12 +15,12 @@ import { insertObjects } from '@/modules/core/repositories/objects' import { - getBatchedStreamCommits, generateCommitId, - insertCommits, - getBatchedBranchCommits, insertStreamCommitsFactory, - insertBranchCommitsFactory + insertBranchCommitsFactory, + getBatchedStreamCommitsFactory, + getBatchedBranchCommitsFactory, + insertCommitsFactory } from '@/modules/core/repositories/commits' import { chunk } from 'lodash' import { @@ -165,6 +165,8 @@ async function cloneCommits(state: CloneStreamInitialState) { // oldCommitId/newCommitId const commitIdMap = new Map() + const insertCommits = insertCommitsFactory({ db }) + const getBatchedStreamCommits = getBatchedStreamCommitsFactory({ db }) for await (const commitsBatch of getBatchedStreamCommits(state.targetStream.id, { trx: state.trx })) { @@ -237,6 +239,8 @@ async function createBranchCommitReferences( branchIdMap: Map ) { const oldBranchIds = [...branchIdMap.keys()] + const getBatchedBranchCommits = getBatchedBranchCommitsFactory({ db }) + for await (const branchCommits of getBatchedBranchCommits(oldBranchIds, { trx: state.trx })) { From 4bcf3cc1d9b181a56fb99acdb617769f29e19c92 Mon Sep 17 00:00:00 2001 From: Alexandru Popovici Date: Mon, 7 Oct 2024 18:16:30 +0300 Subject: [PATCH 09/12] Highlight no longer breaks existing filtering operations (#3187) Co-authored-by: andrewwallacespeckle --- packages/viewer/src/modules/LegacyViewer.ts | 25 +++++++++++++-------- 1 file changed, 16 insertions(+), 9 deletions(-) diff --git a/packages/viewer/src/modules/LegacyViewer.ts b/packages/viewer/src/modules/LegacyViewer.ts index ab25c2beb..b07ba59d0 100644 --- a/packages/viewer/src/modules/LegacyViewer.ts +++ b/packages/viewer/src/modules/LegacyViewer.ts @@ -208,7 +208,7 @@ export class LegacyViewer extends Viewer { ghost = false ): Promise { return new Promise((resolve) => { - const filteringState = this.preserveSelectionFilter(() => { + const filteringState = this.preserveSelectionHighlightFilter(() => { return this.filtering.hideObjects( objectIds, stateKey, @@ -226,7 +226,7 @@ export class LegacyViewer extends Viewer { includeDescendants = false ): Promise { return new Promise((resolve) => { - const filteringState = this.preserveSelectionFilter(() => { + const filteringState = this.preserveSelectionHighlightFilter(() => { return this.filtering.showObjects(objectIds, stateKey, includeDescendants) }) resolve(filteringState) @@ -240,7 +240,7 @@ export class LegacyViewer extends Viewer { ghost = true ): Promise { return new Promise((resolve) => { - const filteringState = this.preserveSelectionFilter(() => { + const filteringState = this.preserveSelectionHighlightFilter(() => { return this.filtering.isolateObjects( objectIds, stateKey, @@ -258,7 +258,7 @@ export class LegacyViewer extends Viewer { includeDescendants = false ): Promise { return new Promise((resolve) => { - const filteringState = this.preserveSelectionFilter(() => { + const filteringState = this.preserveSelectionHighlightFilter(() => { return this.filtering.unIsolateObjects(objectIds, stateKey, includeDescendants) }) resolve(filteringState) @@ -278,7 +278,7 @@ export class LegacyViewer extends Viewer { public setColorFilter(property: PropertyInfo, ghost = true): Promise { return new Promise((resolve) => { - const filteringState = this.preserveSelectionFilter(() => { + const filteringState = this.preserveSelectionHighlightFilter(() => { return this.filtering.setColorFilter(property, ghost) }) resolve(filteringState) @@ -287,7 +287,7 @@ export class LegacyViewer extends Viewer { public removeColorFilter(): Promise { return new Promise((resolve) => { - const filteringState = this.preserveSelectionFilter(() => { + const filteringState = this.preserveSelectionHighlightFilter(() => { return this.filtering.removeColorFilter() }) resolve(filteringState) @@ -298,7 +298,7 @@ export class LegacyViewer extends Viewer { groups: { objectIds: string[]; color: string }[] ): Promise { return new Promise((resolve) => { - const filteringState = this.preserveSelectionFilter(() => { + const filteringState = this.preserveSelectionHighlightFilter(() => { return this.filtering.setUserObjectColors(groups) }) resolve(filteringState) @@ -307,20 +307,25 @@ export class LegacyViewer extends Viewer { public resetFilters(): Promise { return new Promise((resolve) => { - const filteringState = this.preserveSelectionFilter(() => { + const filteringState = this.preserveSelectionHighlightFilter(() => { return this.filtering.resetFilters() }) resolve(filteringState) }) } - private preserveSelectionFilter( + private preserveSelectionHighlightFilter( filterFn: () => FilteringState | null ): FilteringState { const selectedObjects = this.selection .getSelectedObjects() .map((obj) => obj.id) as string[] + const highLightedObjects = this.highlightExtension + .getSelectedObjects() + .map((obj) => obj.id) as string[] if (selectedObjects.length) this.selection.clearSelection() + if (highLightedObjects.length) + this.highlightExtension.unselectObjects(highLightedObjects) const filteringState = filterFn() if (filteringState) { if (!filteringState.selectedObjects) @@ -328,6 +333,8 @@ export class LegacyViewer extends Viewer { this.selection.selectObjects(filteringState.selectedObjects) } + if (highLightedObjects.length) + this.highlightExtension.selectObjects(highLightedObjects) return filteringState || this.filtering.filteringState } From 1e0d625613165a3dba819f56bd629c48918a731e Mon Sep 17 00:00:00 2001 From: Mike Date: Mon, 7 Oct 2024 17:43:38 +0200 Subject: [PATCH 10/12] Fix: Add feedback modal to user nav (#3192) --- .../components/header/NavUserMenu.vue | 21 ++++++++++--------- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/packages/frontend-2/components/header/NavUserMenu.vue b/packages/frontend-2/components/header/NavUserMenu.vue index 4e8dec9b6..ccde2f951 100644 --- a/packages/frontend-2/components/header/NavUserMenu.vue +++ b/packages/frontend-2/components/header/NavUserMenu.vue @@ -78,18 +78,13 @@ Invite to Speckle - - +
@@ -130,6 +125,7 @@ v-model:open="showSettingsDialog" v-model:target-menu-item="settingsDialogTarget" /> +
diff --git a/packages/frontend-2/layouts/default.vue b/packages/frontend-2/layouts/default.vue index 194beb247..7aa1659a9 100644 --- a/packages/frontend-2/layouts/default.vue +++ b/packages/frontend-2/layouts/default.vue @@ -1,7 +1,6 @@