Common API - create common find one query

[etiennejouan]

closes twentyhq/core-team-issues#1418

[github-actions]

TODOs/FIXMEs:

• //TODO : Refacto-common - Should be Common: packages/twenty-server/src/engine/api/graphql/workspace-query-runner/workspace-query-hook/workspace-query-hook.service.ts
• //TODO : Refacto-common - Remove this method and use RestApiRequestContextService instead: packages/twenty-server/src/engine/api/rest/core/interfaces/rest-api-base.handler.ts

Generated by 🚫 dangerJS against 093aa42

[greptile-apps]

Greptile Overview

Summary

This PR introduces a common find one query API that abstracts the find one functionality to be shared between GraphQL and REST endpoints. The implementation successfully consolidates the find one logic into a common service (CommonFindOneQueryRunnerService) that extends a base query runner service.

Key Changes:

• Created CommonFindOneQueryRunnerService that handles find one operations for both REST and GraphQL APIs
• Refactored FindOneResolverFactory to use the common service instead of GraphQL-specific implementation
• Added supporting types and interfaces for the common API pattern
• Updated module dependencies to wire the new common services

Issues Found:

• Multiple TODO comments indicate incomplete refactoring - the service still uses GraphQL-specific components like GraphqlQueryParser and GraphqlQueryRunnerException instead of truly common abstractions
• REST API handler incorrectly throws 400 BadRequest for record not found scenarios instead of the proper 404 NotFound status
• Type casting and string manipulation suggest the abstraction layer is not fully mature

Recommendation:
The PR successfully demonstrates the common API pattern but needs completion of the refactoring to remove GraphQL-specific dependencies and fix the HTTP status code logic in the REST handler.

Confidence Score: 3/5

• This PR is moderately safe to merge but contains incomplete refactoring and incorrect error handling
• Score reflects multiple TODO comments indicating incomplete refactoring, incorrect HTTP status codes in REST handler, and mixed abstraction levels that could cause confusion
• Pay close attention to rest-api-find-one.handler.ts for incorrect HTTP status codes and common-find-one-query-runner.service.ts for incomplete refactoring

Important Files Changed

File Analysis

Filename	Score	Overview
packages/twenty-server/src/engine/api/common/common-query-runners/common-find-one-query-runner.service.ts	3/5	New find one query runner with multiple TODO comments indicating incomplete refactoring and mixed GraphQL/Common abstractions
packages/twenty-server/src/engine/api/common/common-query-runners/common-base-query-runner.service.ts	4/5	Base service with comprehensive execution flow but contains TODO comments about incomplete refactoring
packages/twenty-server/src/engine/api/rest/core/handlers/rest-api-find-one.handler.ts	2/5	REST handler with incorrect HTTP status code logic - throws 400 BadRequest for record not found instead of 404
packages/twenty-server/src/engine/api/graphql/workspace-resolver-builder/factories/find-one-resolver.factory.ts	4/5	Successfully refactored to use common find one query runner instead of GraphQL-specific service

Sequence Diagram

sequenceDiagram
    participant Client as Client (REST/GraphQL)
    participant Handler as REST/GraphQL Handler
    participant CommonService as CommonFindOneQueryRunnerService
    participant BaseService as CommonBaseQueryRunnerService
    participant Parser as GraphqlQueryParser
    participant Repository as WorkspaceRepository
    participant Relations as ProcessNestedRelationsHelper
    participant Parser2 as ObjectRecordsToGraphqlConnectionHelper

    Client->>Handler: FindOne Request
    Handler->>Handler: Parse request & build args/options
    Handler->>CommonService: execute(args, options, 'findOne')
    CommonService->>BaseService: execute(args, options, operationName)
    
    BaseService->>BaseService: validate(args, options)
    BaseService->>BaseService: executePreQueryHooks()
    BaseService->>BaseService: getDataSourceForWorkspace()
    BaseService->>BaseService: validateSettingsPermissions()
    BaseService->>BaseService: getRoleId()
    BaseService->>Repository: getRepository(nameSingular, roleId)
    BaseService->>CommonService: run(executionContext)
    
    CommonService->>Parser: new GraphqlQueryParser()
    CommonService->>Parser: applyFilterToBuilder()
    CommonService->>Parser: applyDeletedAtToBuilder()
    CommonService->>Repository: queryBuilder.getOne()
    Repository-->>CommonService: objectRecord or null
    
    alt Record not found
        CommonService->>CommonService: throw GraphqlQueryRunnerException
    end
    
    alt Has relations
        CommonService->>Relations: processNestedRelations()
        Relations-->>CommonService: enriched records
    end
    
    CommonService->>Parser2: new ObjectRecordsToGraphqlConnectionHelper()
    CommonService->>Parser2: processRecord()
    Parser2-->>CommonService: formatted record
    CommonService-->>BaseService: ObjectRecord
    
    BaseService->>BaseService: executePostQueryHooks()
    BaseService-->>Handler: ObjectRecord
    Handler-->>Client: Formatted Response

Loading

_{22 files reviewed, 5 comments}

_{Edit Code Review Agent Settings | Greptile}

[greptile-apps]

style: TODO comment indicates incomplete refactoring - Exception handler should be Common but still uses GraphqlQueryRunnerException

[greptile-apps]

style: TODO comments indicate incomplete refactoring - unsafe type casting and string manipulation for ResolverArgsType suggest the common API abstraction is incomplete

[github-actions]

🚀 Preview Environment Ready!

Your preview environment is available at: http://bore.pub:15236

This environment will automatically shut down when the PR is closed or after 5 hours.

[Weiko]

Should be a NotFoundException or am I missing something?

[etiennejouan]

weird I agree but BadRequestException initially cf https://github.com/twentyhq/twenty/pull/14720/files#diff-78763e5a11777b61d179d4224564fa26edccee897d702a2991ca1810e081f925L40

[Weiko]

Note: This will do a few things that we are now doing in the common part already (fetching permission/roles for example)