Skip to content

Improvement/stricter serp api enrichment filter #2670

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
epipav merged 9 commits into from
Nov 5, 2024
Merged

Improvement/stricter serp api enrichment filter #2670

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
842aa4f
enrich members with 100 activities serp api
epipav Nov 4, 2024
3d7f7b8
more strict
epipav Nov 4, 2024
52ecd05
support for credit checking, stricter serp api conditions, add activi…
epipav Nov 4, 2024
9d44c1c
Merge branch 'main' into improvement/stricter-serp-api-enrichment-filter
epipav Nov 4, 2024
66f54f0
handle axios errors when getting remaining credits
epipav Nov 4, 2024
aa80955
Merge branch 'improvement/stricter-serp-api-enrichment-filter' of git…
epipav Nov 4, 2024
5e47a45
selecting among multiple gh identities when enriching, prioritizing m…
epipav Nov 5, 2024
abb976d
use the new afterCursor when scheduling member enrichment
epipav Nov 5, 2024
0808d9a
Merge branch 'main' into improvement/stricter-serp-api-enrichment-filter
epipav Nov 5, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions services/apps/premium/members_enrichment_worker/src/activities.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import {
findMemberEnrichmentCache,
findMemberIdentityWithTheMostActivityInPlatform,
Copy link

@coderabbitai coderabbitai bot Nov 5, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codebase verification

Function needs unit tests

The implementation of findMemberIdentityWithTheMostActivityInPlatform was found in services/libs/data-access-layer/src/activities/sql.ts, but no corresponding test files were discovered. Since this is a new backend functionality that executes SQL queries, it should be properly tested.

  • Add unit tests for findMemberIdentityWithTheMostActivityInPlatform in services/libs/data-access-layer/src/activities/sql.test.ts
    • Test the SQL query execution with different scenarios
    • Verify the correct aggregation of activity counts
    • Check handling of edge cases (no activities, multiple usernames)
🔗 Analysis chain

LGTM! Changes follow established patterns.

The addition of findMemberIdentityWithTheMostActivityInPlatform to imports and exports is clean and maintains consistent ordering.

Let's verify the test coverage for this new functionality:

Also applies to: 54-54

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check if tests exist for the new function

# Test 1: Look for test files related to the function
fd -g "*test*" -g "*spec*" | rg -l "findMemberIdentityWithTheMostActivityInPlatform"

# Test 2: Check the implementation and its tests in data-access-layer
rg -A 5 "findMemberIdentityWithTheMostActivityInPlatform" "services/libs/data-access-layer/src"
rg -A 5 "findMemberIdentityWithTheMostActivityInPlatform" "services/libs/data-access-layer/test"

Length of output: 977

Script:

#!/bin/bash
# Let's try a different approach to find test files

# Search for test files in the activities directory
fd -g "*test*" -g "*spec*" -p "services/libs/data-access-layer"

# Check content of any test files in the activities directory
rg -A 5 "activities/sql" "services/libs/data-access-layer"

# Look for the function implementation to understand its complexity
ast-grep --pattern 'export async function findMemberIdentityWithTheMostActivityInPlatform($_) {
  $$$
}'

Length of output: 280

Script:

#!/bin/bash
# Let's try one final approach to find tests

# Search for test files in data-access-layer
fd "test" "services/libs/data-access-layer"

# Look for SQL implementation details
cat services/libs/data-access-layer/src/activities/sql.ts

# Check if there are any test files in the activities folder
fd "test" "services/libs/data-access-layer/src/activities"

Length of output: 36040

getEnrichmentData,
insertMemberEnrichmentCache,
isCacheObsolete,
Expand Down Expand Up @@ -50,4 +51,5 @@ export {
touchMemberEnrichmentCacheUpdatedAt,
updateMemberEnrichmentCache,
isEnrichableBySource,
findMemberIdentityWithTheMostActivityInPlatform,
}
52 changes: 50 additions & 2 deletions services/apps/premium/members_enrichment_worker/src/activities/enrichment.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,10 +1,16 @@
import { findMemberIdentityWithTheMostActivityInPlatform as findMemberIdentityWithTheMostActivityInPlatformQuestDb } from '@crowd/data-access-layer/src/activities'
import {
findMemberEnrichmentCacheDb,
insertMemberEnrichmentCacheDb,
touchMemberEnrichmentCacheUpdatedAtDb,
updateMemberEnrichmentCacheDb,
} from '@crowd/data-access-layer/src/old/apps/premium/members_enrichment_worker'
import { IMemberEnrichmentCache, MemberEnrichmentSource } from '@crowd/types'
import { RedisCache } from '@crowd/redis'
import {
IEnrichableMemberIdentityActivityAggregate,
IMemberEnrichmentCache,
MemberEnrichmentSource,
} from '@crowd/types'

