diff --git a/AGENTS.md b/AGENTS.md index ada10cfa52..dfd293b4a4 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -105,6 +105,14 @@ This file provides guidance to coding agents when working with code in this repo - `.infra/common.ts` - Worker subscription definitions - `.infra/index.ts` - Main Pulumi deployment configuration +## Best Practices & Lessons Learned + +**Avoiding Code Duplication:** +- **Always check for existing implementations** before creating new helper functions. Use Grep or Glob tools to search for similar function names or logic patterns across the codebase. +- **Prefer extracting to common utilities** when logic needs to be shared. Place shared helpers in appropriate `src/common/` subdirectories (e.g., `src/common/opportunity/` for opportunity-related helpers). +- **Export and import, don't duplicate**: When you need the same logic in multiple places, export the function from its original location and import it where needed. This ensures a single source of truth and prevents maintenance issues. +- **Example lesson**: When implementing `handleOpportunityKeywordsUpdate`, the function was duplicated in both `src/common/opportunity/parse.ts` and `src/schema/opportunity.ts`. This caused lint failures and maintenance burden. The correct approach was to export it from `parse.ts` and import it in `opportunity.ts`. + ## Pull Requests Keep PR descriptions concise and to the point. Reviewers should not be exhausted by lengthy explanations. diff --git a/__tests__/schema/opportunity.ts b/__tests__/schema/opportunity.ts index f4fdfcb8ce..86f11d2a3c 100644 --- a/__tests__/schema/opportunity.ts +++ b/__tests__/schema/opportunity.ts @@ -6399,3 +6399,164 @@ describe('query opportunityStats', () => { expect(res.errors[0].extensions.code).toBe('FORBIDDEN'); }); }); + +describe('mutation reimportOpportunity', () => { + const MUTATION = /* GraphQL */ ` + mutation ReimportOpportunity($payload: ReimportOpportunityInput!) { + reimportOpportunity(payload: $payload) { + id + title + tldr + content { + overview { + content + } + requirements { + content + } + responsibilities { + content + } + } + keywords { + keyword + } + } + } + `; + + beforeEach(async () => { + jest.resetAllMocks(); + + const transport = createMockBrokkrTransport(); + const serviceClient = { + instance: createClient(BrokkrService, transport), + garmr: createGarmrMock(), + }; + + jest + .spyOn(brokkrCommon, 'getBrokkrClient') + .mockImplementation((): ServiceClient => { + return serviceClient; + }); + }); + + it('should require authentication', async () => { + loggedUser = null; + + await testMutationErrorCode( + client, + { + mutation: MUTATION, + variables: { + payload: { + opportunityId: opportunitiesFixture[0].id, + url: 'https://example.com/job', + }, + }, + }, + 'UNAUTHENTICATED', + ); + }); + + it('should require recruiter permission', async () => { + loggedUser = '3'; // User 3 is not a recruiter for opportunity 3 + + await testMutationErrorCode( + client, + { + mutation: MUTATION, + variables: { + payload: { + opportunityId: opportunitiesFixture[3].id, + url: 'https://example.com/job', + }, + }, + }, + 'FORBIDDEN', + ); + }); + + it('should fail when neither file nor URL is provided', async () => { + loggedUser = '2'; // User 2 is a recruiter for opportunity 3 + + const res = await client.mutate(MUTATION, { + variables: { + payload: { + opportunityId: opportunitiesFixture[3].id, + }, + }, + }); + + expect(res.errors).toBeTruthy(); + }); + + it('should reimport opportunity from URL and update all fields', async () => { + loggedUser = '2'; // User 2 is a recruiter for opportunity 3 (which is in DRAFT state) + + const fetchSpy = jest.spyOn(globalThis, 'fetch'); + const pdfResponse = new Response('Mocked PDF content', { + status: 200, + headers: { 'Content-Type': 'application/pdf' }, + }); + jest + .spyOn(pdfResponse, 'arrayBuffer') + .mockResolvedValue(new ArrayBuffer(0)); + fetchSpy.mockResolvedValueOnce(pdfResponse); + + fileTypeFromBuffer.mockResolvedValue({ + ext: 'pdf', + mime: 'application/pdf', + }); + + const uploadResumeFromBufferSpy = jest.spyOn( + googleCloud, + 'uploadResumeFromBuffer', + ); + uploadResumeFromBufferSpy.mockResolvedValue( + `https://storage.cloud.google.com/${RESUME_BUCKET_NAME}/file`, + ); + + const deleteFileFromBucketSpy = jest.spyOn( + googleCloud, + 'deleteFileFromBucket', + ); + deleteFileFromBucketSpy.mockResolvedValue(true); + + // Get original opportunity state + const originalOpportunity = await con + .getRepository(OpportunityJob) + .findOneByOrFail({ id: opportunitiesFixture[3].id }); + + const res = await client.mutate(MUTATION, { + variables: { + payload: { + opportunityId: opportunitiesFixture[3].id, + url: 'https://example.com/updated-job', + }, + }, + }); + + expect(res.errors).toBeFalsy(); + expect(res.data.reimportOpportunity.id).toBe(opportunitiesFixture[3].id); + + // Verify fields were updated with mocked Brokkr response + expect(res.data.reimportOpportunity.title).toBe('Mocked Opportunity Title'); + expect(res.data.reimportOpportunity.tldr).toBe( + 'This is a mocked TL;DR of the opportunity.', + ); + expect(res.data.reimportOpportunity.keywords).toEqual([ + { keyword: 'mock' }, + { keyword: 'opportunity' }, + { keyword: 'test' }, + ]); + + // Verify opportunity still exists and was updated + const updatedOpportunity = await con + .getRepository(OpportunityJob) + .findOneByOrFail({ id: opportunitiesFixture[3].id }); + + expect(updatedOpportunity.title).toBe('Mocked Opportunity Title'); + expect(updatedOpportunity.state).toBe(originalOpportunity.state); // State should be preserved + }); +}); diff --git a/src/common/opportunity/parse.ts b/src/common/opportunity/parse.ts index 0f73ce4918..159cd8baf6 100644 --- a/src/common/opportunity/parse.ts +++ b/src/common/opportunity/parse.ts @@ -30,6 +30,7 @@ import { OpportunityUserRecruiter } from '../../entity/opportunities/user/Opport import { findDatasetLocation } from '../../entity/dataset/utils'; import { addOpportunityDefaultQuestionFeedback } from './question'; import type { Opportunity } from '../../entity/opportunities/Opportunity'; +import { EntityManager } from 'typeorm'; interface FileUpload { filename: string; @@ -357,3 +358,112 @@ export async function createOpportunityFromParsedData( return opportunity; }); } + +export interface UpdateOpportunityContext { + con: DataSource; + log: FastifyBaseLogger; +} + +/** + * Handles opportunity keywords updates + * Replaces all existing keywords with the new set + */ +export async function handleOpportunityKeywordsUpdate( + entityManager: EntityManager, + opportunityId: string, + keywords: Array<{ keyword: string }> | undefined, +): Promise { + if (!Array.isArray(keywords)) { + return; + } + + await entityManager.getRepository(OpportunityKeyword).delete({ + opportunityId, + }); + + await entityManager.getRepository(OpportunityKeyword).insert( + keywords.map((keyword) => ({ + opportunityId, + keyword: keyword.keyword, + })), + ); +} + +/** + * Updates an existing opportunity with all parsed data. + * + * @param ctx - Context with database connection and logger + * @param opportunityId - ID of the opportunity to update + * @param parsedData - The parsed opportunity data from Brokkr + * @returns The opportunity ID + */ +export async function updateOpportunityFromParsedData( + ctx: UpdateOpportunityContext, + opportunityId: string, + parsedData: ParsedOpportunityResult, +): Promise { + const { opportunity: parsedOpportunity, content } = parsedData; + + return ctx.con.transaction(async (entityManager) => { + // Fetch the existing opportunity + const existingOpportunity = await entityManager + .getRepository(OpportunityJob) + .findOne({ + where: { id: opportunityId }, + }); + + if (!existingOpportunity) { + throw new ValidationError('Opportunity not found'); + } + + // Build update object with all parsed data + const updateData: Partial = {}; + + if (parsedOpportunity.title) { + updateData.title = parsedOpportunity.title; + } + + if (parsedOpportunity.tldr) { + updateData.tldr = parsedOpportunity.tldr; + } + + // Update content - merge with existing to preserve any sections not in parsed data + // Explicitly list content block keys to avoid iterating over protobuf methods + const contentBlockKeys = [ + 'overview', + 'responsibilities', + 'requirements', + 'whatYoullDo', + 'interviewProcess', + ] as const; + const mergedContent: Partial = {}; + for (const key of contentBlockKeys) { + if (content[key]) { + mergedContent[key] = content[key]; + } + } + updateData.content = { + ...existingOpportunity.content, + ...mergedContent, + } as OpportunityContent; + + // Update the opportunity + if (Object.keys(updateData).length > 0) { + await entityManager + .getRepository(OpportunityJob) + .update({ id: opportunityId }, updateData); + } + + // Update keywords if present in parsed data + if (parsedOpportunity.keywords?.length) { + await handleOpportunityKeywordsUpdate( + entityManager, + opportunityId, + parsedOpportunity.keywords, + ); + } + + // Return the opportunity ID + return opportunityId; + }); +} diff --git a/src/common/schema/opportunities.ts b/src/common/schema/opportunities.ts index aa8303f92d..3e2b7c32a9 100644 --- a/src/common/schema/opportunities.ts +++ b/src/common/schema/opportunities.ts @@ -242,6 +242,37 @@ export const parseOpportunitySchema = z }, ); +export const reimportOpportunitySchema = z + .object({ + opportunityId: z.uuid(), + url: urlParseSchema.optional(), + file: fileUploadSchema.optional(), + }) + .refine( + (data) => { + if (!data.url && !data.file) { + return false; + } + + return true; + }, + { + error: 'Either url or file must be provided.', + }, + ) + .refine( + (data) => { + if (data.url && data.file) { + return false; + } + + return true; + }, + { + error: 'Only one of url or file can be provided.', + }, + ); + export const createSharedSlackChannelSchema = z.object({ organizationId: z.string().uuid('Organization ID must be a valid UUID'), email: z.string().email('Email must be a valid email address'), diff --git a/src/schema/opportunity.ts b/src/schema/opportunity.ts index 592a119504..46f33e1964 100644 --- a/src/schema/opportunity.ts +++ b/src/schema/opportunity.ts @@ -37,7 +37,11 @@ import { } from '../common/schema/opportunityMatch'; import { OpportunityJob } from '../entity/opportunities/OpportunityJob'; import { OpportunityUserRecruiter } from '../entity/opportunities/user/OpportunityUserRecruiter'; -import { ForbiddenError, ValidationError } from 'apollo-server-errors'; +import { + AuthenticationError, + ForbiddenError, + ValidationError, +} from 'apollo-server-errors'; import { ConflictError, NotFoundError, PaymentRequiredError } from '../errors'; import { UserCandidateKeyword } from '../entity/user/UserCandidateKeyword'; import { User } from '../entity/user/User'; @@ -53,10 +57,10 @@ import { opportunityUpdateStateSchema, createSharedSlackChannelSchema, parseOpportunitySchema, + reimportOpportunitySchema, opportunityMatchesQuerySchema, addOpportunitySeatsSchema, } from '../common/schema/opportunities'; -import { OpportunityKeyword } from '../entity/OpportunityKeyword'; import { ensureOpportunityPermissions, OpportunityPermissions, @@ -97,6 +101,8 @@ import { validateOpportunityFileType, parseOpportunityWithBrokkr, createOpportunityFromParsedData, + updateOpportunityFromParsedData, + handleOpportunityKeywordsUpdate, } from '../common/opportunity/parse'; import { isMockEnabled, @@ -740,6 +746,23 @@ export const typeDefs = /* GraphQL */ ` url: String } + input ReimportOpportunityInput { + """ + ID of the opportunity to update + """ + opportunityId: ID! + + """ + PDF, Word file to parse + """ + file: Upload + + """ + URL to scrape and parse + """ + url: String + } + input OpportunitySeatInput { priceId: String! quantity: Int! @@ -878,6 +901,13 @@ export const typeDefs = /* GraphQL */ ` parseOpportunity(payload: ParseOpportunityInput!): Opportunity! @rateLimit(limit: 10, duration: 3600) + """ + Re-import and update an existing opportunity from a URL or file upload + """ + reimportOpportunity(payload: ReimportOpportunityInput!): Opportunity! + @auth + @rateLimit(limit: 10, duration: 3600) + """ Create a shared Slack channel and invite a user by email """ @@ -1085,31 +1115,6 @@ async function handleOpportunityLocationUpdate( } } -/** - * Handles opportunity keywords updates - * Replaces all existing keywords with the new set - */ -async function handleOpportunityKeywordsUpdate( - entityManager: EntityManager, - opportunityId: string, - keywords: Array<{ keyword: string }> | undefined, -): Promise { - if (!Array.isArray(keywords)) { - return; - } - - await entityManager.getRepository(OpportunityKeyword).delete({ - opportunityId, - }); - - await entityManager.getRepository(OpportunityKeyword).insert( - keywords.map((keyword) => ({ - opportunityId, - keyword: keyword.keyword, - })), - ); -} - /** * Handles opportunity screening questions updates * Validates questions ownership and upserts them with proper ordering @@ -2647,6 +2652,109 @@ export const resolvers: IResolvers = traceResolvers< throw error; } }, + reimportOpportunity: async ( + _, + { + payload, + }: { + payload: unknown; + }, + ctx: Context, + info, + ): Promise => { + if (!ctx.userId) { + throw new AuthenticationError('User must be authenticated'); + } + + try { + const startTime = Date.now(); + let stepStart = startTime; + + const parsedPayload = + await reimportOpportunitySchema.parseAsync(payload); + ctx.log.info( + { + durationMs: Date.now() - stepStart, + opportunityId: parsedPayload.opportunityId, + }, + 'reimportOpportunity: payload schema validated', + ); + + // Check user has permission to edit this opportunity + stepStart = Date.now(); + await ensureOpportunityPermissions({ + con: ctx.con.manager, + userId: ctx.userId, + opportunityId: parsedPayload.opportunityId, + permission: OpportunityPermissions.Edit, + isTeamMember: ctx.isTeamMember, + }); + ctx.log.info( + { durationMs: Date.now() - stepStart }, + 'reimportOpportunity: permissions verified', + ); + + stepStart = Date.now(); + const { buffer, extension } = + await getOpportunityFileBuffer(parsedPayload); + ctx.log.info( + { durationMs: Date.now() - stepStart, bufferSize: buffer.length }, + 'reimportOpportunity: file buffer acquired', + ); + + stepStart = Date.now(); + const { mime } = await validateOpportunityFileType(buffer, extension); + ctx.log.info( + { durationMs: Date.now() - stepStart, mime }, + 'reimportOpportunity: file type validated', + ); + + stepStart = Date.now(); + const parsedData = await parseOpportunityWithBrokkr( + buffer, + mime, + ctx.log, + ); + ctx.log.info( + { + durationMs: Date.now() - stepStart, + title: parsedData.opportunity.title, + }, + 'reimportOpportunity: Brokkr parsing completed', + ); + + stepStart = Date.now(); + const opportunityId = await updateOpportunityFromParsedData( + { + con: ctx.con, + log: ctx.log, + }, + parsedPayload.opportunityId, + parsedData, + ); + ctx.log.info( + { durationMs: Date.now() - stepStart, opportunityId }, + 'reimportOpportunity: database records updated', + ); + + const totalDurationMs = Date.now() - startTime; + ctx.log.info( + { totalDurationMs, opportunityId }, + 'reimportOpportunity: completed successfully', + ); + + return graphorm.queryOneOrFail(ctx, info, (builder) => { + builder.queryBuilder.where({ id: opportunityId }); + return builder; + }); + } catch (error) { + ctx.log.error( + { error }, + 'reimportOpportunity: failed to reimport opportunity', + ); + throw error; + } + }, }, OpportunityMatch: { engagementProfile: async ( diff --git a/src/workers/cdc/common.ts b/src/workers/cdc/common.ts index c9f22354aa..7b9d66f08a 100644 --- a/src/workers/cdc/common.ts +++ b/src/workers/cdc/common.ts @@ -2,7 +2,6 @@ import { ContentMeta, ContentQuality, ContentUpdatedMessage, - Translation, } from '@dailydotdev/schema'; import { DataSource, ObjectLiteral } from 'typeorm'; import { EntityTarget } from 'typeorm/common/EntityTarget'; @@ -139,23 +138,6 @@ export const notifyPostContentUpdated = async ({ }), deleted: articlePost.deleted, sharedPostId: sharePost.sharedPostId || undefined, - translation: post.translation - ? Object.entries( - typeof post.translation === 'string' - ? JSON.parse(post.translation) - : post.translation, - ).reduce( - (acc, [key, value]) => { - acc[key] = decodeJsonField({ - value: value as JsonValue, - decoder: new Translation(), - }); - - return acc; - }, - {} as { [key: string]: Translation }, - ) - : undefined, }); await triggerTypedEvent(