From 54b63fcb837ac0e2a7e9a9412268badb7a99b609 Mon Sep 17 00:00:00 2001 From: Kamlesh Patel Date: Tue, 8 Oct 2024 20:52:01 +0530 Subject: [PATCH 1/6] resolved multi team similar repo issue --- web-server/pages/api/integrations/selected.ts | 91 ------------------- .../api/resources/orgs/[org_id]/teams/v2.ts | 62 ++++++++++--- web-server/src/slices/repos.ts | 16 +--- 3 files changed, 52 insertions(+), 117 deletions(-) delete mode 100644 web-server/pages/api/integrations/selected.ts diff --git a/web-server/pages/api/integrations/selected.ts b/web-server/pages/api/integrations/selected.ts deleted file mode 100644 index 5cae1566a..000000000 --- a/web-server/pages/api/integrations/selected.ts +++ /dev/null @@ -1,91 +0,0 @@ -import { groupBy, mapObjIndexed, prop, uniqBy } from 'ramda'; -import * as yup from 'yup'; - -import { Endpoint, nullSchema } from '@/api-helpers/global'; -import { Table, Row } from '@/constants/db'; -import { Integration } from '@/constants/integrations'; -import { selectedDBReposMock } from '@/mocks/github'; -import { - RepoWithMultipleWorkflows, - RepoWithSingleWorkflow -} from '@/types/resources'; -import { db, dbRaw } from '@/utils/db'; - -const getSchema = yup.object().shape({ - org_id: yup.string().uuid().required(), - providers: yup.array(yup.string().oneOf(Object.values(Integration))) -}); - -const endpoint = new Endpoint(nullSchema); - -endpoint.handle.GET(getSchema, async (req, res) => { - if (req.meta?.features?.use_mock_data) { - return res.send(selectedDBReposMock); - } - - const { org_id, providers } = req.payload; - - res.send(await getSelectedReposForOrg(org_id, providers as Integration[])); -}); - -export const getSelectedReposForOrg = async ( - org_id: ID, - providers: Integration[] -): Promise => { - const dbRepos: RepoWithSingleWorkflow[] = await db(Table.OrgRepo) - .leftJoin({ rw: Table.RepoWorkflow }, function () { - this.on('OrgRepo.id', 'rw.org_repo_id').andOn( - 'rw.is_active', - '=', - dbRaw.raw(true) - ); - }) - .leftJoin({ tr: Table.TeamRepos }, function () { - this.on('OrgRepo.id', 'tr.org_repo_id'); - }) - .select('OrgRepo.*') - .select(dbRaw.raw('to_json(rw) as repo_workflow')) - .select('tr.deployment_type', 'tr.team_id') - .from('OrgRepo') - .where('org_id', org_id) - .and.whereIn('OrgRepo.provider', providers) - .andWhereNot('OrgRepo.is_active', false); - - const repoToWorkflowMap = dbRepos.reduce( - (map, repo) => { - return { - ...map, - [repo.id]: uniqBy( - prop('id'), - [...(map[repo.id] || []), repo.repo_workflow].filter(Boolean) - ) - }; - }, - {} as Record[]> - ); - - const reposGroupedById = mapObjIndexed( - (repos: RepoWithSingleWorkflow[]) => { - return repos.reduce((_, repo) => { - const updatedRepo = { - ...repo, - repo_workflows: repoToWorkflowMap[repo.id] - .map((workflow) => { - return { - name: workflow.name, - value: workflow.provider_workflow_id - }; - }) - .filter((workflow) => workflow.value) - }; - delete updatedRepo.repo_workflow; - - return updatedRepo as any as RepoWithMultipleWorkflows; - }, {} as RepoWithMultipleWorkflows); - }, - groupBy(prop('id'), dbRepos) - ); - return Object.values(reposGroupedById); -}; - -export default endpoint.serve(); diff --git a/web-server/pages/api/resources/orgs/[org_id]/teams/v2.ts b/web-server/pages/api/resources/orgs/[org_id]/teams/v2.ts index d32983e5a..07d64e014 100644 --- a/web-server/pages/api/resources/orgs/[org_id]/teams/v2.ts +++ b/web-server/pages/api/resources/orgs/[org_id]/teams/v2.ts @@ -6,7 +6,6 @@ import { } from 'ramda'; import * as yup from 'yup'; -import { getSelectedReposForOrg } from '@/api/integrations/selected'; import { syncReposForOrg } from '@/api/internal/[org_id]/sync_repos'; import { getOnBoardingState, @@ -22,8 +21,12 @@ import { } from '@/constants/integrations'; import { getTeamV2Mock } from '@/mocks/teams'; import { BaseTeam } from '@/types/api/teams'; -import { OnboardingStep, ReqRepoWithProvider } from '@/types/resources'; -import { db, getFirstRow } from '@/utils/db'; +import { + OnboardingStep, + RepoWithMultipleWorkflows, + ReqRepoWithProvider +} from '@/types/resources'; +import { db, dbRaw, getFirstRow } from '@/utils/db'; import groupBy from '@/utils/objectArray'; const repoSchema = yup.object().shape({ @@ -84,17 +87,16 @@ endpoint.handle.GET(getSchema, async (req, res) => { .orderBy('name', 'asc'); const teams = await getQuery; - const reposWithWorkflows = await getSelectedReposForOrg( + const teamReposMap = await getTeamReposMap( org_id, providers?.length ? (providers as Integration[]) : [Integration.GITHUB, Integration.GITLAB] - ).then((res) => res.flat()); + ); res.send({ teams: teams, - teamReposMap: ramdaGroupBy(prop('team_id'), reposWithWorkflows), - reposWithWorkflows + teamReposMap }); }); @@ -134,7 +136,7 @@ endpoint.handle.POST(postSchema, async (req, res) => { const providers = Array.from(new Set(orgReposList.map((r) => r.provider))); await updateReposWorkflows(org_id, orgReposList); - const reposWithWorkflows = await getSelectedReposForOrg( + const teamReposMap = await getTeamReposMap( org_id, providers as Integration[] ); @@ -143,7 +145,7 @@ endpoint.handle.POST(postSchema, async (req, res) => { res.send({ team, - teamReposMap: ramdaGroupBy(prop('team_id'), reposWithWorkflows) + teamReposMap }); }); @@ -177,14 +179,14 @@ endpoint.handle.PATCH(patchSchema, async (req, res) => { const providers = Array.from(new Set(orgReposList.map((r) => r.provider))); - const reposWithWorkflows = await getSelectedReposForOrg( + const teamReposMap = await getTeamReposMap( org_id, providers as Integration[] ); syncReposForOrg(); res.send({ team, - teamReposMap: ramdaGroupBy(prop('team_id'), reposWithWorkflows) + teamReposMap }); }); @@ -200,6 +202,10 @@ endpoint.handle.DELETE(deleteSchema, async (req, res) => { .returning('*') .then(getFirstRow); + await db('TeamRepos') + .update('is_active', false) + .where('team_id', req.payload.id); + res.send(data); }); @@ -302,3 +308,37 @@ const updateReposWorkflows = async ( } } }; + +const getTeamReposMap = async (org_id: ID, providers: Integration[]) => { + const dbRepos: RepoWithMultipleWorkflows[] = await db(Table.OrgRepo) + .leftJoin({ tr: Table.TeamRepos }, function () { + this.on('OrgRepo.id', 'tr.org_repo_id'); + }) + .where('tr.is_active', true) + .leftJoin({ rw: Table.RepoWorkflow }, function () { + this.on('OrgRepo.id', 'rw.org_repo_id').andOn( + 'rw.is_active', + '=', + dbRaw.raw(true) + ); + }) + .select('OrgRepo.*') + .select( + dbRaw.raw( + "COALESCE(json_agg(json_build_object('name', rw.name, 'value', rw.provider_workflow_id)) FILTER (WHERE rw.id IS NOT NULL), '[]') as repo_workflows" + ) + ) + .select('tr.deployment_type', 'tr.team_id') + .from('OrgRepo') + .where('org_id', org_id) + .and.whereIn('OrgRepo.provider', providers) + .andWhereNot('OrgRepo.is_active', false) + .groupBy( + 'OrgRepo.id', + 'rw.org_repo_id', + 'tr.deployment_type', + 'tr.team_id' + ); + + return ramdaGroupBy(prop('team_id'), dbRepos); +}; diff --git a/web-server/src/slices/repos.ts b/web-server/src/slices/repos.ts index d28351682..b2a2c6327 100644 --- a/web-server/src/slices/repos.ts +++ b/web-server/src/slices/repos.ts @@ -8,11 +8,7 @@ import { Integration } from '@/constants/integrations'; import { RootState } from '@/store/index'; import { LoadedOrg } from '@/types/github'; import { StateFetchConfig } from '@/types/redux'; -import { - BaseRepo, - RepoUniqueDetails, - RepoWithMultipleWorkflows -} from '@/types/resources'; +import { BaseRepo, RepoUniqueDetails } from '@/types/resources'; import { addFetchCasesToReducer } from '@/utils/redux'; type RepoSelectionMap = Record; @@ -155,13 +151,3 @@ export const fetchReposForOrgFromProvider = createAsyncThunk( ); } ); - -export const fetchSelectedRepos = createAsyncThunk( - 'repos/fetchSelectedRepos', - async (params: { orgId: ID; provider: Integration }) => { - return await handleApi( - `/integrations/selected`, - { params: { org_id: params.orgId, provider: params.provider } } - ); - } -); From eea4f8579d6da145080d588e34109ff88475d30d Mon Sep 17 00:00:00 2001 From: Kamlesh Patel Date: Thu, 10 Oct 2024 20:38:11 +0530 Subject: [PATCH 2/6] Updated teams repo query for better readability --- .../api/resources/orgs/[org_id]/teams/v2.ts | 55 +++++++++++-------- 1 file changed, 33 insertions(+), 22 deletions(-) diff --git a/web-server/pages/api/resources/orgs/[org_id]/teams/v2.ts b/web-server/pages/api/resources/orgs/[org_id]/teams/v2.ts index 07d64e014..b0a44293a 100644 --- a/web-server/pages/api/resources/orgs/[org_id]/teams/v2.ts +++ b/web-server/pages/api/resources/orgs/[org_id]/teams/v2.ts @@ -310,35 +310,46 @@ const updateReposWorkflows = async ( }; const getTeamReposMap = async (org_id: ID, providers: Integration[]) => { - const dbRepos: RepoWithMultipleWorkflows[] = await db(Table.OrgRepo) - .leftJoin({ tr: Table.TeamRepos }, function () { - this.on('OrgRepo.id', 'tr.org_repo_id'); - }) - .where('tr.is_active', true) - .leftJoin({ rw: Table.RepoWorkflow }, function () { + const baseQuery = db(Table.OrgRepo) + .where('org_id', org_id) + .andWhere('OrgRepo.is_active', true) + .whereIn('OrgRepo.provider', providers); + + const teamJoin = baseQuery + .leftJoin({ tr: Table.TeamRepos }, 'OrgRepo.id', 'tr.org_repo_id') + .andWhere('tr.is_active', true); + + const workflowJoin = teamJoin.leftJoin( + { rw: Table.RepoWorkflow }, + function () { this.on('OrgRepo.id', 'rw.org_repo_id').andOn( 'rw.is_active', - '=', - dbRaw.raw(true) + 'tr.is_active' ); - }) + } + ); + + const dbRepos: RepoWithMultipleWorkflows[] = await workflowJoin .select('OrgRepo.*') .select( dbRaw.raw( - "COALESCE(json_agg(json_build_object('name', rw.name, 'value', rw.provider_workflow_id)) FILTER (WHERE rw.id IS NOT NULL), '[]') as repo_workflows" + "COALESCE(json_agg(rw.*) FILTER (WHERE rw.id IS NOT NULL), '[]') as repo_workflows" ) ) .select('tr.deployment_type', 'tr.team_id') - .from('OrgRepo') - .where('org_id', org_id) - .and.whereIn('OrgRepo.provider', providers) - .andWhereNot('OrgRepo.is_active', false) - .groupBy( - 'OrgRepo.id', - 'rw.org_repo_id', - 'tr.deployment_type', - 'tr.team_id' - ); - - return ramdaGroupBy(prop('team_id'), dbRepos); + .groupBy('OrgRepo.id', 'tr.deployment_type', 'tr.team_id'); + + const repoWithWorkflows = dbRepos.map((repo) => { + const updatedWorkflows = repo.repo_workflows.map((workflow) => ({ + name: workflow.name, + value: workflow.provider_workflow_id + })); + + return { + ...repo, + repo_workflows: updatedWorkflows + }; + }); + + return ramdaGroupBy(prop('team_id'), repoWithWorkflows); }; From c718b3c197c1c55596a9ad98bb707d0b69f8af55 Mon Sep 17 00:00:00 2001 From: Kamlesh Patel Date: Fri, 11 Oct 2024 03:16:34 +0530 Subject: [PATCH 3/6] mark corresponding TeamRepo and OrgRepo inactive if a team is deleted --- .../api/resources/orgs/[org_id]/teams/v2.ts | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/web-server/pages/api/resources/orgs/[org_id]/teams/v2.ts b/web-server/pages/api/resources/orgs/[org_id]/teams/v2.ts index b0a44293a..dbe443a7a 100644 --- a/web-server/pages/api/resources/orgs/[org_id]/teams/v2.ts +++ b/web-server/pages/api/resources/orgs/[org_id]/teams/v2.ts @@ -202,9 +202,22 @@ endpoint.handle.DELETE(deleteSchema, async (req, res) => { .returning('*') .then(getFirstRow); - await db('TeamRepos') + const activeTeamReposIds = await db('TeamRepos') .update('is_active', false) - .where('team_id', req.payload.id); + .where('team_id', req.payload.id) + .andWhere('is_active', true) + .returning('org_repo_id') + .then((result) => result.map((row) => row.org_repo_id)); + + const activeOrgReposIds = await db('TeamRepos') + .where('is_active', true) + .whereIn('org_repo_id', activeTeamReposIds) + .then((result) => result.map((row) => row.org_repo_id)); + + await db('OrgRepo') + .update('is_active', false) + .whereIn('id', activeTeamReposIds) + .whereNotIn('id', activeOrgReposIds); res.send(data); }); From 2273464e80e370bb89e2bd920828973448dada44 Mon Sep 17 00:00:00 2001 From: Kamlesh Patel Date: Fri, 11 Oct 2024 10:09:08 +0530 Subject: [PATCH 4/6] updated delete team api --- .../pages/api/resources/orgs/[org_id]/teams/v2.ts | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/web-server/pages/api/resources/orgs/[org_id]/teams/v2.ts b/web-server/pages/api/resources/orgs/[org_id]/teams/v2.ts index dbe443a7a..df5a77299 100644 --- a/web-server/pages/api/resources/orgs/[org_id]/teams/v2.ts +++ b/web-server/pages/api/resources/orgs/[org_id]/teams/v2.ts @@ -202,22 +202,26 @@ endpoint.handle.DELETE(deleteSchema, async (req, res) => { .returning('*') .then(getFirstRow); - const activeTeamReposIds = await db('TeamRepos') + const inactiveTeamReposIds: string[] = await db('TeamRepos') .update('is_active', false) .where('team_id', req.payload.id) .andWhere('is_active', true) .returning('org_repo_id') .then((result) => result.map((row) => row.org_repo_id)); - const activeOrgReposIds = await db('TeamRepos') + const activeOrgReposIds: string[] = await db('TeamRepos') .where('is_active', true) - .whereIn('org_repo_id', activeTeamReposIds) + .whereIn('org_repo_id', inactiveTeamReposIds) + .distinct('org_repo_id') .then((result) => result.map((row) => row.org_repo_id)); + const orgReposToBeInactivate = inactiveTeamReposIds.filter( + (id) => !activeOrgReposIds.includes(id) + ); + await db('OrgRepo') .update('is_active', false) - .whereIn('id', activeTeamReposIds) - .whereNotIn('id', activeOrgReposIds); + .whereIn('id', orgReposToBeInactivate); res.send(data); }); From 01b09e69d6e97d26a0fe8ea7055abdcbeb9a4942 Mon Sep 17 00:00:00 2001 From: Kamlesh Patel Date: Sun, 13 Oct 2024 00:08:58 +0530 Subject: [PATCH 5/6] added transaction for team deletion --- .../api/resources/orgs/[org_id]/teams/v2.ts | 67 +++++++++++-------- 1 file changed, 40 insertions(+), 27 deletions(-) diff --git a/web-server/pages/api/resources/orgs/[org_id]/teams/v2.ts b/web-server/pages/api/resources/orgs/[org_id]/teams/v2.ts index df5a77299..b49eebd4e 100644 --- a/web-server/pages/api/resources/orgs/[org_id]/teams/v2.ts +++ b/web-server/pages/api/resources/orgs/[org_id]/teams/v2.ts @@ -195,33 +195,46 @@ endpoint.handle.DELETE(deleteSchema, async (req, res) => { return res.send(getTeamV2Mock); } - const data = await db('Team') - .update('is_deleted', true) - .where('id', req.payload.id) - .orderBy('name', 'asc') - .returning('*') - .then(getFirstRow); - - const inactiveTeamReposIds: string[] = await db('TeamRepos') - .update('is_active', false) - .where('team_id', req.payload.id) - .andWhere('is_active', true) - .returning('org_repo_id') - .then((result) => result.map((row) => row.org_repo_id)); - - const activeOrgReposIds: string[] = await db('TeamRepos') - .where('is_active', true) - .whereIn('org_repo_id', inactiveTeamReposIds) - .distinct('org_repo_id') - .then((result) => result.map((row) => row.org_repo_id)); - - const orgReposToBeInactivate = inactiveTeamReposIds.filter( - (id) => !activeOrgReposIds.includes(id) - ); - - await db('OrgRepo') - .update('is_active', false) - .whereIn('id', orgReposToBeInactivate); + const data = await dbRaw.transaction(async (trx) => { + const deletedTeamRow = await trx('Team') + .update({ + is_deleted: true, + updated_at: new Date() + }) + .where('id', req.payload.id) + .orderBy('name', 'asc') + .returning('*') + .then(getFirstRow); + + const inactiveTeamReposIds: string[] = await trx('TeamRepos') + .update({ + is_active: false, + updated_at: new Date() + }) + .where('team_id', req.payload.id) + .andWhere('is_active', true) + .returning('org_repo_id') + .then((result) => result.map((row) => row.org_repo_id)); + + const activeOrgReposIds: string[] = await trx('TeamRepos') + .where('is_active', true) + .whereIn('org_repo_id', inactiveTeamReposIds) + .distinct('org_repo_id') + .then((result) => result.map((row) => row.org_repo_id)); + + const orgReposToBeInactivate = inactiveTeamReposIds.filter( + (id) => !activeOrgReposIds.includes(id) + ); + + await trx('OrgRepo') + .update({ + is_active: false, + updated_at: new Date() + }) + .whereIn('id', orgReposToBeInactivate); + + return deletedTeamRow; + }); res.send(data); }); From 26d47960c10e789bd8efbdd12f739e7c3a50c4e1 Mon Sep 17 00:00:00 2001 From: Kamlesh Patel Date: Mon, 14 Oct 2024 11:53:30 +0530 Subject: [PATCH 6/6] Added comments for the logic of deleting a team --- .../pages/api/resources/orgs/[org_id]/teams/v2.ts | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/web-server/pages/api/resources/orgs/[org_id]/teams/v2.ts b/web-server/pages/api/resources/orgs/[org_id]/teams/v2.ts index b49eebd4e..bf0db917e 100644 --- a/web-server/pages/api/resources/orgs/[org_id]/teams/v2.ts +++ b/web-server/pages/api/resources/orgs/[org_id]/teams/v2.ts @@ -206,7 +206,8 @@ endpoint.handle.DELETE(deleteSchema, async (req, res) => { .returning('*') .then(getFirstRow); - const inactiveTeamReposIds: string[] = await trx('TeamRepos') + // 1. Mark inactive and Get Repo Ids of this deleted team. Ex: [ '123', '456', '789' ] + const deletedTeamRepoIds: string[] = await trx('TeamRepos') .update({ is_active: false, updated_at: new Date() @@ -216,14 +217,16 @@ endpoint.handle.DELETE(deleteSchema, async (req, res) => { .returning('org_repo_id') .then((result) => result.map((row) => row.org_repo_id)); - const activeOrgReposIds: string[] = await trx('TeamRepos') + // 2. Get Repo Ids which are still used across other teams. Ex: [ '456', '789' ] + const activeRepoIds: string[] = await trx('TeamRepos') .where('is_active', true) - .whereIn('org_repo_id', inactiveTeamReposIds) + .whereIn('org_repo_id', deletedTeamRepoIds) .distinct('org_repo_id') .then((result) => result.map((row) => row.org_repo_id)); - const orgReposToBeInactivate = inactiveTeamReposIds.filter( - (id) => !activeOrgReposIds.includes(id) + // 3. Repo Ids which are nowhere used. Ex: [ '123' ] + const orgRepoIdsToBeInactivate = deletedTeamRepoIds.filter( + (id) => !activeRepoIds.includes(id) ); await trx('OrgRepo') @@ -231,7 +234,7 @@ endpoint.handle.DELETE(deleteSchema, async (req, res) => { is_active: false, updated_at: new Date() }) - .whereIn('id', orgReposToBeInactivate); + .whereIn('id', orgRepoIdsToBeInactivate); return deletedTeamRow; });