import { EnrichmentSourceServiceFactory } from '../factory'
import { svc } from '../main'
Expand All @@ -27,7 +33,7 @@ export async function getEnrichmentData(
input: IEnrichmentSourceInput,
): Promise<IMemberEnrichmentData | null> {
const service = EnrichmentSourceServiceFactory.getEnrichmentSourceService(source, svc.log)
if (service.isEnrichableBySource(input)) {
if (service.isEnrichableBySource(input) && (await hasRemainingCredits(source))) {
return service.getData(input)
}
return null
Expand All @@ -52,6 +58,41 @@ export async function isCacheObsolete(
)
}

export async function setHasRemainingCredits(
source: MemberEnrichmentSource,
hasCredits: boolean,
): Promise<void> {
const redisCache = new RedisCache(`enrichment-${source}`, svc.redis, svc.log)
if (hasCredits) {
await redisCache.set('hasRemainingCredits', 'true', 60)
} else {
await redisCache.set('hasRemainingCredits', 'false', 60)
}
}

export async function getHasRemainingCredits(source: MemberEnrichmentSource): Promise<boolean> {
const redisCache = new RedisCache(`enrichment-${source}`, svc.redis, svc.log)
return (await redisCache.get('hasRemainingCredits')) === 'true'
}
Comment on lines +73 to +76
Copy link

@coderabbitai coderabbitai bot Nov 4, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Handle Missing Cache Entries in getHasRemainingCredits

If the cache key does not exist, redisCache.get may return null, leading to unexpected behavior when comparing with 'true'.

Modify the function to handle null values explicitly:

export async function getHasRemainingCredits(source: MemberEnrichmentSource): Promise<boolean> {
  const redisCache = new RedisCache(`enrichment-${source}`, svc.redis, svc.log)
- return (await redisCache.get('hasRemainingCredits')) === 'true'
+ const value = await redisCache.get('hasRemainingCredits')
+ if (value === 'true') {
+   return true
+ } else if (value === 'false') {
+   return false
+ } else {
+   return false // or consider throwing an error if appropriate
+ }
}
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
export async function getHasRemainingCredits(source: MemberEnrichmentSource): Promise<boolean> {
const redisCache = new RedisCache(`enrichment-${source}`, svc.redis, svc.log)
return (await redisCache.get('hasRemainingCredits')) === 'true'
}
export async function getHasRemainingCredits(source: MemberEnrichmentSource): Promise<boolean> {
const redisCache = new RedisCache(`enrichment-${source}`, svc.redis, svc.log)
const value = await redisCache.get('hasRemainingCredits')
if (value === 'true') {
return true
} else if (value === 'false') {
return false
} else {
return false // or consider throwing an error if appropriate
}
}


export async function hasRemainingCreditsExists(source: MemberEnrichmentSource): Promise<boolean> {
const redisCache = new RedisCache(`enrichment-${source}`, svc.redis, svc.log)
return await redisCache.exists('hasRemainingCredits')
}

export async function hasRemainingCredits(source: MemberEnrichmentSource): Promise<boolean> {
const service = EnrichmentSourceServiceFactory.getEnrichmentSourceService(source, svc.log)

if (await hasRemainingCreditsExists(source)) {
return getHasRemainingCredits(source)
}

const hasCredits = await service.hasRemainingCredits()

await setHasRemainingCredits(source, hasCredits)
return hasCredits
}
Comment on lines +83 to +94
Copy link

@coderabbitai coderabbitai bot Nov 4, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Optimize hasRemainingCredits Function Logic

The current implementation of hasRemainingCredits involves multiple function calls and checks, which can be streamlined for better performance and readability.

Refactor the function as follows:

export async function hasRemainingCredits(source: MemberEnrichmentSource): Promise<boolean> {
  const cachedValue = await getHasRemainingCredits(source)
- if (await hasRemainingCreditsExists(source)) {
-   return getHasRemainingCredits(source)
- }
+ if (cachedValue !== null) {
+   return cachedValue
+ }

  const service = EnrichmentSourceServiceFactory.getEnrichmentSourceService(source, svc.log)
  const hasCredits = await service.hasRemainingCredits()

  await setHasRemainingCredits(source, hasCredits)
  return hasCredits
}
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
export async function hasRemainingCredits(source: MemberEnrichmentSource): Promise<boolean> {
const service = EnrichmentSourceServiceFactory.getEnrichmentSourceService(source, svc.log)
if (await hasRemainingCreditsExists(source)) {
return getHasRemainingCredits(source)
}
const hasCredits = await service.hasRemainingCredits()
await setHasRemainingCredits(source, hasCredits)
return hasCredits
}
export async function hasRemainingCredits(source: MemberEnrichmentSource): Promise<boolean> {
const cachedValue = await getHasRemainingCredits(source)
if (cachedValue !== null) {
return cachedValue
}
const service = EnrichmentSourceServiceFactory.getEnrichmentSourceService(source, svc.log)
const hasCredits = await service.hasRemainingCredits()
await setHasRemainingCredits(source, hasCredits)
return hasCredits
}


export async function findMemberEnrichmentCache(
source: MemberEnrichmentSource,
memberId: string,
Expand Down Expand Up @@ -81,3 +122,10 @@ export async function touchMemberEnrichmentCacheUpdatedAt(
): Promise<void> {
await touchMemberEnrichmentCacheUpdatedAtDb(svc.postgres.writer.connection(), memberId, source)
}

export async function findMemberIdentityWithTheMostActivityInPlatform(
memberId: string,
platform: string,
): Promise<IEnrichableMemberIdentityActivityAggregate> {
return findMemberIdentityWithTheMostActivityInPlatformQuestDb(svc.questdbSQL, memberId, platform)
}
4 changes: 2 additions & 2 deletions services/apps/premium/members_enrichment_worker/src/activities/getMembers.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import { svc } from '../main'
export async function getEnrichableMembers(
limit: number,
sources: MemberEnrichmentSource[],
afterId: string,
afterCursor: { activityCount: number; memberId: string } | null,
): Promise<IEnrichableMember[]> {
let rows: IEnrichableMember[] = []
const sourceInputs: IMemberEnrichmentSourceQueryInput[] = sources.map((s) => {
Expand All @@ -23,7 +23,7 @@ export async function getEnrichableMembers(
}
})
const db = svc.postgres.reader
rows = await fetchMembersForEnrichment(db, limit, sourceInputs, afterId)
rows = await fetchMembersForEnrichment(db, limit, sourceInputs, afterCursor)

return rows
}
8 changes: 7 additions & 1 deletion services/apps/premium/members_enrichment_worker/src/sources/clearbit/service.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,9 @@ import {
export default class EnrichmentServiceClearbit extends LoggerBase implements IEnrichmentService {
public source: MemberEnrichmentSource = MemberEnrichmentSource.CLEARBIT
public platform = `enrichment-${this.source}`
public enrichableBySql = `mi.type = 'email' and mi.verified`
public enrichMembersWithActivityMoreThan = 10

public enrichableBySql = `"activitySummary".total_count > ${this.enrichMembersWithActivityMoreThan} AND mi.type = 'email' and mi.verified`

// bust cache after 120 days
public cacheObsoleteAfterSeconds = 60 * 60 * 24 * 120
Expand Down Expand Up @@ -63,6 +65,10 @@ export default class EnrichmentServiceClearbit extends LoggerBase implements IEn
return enriched
}

async hasRemainingCredits(): Promise<boolean> {
return true
}
Comment on lines +68 to +70
Copy link

@coderabbitai coderabbitai bot Nov 4, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codebase verification

⚠️ Potential issue

Implement actual credit checking logic similar to SERP service

The current implementation always returns true, while other services like SERP implement proper credit checking. For Clearbit service, consider implementing similar logic:

-  async hasRemainingCredits(): Promise<boolean> {
-    return true
-  }
+  async hasRemainingCredits(): Promise<boolean> {
+    try {
+      const response = await axios.get('https://api.clearbit.com/v1/remaining_credits', {
+        auth: {
+          username: process.env['CROWD_ENRICHMENT_CLEARBIT_API_KEY'],
+          password: ''
+        }
+      });
+      return response.data.remaining > 0;
+    } catch (error) {
+      this.log.error('Failed to check Clearbit credits:', error);
+      return false;
+    }
+  }
  • The SERP service properly checks remaining credits via their API
  • The ProgAI service also needs proper implementation
  • Credit checking is crucial as mentioned in the interface comments: "if returned false the source will be skipped"
🔗 Analysis chain

Implement actual credit checking logic.

The current implementation always returns true, which defeats the purpose of credit management. This should be implemented to actually check Clearbit API credits.

Let's verify if other enrichment services have proper credit checking implemented:

Consider implementing proper credit checking:

-  async hasRemainingCredits(): Promise<boolean> {
-    return true
-  }
+  async hasRemainingCredits(): Promise<boolean> {
+    try {
+      // Add actual credit checking logic here
+      // Example: Check Clearbit API quota/usage endpoints
+      return true
+    } catch (err) {
+      this.log.error(`Failed to check Clearbit credits: ${err}`)
+      return false
+    }
+  }
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for hasRemainingCredits implementations in other enrichment services
ast-grep --pattern 'async hasRemainingCredits(): Promise<boolean> {
  $$$
}'

Length of output: 78

Script:

#!/bin/bash
# Let's try a broader search for hasRemainingCredits implementations
rg -l "hasRemainingCredits"

# Then let's look at the interface definition
rg "interface.*EnrichmentService" -A 10

Length of output: 1616

Script:

#!/bin/bash
# Let's examine the implementations in other services
rg "hasRemainingCredits.*Promise<boolean>" -A 5 services/apps/premium/members_enrichment_worker/src/sources/serp/service.ts services/apps/premium/members_enrichment_worker/src/sources/progai/service.ts

Length of output: 1554

Script:

#!/bin/bash
# Let's see the complete SERP implementation to understand the credit checking logic
rg "hasRemainingCredits.*Promise<boolean>" -A 15 services/apps/premium/members_enrichment_worker/src/sources/serp/service.ts

Length of output: 622


private async getDataUsingEmail(email: string): Promise<IMemberEnrichmentDataClearbit> {
let response: IMemberEnrichmentClearbitAPIResponse

Expand Down
4 changes: 4 additions & 0 deletions services/apps/premium/members_enrichment_worker/src/sources/progai/service.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,10 @@ export default class EnrichmentServiceProgAI extends LoggerBase implements IEnri
return enrichableUsingGithubHandle || enrichableUsingEmail
}

async hasRemainingCredits(): Promise<boolean> {
return true
}
Comment on lines +127 to +129
Copy link

@coderabbitai coderabbitai bot Nov 4, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Align implementation with existing patterns.

To maintain consistency with the rest of the codebase, consider these improvements:

  1. Add error handling similar to getData methods
  2. Use the logger for monitoring credit checks
  3. Consider adding credit threshold configuration via environment variables

Here's a suggested implementation:

  async hasRemainingCredits(): Promise<boolean> {
-   return true
+   try {
+     this.log.debug('Checking ProgAI remaining credits')
+     // Add credit checking logic here
+     const hasCredits = true // Replace with actual check
+     this.log.debug(`ProgAI credits available: ${hasCredits}`)
+     return hasCredits
+   } catch (err) {
+     this.log.error('Error checking ProgAI credits', err)
+     throw new Error(`Failed to check ProgAI credits: ${err.message}`)
+   }
  }

Committable suggestion skipped: line range outside the PR's diff.


async getData(input: IEnrichmentSourceInput): Promise<IMemberEnrichmentDataProgAI> {
let enriched: IMemberEnrichmentDataProgAI = null

Expand Down
27 changes: 25 additions & 2 deletions services/apps/premium/members_enrichment_worker/src/sources/serp/service.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,12 +9,16 @@ import {
IMemberEnrichmentDataNormalized,
} from '../../types'

import { IMemberEnrichmentDataSerp, IMemberEnrichmentSerpApiResponse } from './types'
import {
IMemberEnrichmentDataSerp,
IMemberEnrichmentSerpApiResponse,
ISerpApiAccountUsageData,
} from './types'

export default class EnrichmentServiceSerpApi extends LoggerBase implements IEnrichmentService {
public source: MemberEnrichmentSource = MemberEnrichmentSource.SERP
public platform = `enrichment-${this.source}`
public enrichMembersWithActivityMoreThan = 10
public enrichMembersWithActivityMoreThan = 500

public enrichableBySql = `
("activitySummary".total_count > ${this.enrichMembersWithActivityMoreThan}) AND
Expand Down Expand Up @@ -45,6 +49,25 @@ export default class EnrichmentServiceSerpApi extends LoggerBase implements IEnr
)
}

async hasRemainingCredits(): Promise<boolean> {
try {
const config = {
method: 'get',
url: `https://serpapi.com/account`,
params: {
api_key: process.env['CROWD_ENRICHMENT_SERP_API_KEY'],
},
}

const response: ISerpApiAccountUsageData = (await axios(config)).data

return response.total_searches_left > 0
} catch (error) {
this.log.error('Error while checking serpapi account usage', error)
return false
}
}

async getData(input: IEnrichmentSourceInput): Promise<IMemberEnrichmentDataSerp | null> {
let enriched: IMemberEnrichmentDataSerp = null

Expand Down
18 changes: 18 additions & 0 deletions services/apps/premium/members_enrichment_worker/src/sources/serp/types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,24 @@ export interface IMemberEnrichmentSerpApiResponse {
search_information: IMemberEnrichmentSerpApiResponseSearchInformation
}

export interface ISerpApiAccountUsageData {
account_id: string
api_key: string
account_email: string
account_status: string
plan_id: string
plan_name: string
plan_monthly_price: number
searches_per_month: number
plan_searches_left: number
extra_credits: number
total_searches_left: number
this_month_usage: number
this_hour_searches: number
last_hour_searches: number
account_rate_limit_per_hour: number
}

export interface IMemberEnrichmentSerpApiResponseSearchInformation {
query_displayed: string
total_results: number
Expand Down
6 changes: 5 additions & 1 deletion services/apps/premium/members_enrichment_worker/src/types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,10 @@ export interface IEnrichmentService {
// can the source enrich using this input
isEnrichableBySource(input: IEnrichmentSourceInput): boolean

// does the source have credits to enrich members, if returned false the source will be skipped
// response will be saved to redis for 60 seconds and will be used for subsequent calls
hasRemainingCredits(): Promise<boolean>
Comment on lines +40 to +42
Copy link

@coderabbitai coderabbitai bot Nov 4, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codebase verification

Implementation check reveals potential issues with credit validation

The verification shows that:

  • SERP API service has a proper implementation checking the API account status
  • Both Clearbit and ProgAI services have placeholder implementations that always return true
  • This could lead to unnecessary API calls even when credits are exhausted

• services/apps/premium/members_enrichment_worker/src/sources/clearbit/service.ts: Implement proper credit checking
• services/apps/premium/members_enrichment_worker/src/sources/progai/service.ts: Implement proper credit checking

🔗 Analysis chain

Verify implementation across all enrichment services.

Let's ensure all enrichment services properly implement the new hasRemainingCredits method.

Let me try a different approach to search for the implementations.

Let me check specifically for the hasRemainingCredits implementation in these identified service files.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check implementation of hasRemainingCredits across enrichment services

# Test: Search for class implementations of enrichment services
echo "Searching for enrichment service implementations..."
ast-grep --pattern 'class $_ implements IEnrichmentService {
  $$$
}'

# Test: Verify hasRemainingCredits implementation in each service
echo "Verifying hasRemainingCredits implementation..."
ast-grep --pattern 'async hasRemainingCredits(): Promise<boolean> {
  $$$
}'

Length of output: 368

Script:

#!/bin/bash
# Search for files containing IEnrichmentService
echo "Files referencing IEnrichmentService:"
rg "IEnrichmentService" -l

# Search for class implementations and hasRemainingCredits
echo -e "\nImplementations and method details:"
rg "class.*implements.*IEnrichmentService" -A 20

Length of output: 8082

Script:

#!/bin/bash
# Check for hasRemainingCredits implementation in each service file
echo "Checking SERP implementation:"
rg "hasRemainingCredits" services/apps/premium/members_enrichment_worker/src/sources/serp/service.ts -A 5

echo -e "\nChecking Clearbit implementation:"
rg "hasRemainingCredits" services/apps/premium/members_enrichment_worker/src/sources/clearbit/service.ts -A 5

echo -e "\nChecking ProgAI implementation:"
rg "hasRemainingCredits" services/apps/premium/members_enrichment_worker/src/sources/progai/service.ts -A 5

Length of output: 1195


// SQL filter to get enrichable members for a source
// members table is available as "members" alias
// memberIdentities table is available as "mi" alias
Expand Down Expand Up @@ -66,7 +70,7 @@ export interface IMemberEnrichmentDataNormalizedOrganization {
}

export interface IGetMembersForEnrichmentArgs {
afterId?: string
afterCursor: { activityCount: number; memberId: string } | null
}

export interface IMemberEnrichmentSocialData {
Expand Down
31 changes: 25 additions & 6 deletions services/apps/premium/members_enrichment_worker/src/workflows/enrichMember.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ const {
updateMemberEnrichmentCache,
isCacheObsolete,
normalizeEnrichmentData,
findMemberIdentityWithTheMostActivityInPlatform,
} = proxyActivities<typeof activities>({
startToCloseTimeout: '20 seconds',
retry: {
Expand All @@ -42,12 +43,6 @@ export async function enrichMember(
// cache is obsolete when it's not found or cache.updatedAt is older than cacheObsoleteAfterSeconds
if (await isCacheObsolete(source, cache)) {
const enrichmentInput: IEnrichmentSourceInput = {
github: input.identities.find(
(i) =>
i.verified &&
i.platform === PlatformType.GITHUB &&
i.type === MemberIdentityType.USERNAME,
),
email: input.identities.find((i) => i.verified && i.type === MemberIdentityType.EMAIL),
linkedin: input.identities.find(
(i) =>
Expand All @@ -61,6 +56,30 @@ export async function enrichMember(
activityCount: input.activityCount || 0,
}

// there can be multiple verified identities in github, we select the one with the most activities
const verifiedGithubIdentities = input.identities.filter(
(i) =>
i.verified &&
i.platform === PlatformType.GITHUB &&
i.type === MemberIdentityType.USERNAME,
)

if (verifiedGithubIdentities.length > 1) {
const ghIdentityWithTheMostActivities =
await findMemberIdentityWithTheMostActivityInPlatform(input.id, PlatformType.GITHUB)
if (ghIdentityWithTheMostActivities) {
enrichmentInput.github = input.identities.find(
(i) =>
i.verified &&
i.platform === PlatformType.GITHUB &&
i.type === MemberIdentityType.USERNAME &&
i.value === ghIdentityWithTheMostActivities.username,
)
}
} else {
enrichmentInput.github = verifiedGithubIdentities?.[0] || undefined
}

const data = await getEnrichmentData(source, enrichmentInput)

if (!cache) {
Expand Down
9 changes: 6 additions & 3 deletions services/apps/premium/members_enrichment_worker/src/workflows/getMembersToEnrich.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,14 +19,14 @@ const { getEnrichableMembers } = proxyActivities<typeof activities>({

export async function getMembersToEnrich(args: IGetMembersForEnrichmentArgs): Promise<void> {
const MEMBER_ENRICHMENT_PER_RUN = 100
const afterId = args?.afterId || null
const afterCursor = args?.afterCursor || null
const sources = [
MemberEnrichmentSource.PROGAI,
MemberEnrichmentSource.CLEARBIT,
MemberEnrichmentSource.SERP,
]

const members = await getEnrichableMembers(MEMBER_ENRICHMENT_PER_RUN, sources, afterId)
const members = await getEnrichableMembers(MEMBER_ENRICHMENT_PER_RUN, sources, afterCursor)

if (members.length === 0) {
return
Expand Down Expand Up @@ -54,6 +54,9 @@ export async function getMembersToEnrich(args: IGetMembersForEnrichmentArgs): Pr
)

await continueAsNew<typeof getMembersToEnrich>({
afterId: members[members.length - 1].id,
afterCursor: {
memberId: members[members.length - 1].id,
activityCount: members[members.length - 1].activityCount,
},
})
}
22 changes: 22 additions & 0 deletions services/libs/data-access-layer/src/activities/sql.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import {
ActivityDisplayVariant,
IActivityBySentimentMoodResult,
IActivityByTypeAndPlatformResult,
IEnrichableMemberIdentityActivityAggregate,
IMemberIdentity,
ITimeseriesDatapoint,
MemberIdentityType,
Expand Down Expand Up @@ -1391,3 +1392,24 @@ export async function getLastActivitiesForMembers(
segmentIds,
)
}

export async function findMemberIdentityWithTheMostActivityInPlatform(
qdbConn: DbConnOrTx,
platform: string,
memberId: string,
): Promise<IEnrichableMemberIdentityActivityAggregate> {
const query = `
SELECT count(a.id) AS "activityCount", a.platform, a.username
FROM activities a
WHERE a."memberId" = $(memberId)
AND a.platform = $(platform)
GROUP BY a.platform, a.username
ORDER BY activity_count DESC
LIMIT 1;
`

return qdbConn.oneOrNone(query, {
memberId,
platform,
})
}
Comment on lines +1396 to +1415
Copy link

@coderabbitai coderabbitai bot Nov 5, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Fix SQL query issues and add proper type mapping

There are several issues in the implementation:

  1. The ORDER BY clause uses incorrect column name activity_count instead of "activityCount"
  2. Missing soft delete filter that's used in all other queries
  3. Inconsistent column name quoting
  4. Raw database result is returned without mapping to the interface type

Apply this diff to fix the issues:

 export async function findMemberIdentityWithTheMostActivityInPlatform(
   qdbConn: DbConnOrTx,
   platform: string,
   memberId: string,
 ): Promise<IEnrichableMemberIdentityActivityAggregate> {
   const query = `
-  SELECT count(a.id) AS "activityCount", a.platform, a.username
+  SELECT COUNT(a.id) AS "activityCount", a."platform", a."username"
       FROM activities a
       WHERE a."memberId" = $(memberId)
         AND a.platform = $(platform)
+        AND a."deletedAt" IS NULL
-      GROUP BY a.platform, a.username
-      ORDER BY activity_count DESC
+      GROUP BY a."platform", a."username"
+      ORDER BY "activityCount" DESC
     LIMIT 1;
   `

-  return qdbConn.oneOrNone(query, {
+  const result = await qdbConn.oneOrNone(query, {
     memberId,
     platform,
   })
+  
+  return result ? {
+    activityCount: parseInt(result.activityCount),
+    platform: result.platform,
+    username: result.username
+  } : null
 }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
export async function findMemberIdentityWithTheMostActivityInPlatform(
qdbConn: DbConnOrTx,
platform: string,
memberId: string,
): Promise<IEnrichableMemberIdentityActivityAggregate> {
const query = `
SELECT count(a.id) AS "activityCount", a.platform, a.username
FROM activities a
WHERE a."memberId" = $(memberId)
AND a.platform = $(platform)
GROUP BY a.platform, a.username
ORDER BY activity_count DESC
LIMIT 1;
`
return qdbConn.oneOrNone(query, {
memberId,
platform,
})
}
export async function findMemberIdentityWithTheMostActivityInPlatform(
qdbConn: DbConnOrTx,
platform: string,
memberId: string,
): Promise<IEnrichableMemberIdentityActivityAggregate> {
const query = `
SELECT COUNT(a.id) AS "activityCount", a."platform", a."username"
FROM activities a
WHERE a."memberId" = $(memberId)
AND a.platform = $(platform)
AND a."deletedAt" IS NULL
GROUP BY a."platform", a."username"
ORDER BY "activityCount" DESC
LIMIT 1;
`
const result = await qdbConn.oneOrNone(query, {
memberId,
platform,
})
return result ? {
activityCount: parseInt(result.activityCount),
platform: result.platform,
username: result.username
} : null
}

Loading