diff --git a/AGENTS.md b/AGENTS.md index d5ae3684ed..de89e622c6 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -86,6 +86,7 @@ The migration generator compares entities against the local database schema. Ens - **Docs**: See `src/graphorm/AGENTS.md` for comprehensive guide on using GraphORM to solve N+1 queries. GraphORM is the default and preferred method for all GraphQL query responses. Use GraphORM instead of TypeORM repositories for GraphQL resolvers to prevent N+1 queries and enforce best practices. - **GraphORM mappings**: Only add entries in `src/graphorm/index.ts` when you need custom mapping/fields/transforms or GraphQL type names differ from TypeORM entity names. For straightforward reads, keep GraphQL type names aligned with entities and use GraphORM without extra config. - For GraphQL query resolvers, prefer `graphorm.query`, `graphorm.queryOne`, or `graphorm.queryPaginated` over custom TypeORM fetch/pagination code whenever GraphORM can express the query. Reach for manual TypeORM reads only when GraphORM genuinely cannot support the access pattern. +- When a GraphQL field must be nulled based on viewer permissions, define the rule in `src/graphorm/index.ts` as a shared transform/helper (for example `nullIfNotLoggedIn`). If a resolver cannot use GraphORM for the full query, reuse that GraphORM field mapping from the manual path instead of re-implementing the permission rule in a schema field resolver. - For offset-paginated GraphQL reads that only need `pageInfo.hasNextPage`, prefer overfetching one extra row and slicing it in the page generator. Avoid separate `COUNT(*)`/`COUNT(DISTINCT ...)` queries unless the client explicitly needs a total. **Data Layer:** diff --git a/__tests__/schema/feedback.ts b/__tests__/schema/feedback.ts index 2995aee870..cd1c66c586 100644 --- a/__tests__/schema/feedback.ts +++ b/__tests__/schema/feedback.ts @@ -61,6 +61,7 @@ const USER_FEEDBACK_QUERY = ` id category description + linearIssueUrl status createdAt replies { @@ -90,6 +91,7 @@ const FEEDBACK_LIST_QUERY = ` edges { node { id + linearIssueUrl status category user { @@ -118,6 +120,7 @@ const USER_FEEDBACK_BY_USER_ID_QUERY = ` id category description + linearIssueUrl status user { id @@ -148,6 +151,7 @@ describe('feedback schema', () => { userId: '1', category: UserFeedbackCategory.BUG, description: 'Own feedback', + linearIssueUrl: 'https://linear.app/dailydev/issue/ENG-1159', status: FeedbackStatus.Pending, flags: {}, }); @@ -178,6 +182,7 @@ describe('feedback schema', () => { expect(res.data.userFeedback.edges[0].node).toMatchObject({ id: ownFeedback.id, description: 'Own feedback', + linearIssueUrl: null, replies: [{ body: 'Thanks for sharing', authorName: 'Chris' }], }); }); @@ -197,6 +202,7 @@ describe('feedback schema', () => { userId: '1', category: UserFeedbackCategory.BUG, description: 'Pending feedback', + linearIssueUrl: 'https://linear.app/dailydev/issue/ENG-1159', status: FeedbackStatus.Pending, flags: {}, }); @@ -227,6 +233,7 @@ describe('feedback schema', () => { expect(res.data.feedbackList.edges).toHaveLength(1); expect(res.data.feedbackList.edges[0].node).toMatchObject({ id: pendingFeedback.id, + linearIssueUrl: 'https://linear.app/dailydev/issue/ENG-1159', user: { id: '1', }, @@ -238,6 +245,7 @@ describe('feedback schema', () => { userId: '1', category: UserFeedbackCategory.CONTENT_QUALITY, description: 'Matched feedback', + linearIssueUrl: 'https://linear.app/dailydev/issue/ENG-1159', status: FeedbackStatus.Completed, flags: {}, }); @@ -271,6 +279,7 @@ describe('feedback schema', () => { expect(res.data.feedbackList.edges).toHaveLength(1); expect(res.data.feedbackList.edges[0].node).toMatchObject({ id: matched.id, + linearIssueUrl: 'https://linear.app/dailydev/issue/ENG-1159', status: FeedbackStatus.Completed, category: UserFeedbackCategory.CONTENT_QUALITY, user: { @@ -309,6 +318,7 @@ describe('feedback schema', () => { userId: '2', category: UserFeedbackCategory.BUG, description: 'User 2 feedback 1', + linearIssueUrl: 'https://linear.app/dailydev/issue/ENG-1158', status: FeedbackStatus.Pending, flags: {}, }); @@ -317,6 +327,7 @@ describe('feedback schema', () => { userId: '2', category: UserFeedbackCategory.FEATURE_REQUEST, description: 'User 2 feedback 2', + linearIssueUrl: 'https://linear.app/dailydev/issue/ENG-1159', status: FeedbackStatus.Processing, flags: {}, }); @@ -348,6 +359,7 @@ describe('feedback schema', () => { res.data.userFeedbackByUserId.edges.map((edge) => edge.node.id), ).toEqual([secondFeedback.id, firstFeedback.id]); expect(res.data.userFeedbackByUserId.edges[0].node).toMatchObject({ + linearIssueUrl: 'https://linear.app/dailydev/issue/ENG-1159', user: { id: '2', }, @@ -355,6 +367,9 @@ describe('feedback schema', () => { { body: 'Thanks, we are investigating', authorName: 'Support' }, ], }); + expect(res.data.userFeedbackByUserId.edges[1].node).toMatchObject({ + linearIssueUrl: 'https://linear.app/dailydev/issue/ENG-1158', + }); }); it('should paginate userFeedbackByUserId results for team members', async () => { diff --git a/src/graphorm/index.ts b/src/graphorm/index.ts index 00e64f9322..c2301cb26b 100644 --- a/src/graphorm/index.ts +++ b/src/graphorm/index.ts @@ -26,6 +26,7 @@ import { import { type Organization } from '../entity/Organization'; import { OrganizationMemberRole, + Roles, SourceMemberRoles, rankToSourceRole, sourceRoleRank, @@ -136,6 +137,15 @@ const existsByUserAndHotTake = const nullIfNotLoggedIn = (value: T, ctx: Context): T | null => ctx.userId ? value : null; +export const nullIfNotTeamMember = < + T, + Ctx extends Pick, +>( + value: T, + ctx: Ctx, +): T | null => + ctx.isTeamMember || ctx.roles.includes(Roles.Moderator) ? value : null; + const nullIfNotSameUser = ( value: T, ctx: Context, @@ -2454,6 +2464,13 @@ const obj = new GraphORM({ }, }, }, + FeedbackItem: { + fields: { + linearIssueUrl: { + transform: nullIfNotTeamMember, + }, + }, + }, Achievement: { fields: { criteria: { jsonType: true }, diff --git a/src/schema/feedback.ts b/src/schema/feedback.ts index a370475759..adc13c71d5 100644 --- a/src/schema/feedback.ts +++ b/src/schema/feedback.ts @@ -23,6 +23,8 @@ import { import { Roles } from '../roles'; import { queryReadReplica } from '../common/queryReadReplica'; import { QuestEventType } from '../entity/Quest'; +import graphorm from '../graphorm'; +import { Context } from '../Context'; type GQLFeedbackInput = { category: number; @@ -45,6 +47,7 @@ type GQLFeedbackItem = Pick< | 'id' | 'category' | 'description' + | 'linearIssueUrl' | 'status' | 'screenshotUrl' | 'createdAt' @@ -137,6 +140,7 @@ export const typeDefs = /* GraphQL */ ` id: ID! category: ProtoEnumValue! description: String! + linearIssueUrl: String status: Int! screenshotUrl: String createdAt: DateTime! @@ -214,11 +218,13 @@ const mapRepliesByFeedbackId = ({ }; const fetchFeedbackConnectionNodes = async ({ + ctx, manager, where, page, includeUsers, }: { + ctx: BaseContext; manager: EntityManager; where: FindOptionsWhere; page: ReturnType; @@ -255,11 +261,21 @@ const fetchFeedbackConnectionNodes = async ({ usersById = new Map(users.map((user) => [user.id, user])); } + const linearIssueUrlTransform = + graphorm.mappings?.FeedbackItem?.fields?.linearIssueUrl?.transform; + return { nodes: feedbackItems.map((feedback) => ({ id: feedback.id, category: feedback.category, description: feedback.description, + linearIssueUrl: linearIssueUrlTransform + ? linearIssueUrlTransform( + feedback.linearIssueUrl, + ctx as Context, + feedback, + ) + : feedback.linearIssueUrl, status: feedback.status, screenshotUrl: feedback.screenshotUrl, createdAt: feedback.createdAt, @@ -284,6 +300,7 @@ export const resolvers: IResolvers = { ctx.con, ({ queryRunner }) => fetchFeedbackConnectionNodes({ + ctx, manager: queryRunner.manager, where: { userId: ctx.userId }, page, @@ -314,6 +331,7 @@ export const resolvers: IResolvers = { ctx.con, ({ queryRunner }) => fetchFeedbackConnectionNodes({ + ctx, manager: queryRunner.manager, where: { userId: args.userId }, page, @@ -361,6 +379,7 @@ export const resolvers: IResolvers = { ctx.con, ({ queryRunner }) => fetchFeedbackConnectionNodes({ + ctx, manager: queryRunner.manager, where, page,