Skip to content

Commit

Permalink
Merge remote-tracking branch 'origin/main' into improvement/async-mem…
Browse files Browse the repository at this point in the history
…ber-affiliations-update
  • Loading branch information
loicsaintroch committed Jan 5, 2024
2 parents b4627b1 + 1f4545e commit f73966e
Show file tree
Hide file tree
Showing 29 changed files with 530 additions and 265 deletions.
3 changes: 3 additions & 0 deletions backend/src/api/tenant/tenantUpdate.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { Error403 } from '@crowd/common'
import TenantService from '../../services/tenantService'
import identifyTenant from '@/segment/identifyTenant'

export default async (req, res) => {
if (!req.currentUser || !req.currentUser.id) {
Expand All @@ -10,5 +11,7 @@ export default async (req, res) => {
// checked inside the service
const payload = await new TenantService(req).update(req.params.id, req.body)

identifyTenant({ ...req, currentTenant: payload })

await req.responseHandler.success(req, res, payload)
}
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
alter table "tenants" drop column "memberMergeSuggestionsLastGeneratedAt";
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
alter table "tenants"
add column "memberMergeSuggestionsLastGeneratedAt" timestamp with time zone null;
61 changes: 33 additions & 28 deletions backend/src/database/repositories/memberRepository.ts
Original file line number Diff line number Diff line change
Expand Up @@ -238,44 +238,49 @@ class MemberRepository {
}

static async findMembersWithMergeSuggestions(
{ limit = 20, offset = 0 },
{ limit = 20, offset = 0, memberId = undefined },
options: IRepositoryOptions,
) {
const currentTenant = SequelizeRepository.getCurrentTenant(options)
const segmentIds = SequelizeRepository.getSegmentIds(options)

const isSegmentsEnabled = await isFeatureEnabled(FeatureFlag.SEGMENTS, options)

const order = isSegmentsEnabled
? 'mtm."activityEstimate" desc, mtm.similarity desc, mtm."memberId", mtm."toMergeId"'
: 'mtm.similarity desc, mtm."activityEstimate" desc, mtm."memberId", mtm."toMergeId"'

const similarityFilter = isSegmentsEnabled ? ' and mtm.similarity > 0.95 ' : ''

const memberFilter = memberId
? ` and (mtm."memberId" = :memberId OR mtm."toMergeId" = :memberId)`
: ''

const mems = await options.database.sequelize.query(
`SELECT
"membersToMerge".id,
"membersToMerge"."toMergeId",
"membersToMerge"."total_count",
"membersToMerge"."similarity",
"membersToMerge"."activityEstimate"
FROM
(
SELECT DISTINCT ON (Greatest(Hashtext(Concat(mem.id, mtm."toMergeId")), Hashtext(Concat(mtm."toMergeId", mem.id))))
mem.id,
mtm."toMergeId",
mem."joinedAt",
COUNT(*) OVER() AS total_count,
mtm."similarity",
mtm."activityEstimate"
FROM members mem
INNER JOIN "memberToMerge" mtm ON mem.id = mtm."memberId"
JOIN "memberSegments" ms ON ms."memberId" = mem.id
WHERE mem."tenantId" = :tenantId
AND ms."segmentId" IN (:segmentIds)
) AS "membersToMerge"
ORDER BY
"membersToMerge"."activityEstimate", "membersToMerge"."similarity" DESC
LIMIT :limit OFFSET :offset
`
select
mtm."memberId" AS id,
mtm."toMergeId",
count(*) over() AS total_count,
mtm.similarity
from
"memberToMerge" mtm
where exists (
select 1
from "memberSegments" ms
where ms."segmentId" in (:segmentIds) and ms."memberId" = mtm."memberId"
${memberFilter}
)
${similarityFilter}
order by ${order}
limit :limit
offset :offset;
`,
{
replacements: {
tenantId: currentTenant.id,
segmentIds,
limit,
offset,
memberId,
},
type: QueryTypes.SELECT,
},
Expand All @@ -297,7 +302,7 @@ class MemberRepository {
members: [i, memberToMergeResults[idx]],
similarity: mems[idx].similarity,
}))
return { rows: result, count: mems[0].total_count / 2, limit, offset }
return { rows: result, count: mems[0].total_count, limit, offset }
}

return { rows: [{ members: [], similarity: 0 }], count: 0, limit, offset }
Expand Down
8 changes: 7 additions & 1 deletion backend/src/database/repositories/organizationRepository.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1445,12 +1445,16 @@ class OrganizationRepository {
}

static async findOrganizationsWithMergeSuggestions(
{ limit = 20, offset = 0 },
{ limit = 20, offset = 0, organizationId = undefined },
options: IRepositoryOptions,
) {
const currentTenant = SequelizeRepository.getCurrentTenant(options)
const segmentIds = SequelizeRepository.getSegmentIds(options)

const organizationFilter = organizationId
? ` AND ("otm"."organizationId" = :organizationId OR "otm"."toMergeId" = :organizationId)`
: ''

const orgs = await options.database.sequelize.query(
`WITH
cte AS (
Expand All @@ -1473,6 +1477,7 @@ class OrganizationRepository {
WHERE org."tenantId" = :tenantId
AND os."segmentId" IN (:segmentIds)
AND (ma.id IS NULL OR ma.state = :mergeActionStatus)
${organizationFilter}
),
count_cte AS (
Expand Down Expand Up @@ -1510,6 +1515,7 @@ class OrganizationRepository {
offset,
mergeActionType: MergeActionType.ORG,
mergeActionStatus: MergeActionState.ERROR,
organizationId,
},
type: QueryTypes.SELECT,
},
Expand Down
9 changes: 6 additions & 3 deletions backend/src/segment/identifyTenant.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,12 @@ export default async function identifyTenant(req) {
analytics.group({
userId: req.currentUser.id,
groupId: req.currentTenant.id,
traits: {
name: req.currentTenant.name,
},
traits:
req.currentTenant.name !== 'temporaryName'
? {
name: req.currentTenant.name,
}
: undefined,
})
}
} else if (API_CONFIG.edition === Edition.COMMUNITY) {
Expand Down
91 changes: 0 additions & 91 deletions backend/src/services/__tests__/tenantService.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,97 +24,6 @@ describe('TenantService tests', () => {
await SequelizeTestUtils.closeConnection(db)
})

describe('findMembersToMerge', () => {
it('Should show the same merge suggestion once, with reverse order', async () => {
const mockIServiceOptions = await SequelizeTestUtils.getTestIServiceOptions(db)
const memberService = new MemberService(mockIServiceOptions)
const tenantService = new TenantService(mockIServiceOptions)

const memberToCreate1 = {
username: {
[PlatformType.SLACK]: {
username: 'member 1',
integrationId: generateUUIDv1(),
},
},
platform: PlatformType.SLACK,
email: '[email protected]',
joinedAt: '2020-05-27T15:13:30Z',
}

const memberToCreate2 = {
username: {
[PlatformType.DISCORD]: {
username: 'member 2',
integrationId: generateUUIDv1(),
},
},
platform: PlatformType.DISCORD,
email: '[email protected]',
joinedAt: '2020-05-26T15:13:30Z',
}

const memberToCreate3 = {
username: {
[PlatformType.GITHUB]: {
username: 'member 3',
integrationId: generateUUIDv1(),
},
},
platform: PlatformType.GITHUB,
email: '[email protected]',
joinedAt: '2020-05-25T15:13:30Z',
}

const memberToCreate4 = {
username: {
[PlatformType.TWITTER]: {
username: 'member 4',
integrationId: generateUUIDv1(),
},
},
platform: PlatformType.TWITTER,
email: '[email protected]',
joinedAt: '2020-05-24T15:13:30Z',
}

const member1 = await memberService.upsert(memberToCreate1)
let member2 = await memberService.upsert(memberToCreate2)
const member3 = await memberService.upsert(memberToCreate3)
let member4 = await memberService.upsert(memberToCreate4)

await memberService.addToMerge([{ members: [member1.id, member2.id], similarity: 1 }])
await memberService.addToMerge([{ members: [member3.id, member4.id], similarity: 0.5 }])

member2 = await memberService.findById(member2.id)
member4 = await memberService.findById(member4.id)

const memberToMergeSuggestions = await tenantService.findMembersToMerge({})

// In the DB there should be:
// - Member 1 should have member 2 in toMerge
// - Member 3 should have member 4 in toMerge
// - Member 4 should have member 3 in toMerge
// - We should get these 4 combinations
// But this function should not return duplicates, so we should get
// only two pairs: [m2, m1] and [m4, m3]

expect(memberToMergeSuggestions.count).toEqual(1)

expect(
memberToMergeSuggestions.rows[0].members
.sort((a, b) => (a.createdAt > b.createdAt ? 1 : -1))
.map((m) => m.id),
).toStrictEqual([member1.id, member2.id])

expect(
memberToMergeSuggestions.rows[1].members
.sort((a, b) => (a.createdAt > b.createdAt ? 1 : -1))
.map((m) => m.id),
).toStrictEqual([member3.id, member4.id])
})
})

describe('_findAndCountAllForEveryUser method', () => {
it('Should succesfully find all tenants without filtering by currentUser', async () => {
let tenants = await TenantService._findAndCountAllForEveryUser({ filter: {} })
Expand Down
135 changes: 135 additions & 0 deletions frontend/src/modules/member/components/member-actions.vue
Original file line number Diff line number Diff line change
@@ -0,0 +1,135 @@
<template>
<div>
<el-button-group class="ml-4">
<!-- Edit contact -->
<el-button class="btn btn--bordered btn--sm !h-8" :disabled="isEditLockedForSampleData" @click="edit()">
<span class="ri-pencil-line text-base mr-2" />Edit contact
</el-button>
<el-button
v-if="mergeSuggestionsCount > 0"
class="btn btn--bordered btn--sm !h-8 !-ml-px !-mr-0.5"
:disabled="isEditLockedForSampleData"
@click="mergeSuggestions()"
>
<span class="mr-2 h-5 px-1.5 rounded-md bg-brand-100 text-brand-500 leading-5">{{ mergeSuggestionsCount }}</span>Merge suggestion
</el-button>

<el-button
v-else
class="btn btn--bordered btn--sm !h-8 !-ml-px !-mr-0.5"
:disabled="isEditLockedForSampleData"
@click="merge()"
>
<span class="ri-shuffle-line text-base mr-2" />Merge
</el-button>
<app-member-dropdown
:member="props.member"
:hide-merge="true"
:hide-edit="true"
@find-github="isFindGithubDrawerOpen = member"
>
<template #trigger>
<el-button class="btn btn--bordered btn--sm !p-2 !h-8 !border-l-2 !border-l-gray-200">
<span class="ri-more-fill text-base" />
</el-button>
</template>
</app-member-dropdown>
</el-button-group>
</div>
<app-member-find-github-drawer
v-if="isFindGithubDrawerOpen"
v-model="isFindGithubDrawerOpen"
/>
<app-member-merge-dialog
v-if="isMergeDialogOpen"
v-model="isMergeDialogOpen"
/>
</template>

<script setup>
import AppMemberDropdown from '@/modules/member/components/member-dropdown.vue';
import {
computed, onMounted, ref, watch,
} from 'vue';
import { MemberPermissions } from '@/modules/member/member-permissions';
import { mapGetters } from '@/shared/vuex/vuex.helpers';
import { useRouter } from 'vue-router';
import AppMemberFindGithubDrawer from '@/modules/member/components/member-find-github-drawer.vue';
import AppMemberMergeDialog from '@/modules/member/components/member-merge-dialog.vue';
import { MemberService } from '@/modules/member/member-service';
const props = defineProps({
member: {
type: Object,
default: () => {},
},
});
const router = useRouter();
const { currentUser, currentTenant } = mapGetters('auth');
const isMergeDialogOpen = ref(null);
const isFindGithubDrawerOpen = ref(null);
const mergeSuggestionsCount = ref(0);
const isEditLockedForSampleData = computed(
() => new MemberPermissions(currentTenant.value, currentUser.value)
.editLockedForSampleData,
);
const fetchMembersToMergeCount = () => {
MemberService.fetchMergeSuggestions(1, 0, {
memberId: props.member.id,
})
.then(({ count }) => {
mergeSuggestionsCount.value = count;
});
};
const edit = () => {
if (isEditLockedForSampleData.value) {
return;
}
router.push({
name: 'memberEdit',
params: {
id: props.member.id,
},
});
};
const mergeSuggestions = () => {
if (isEditLockedForSampleData.value) {
return;
}
router.push({
name: 'memberMergeSuggestions',
query: {
memberId: props.member.id,
},
});
};
const merge = () => {
if (isEditLockedForSampleData.value) {
return;
}
isMergeDialogOpen.value = props.member;
};
watch(() => props.member, () => {
fetchMembersToMergeCount();
});
onMounted(() => {
fetchMembersToMergeCount();
});
</script>

<script>
export default {
name: 'AppMemberActions',
};
</script>

<style lang="scss">
</style>
Loading

0 comments on commit f73966e

Please sign in to comment.