From a4f4737eb1e0bcf85d53ef6adba8f2a166dc6fe3 Mon Sep 17 00:00:00 2001 From: Kat Schelonka <34227334+kschelonka@users.noreply.github.com> Date: Wed, 11 Dec 2024 10:15:05 -0800 Subject: [PATCH] feat(notes): resolve notes from a SavedItem (#994) [POCKET-10867] --- servers/notes-api/codegen.ts | 2 + servers/notes-api/schema.graphql | 27 +++ .../notes-api/src/__generated__/graphql.ts | 47 ++++- servers/notes-api/src/apollo/resolvers.ts | 20 ++ .../notes-api/src/datasources/NoteService.ts | 9 +- servers/notes-api/src/models/Note.ts | 15 +- servers/notes-api/src/models/SavedItem.ts | 44 +++++ .../notes-api/src/test/operations/queries.ts | 29 +++ .../src/test/queries/notes.integration.ts | 6 +- .../src/test/queries/savedItem.integration.ts | 187 ++++++++++++++++++ 10 files changed, 369 insertions(+), 17 deletions(-) create mode 100644 servers/notes-api/src/models/SavedItem.ts create mode 100644 servers/notes-api/src/test/queries/savedItem.integration.ts diff --git a/servers/notes-api/codegen.ts b/servers/notes-api/codegen.ts index a487ff479..e245c3b51 100644 --- a/servers/notes-api/codegen.ts +++ b/servers/notes-api/codegen.ts @@ -18,6 +18,8 @@ const config: CodegenConfig = { }, mappers: { NoteConnection: '../models/Note#NoteConnectionModel', + SavedItem: '../models/SavedItem#SavedItemModel', + Note: '../models/Note#NoteResponse', }, }, plugins: [ diff --git a/servers/notes-api/schema.graphql b/servers/notes-api/schema.graphql index 67a3c245a..d732da5e3 100644 --- a/servers/notes-api/schema.graphql +++ b/servers/notes-api/schema.graphql @@ -63,6 +63,15 @@ type Note @key(fields: "id") { type SavedItem @key(fields: "url") { """The URL of the SavedItem""" url: String! + """ + The notes associated with this SavedItem, optionally + filtered to retrieve after a 'since' parameter. + """ + notes( + pagination: PaginationInput, + sort: NoteSortInput, + filter: SavedItemNoteFilterInput + ): NoteConnection! } """Information about pagination in a connection.""" @@ -182,6 +191,24 @@ input NoteFilterInput { excludeDeleted: Boolean } +"""Filter for retrieving Notes attached to a SavedItem""" +input SavedItemNoteFilterInput { + """ + Filter to retrieve Notes by archived status (true/false). + If not provided, notes will not be filtered by archived status. + """ + archived: Boolean + """ + Filter to retrieve notes after a timestamp, e.g. for syncing. + """ + since: ISOString + """ + Filter to choose whether to include notes marked for server-side + deletion in the response (defaults to false). + """ + excludeDeleted: Boolean +} + type Query { """Retrieve a specific Note""" diff --git a/servers/notes-api/src/__generated__/graphql.ts b/servers/notes-api/src/__generated__/graphql.ts index cd624cd59..fde587af0 100644 --- a/servers/notes-api/src/__generated__/graphql.ts +++ b/servers/notes-api/src/__generated__/graphql.ts @@ -2,7 +2,8 @@ /* eslint-disable */ /* tslint:disable */ import { GraphQLResolveInfo, GraphQLScalarType, GraphQLScalarTypeConfig } from 'graphql'; -import { NoteConnectionModel } from '../models/Note'; +import { NoteConnectionModel, NoteResponse } from '../models/Note'; +import { SavedItemModel } from '../models/SavedItem'; import { IContext } from '../apollo/context'; export type Maybe = T | null; export type InputMaybe = Maybe; @@ -11,6 +12,7 @@ export type MakeOptional = Omit & { [SubKey in K]?: export type MakeMaybe = Omit & { [SubKey in K]: Maybe }; export type MakeEmpty = { [_ in K]?: never }; export type Incremental = T | { [P in keyof T]?: P extends ' $fragmentName' | '__typename' ? T[P] : never }; +export type Omit = Pick>; export type RequireFields = Omit & { [P in K]-?: NonNullable }; /** All built-in and custom scalars, mapped to their actual values */ export type Scalars = { @@ -373,10 +375,38 @@ export type QueryNotesArgs = { export type SavedItem = { __typename?: 'SavedItem'; + /** + * The notes associated with this SavedItem, optionally + * filtered to retrieve after a 'since' parameter. + */ + notes: NoteConnection; /** The URL of the SavedItem */ url: Scalars['String']['output']; }; + +export type SavedItemNotesArgs = { + filter?: InputMaybe; + pagination?: InputMaybe; + sort?: InputMaybe; +}; + +/** Filter for retrieving Notes attached to a SavedItem */ +export type SavedItemNoteFilterInput = { + /** + * Filter to retrieve Notes by archived status (true/false). + * If not provided, notes will not be filtered by archived status. + */ + archived?: InputMaybe; + /** + * Filter to choose whether to include notes marked for server-side + * deletion in the response (defaults to false). + */ + excludeDeleted?: InputMaybe; + /** Filter to retrieve notes after a timestamp, e.g. for syncing. */ + since?: InputMaybe; +}; + export type WithIndex = TObject & Record; export type ResolversObject = WithIndex; @@ -471,11 +501,11 @@ export type ResolversTypes = ResolversObject<{ ISOString: ResolverTypeWrapper; Markdown: ResolverTypeWrapper; Mutation: ResolverTypeWrapper<{}>; - Note: ResolverTypeWrapper; + Note: ResolverTypeWrapper; Boolean: ResolverTypeWrapper; NoteConnection: ResolverTypeWrapper; Int: ResolverTypeWrapper; - NoteEdge: ResolverTypeWrapper; + NoteEdge: ResolverTypeWrapper & { node?: Maybe }>; NoteFilterInput: NoteFilterInput; NoteSortBy: NoteSortBy; NoteSortInput: NoteSortInput; @@ -484,7 +514,8 @@ export type ResolversTypes = ResolversObject<{ PaginationInput: PaginationInput; ProseMirrorJson: ResolverTypeWrapper; Query: ResolverTypeWrapper<{}>; - SavedItem: ResolverTypeWrapper; + SavedItem: ResolverTypeWrapper; + SavedItemNoteFilterInput: SavedItemNoteFilterInput; ValidUrl: ResolverTypeWrapper; }>; @@ -501,18 +532,19 @@ export type ResolversParentTypes = ResolversObject<{ ISOString: Scalars['ISOString']['output']; Markdown: Scalars['Markdown']['output']; Mutation: {}; - Note: Note; + Note: NoteResponse; Boolean: Scalars['Boolean']['output']; NoteConnection: NoteConnectionModel; Int: Scalars['Int']['output']; - NoteEdge: NoteEdge; + NoteEdge: Omit & { node?: Maybe }; NoteFilterInput: NoteFilterInput; NoteSortInput: NoteSortInput; PageInfo: PageInfo; PaginationInput: PaginationInput; ProseMirrorJson: Scalars['ProseMirrorJson']['output']; Query: {}; - SavedItem: SavedItem; + SavedItem: SavedItemModel; + SavedItemNoteFilterInput: SavedItemNoteFilterInput; ValidUrl: Scalars['ValidUrl']['output']; }>; @@ -581,6 +613,7 @@ export type QueryResolvers = ResolversObject<{ __resolveReference?: ReferenceResolver, { __typename: 'SavedItem' } & GraphQLRecursivePick, ContextType>; + notes?: Resolver>; url?: Resolver; __isTypeOf?: IsTypeOfResolverFn; }>; diff --git a/servers/notes-api/src/apollo/resolvers.ts b/servers/notes-api/src/apollo/resolvers.ts index 1540d0f82..5f6caae6c 100644 --- a/servers/notes-api/src/apollo/resolvers.ts +++ b/servers/notes-api/src/apollo/resolvers.ts @@ -1,6 +1,7 @@ import { PocketDefaultScalars } from '@pocket-tools/apollo-utils'; import { Resolvers } from '../__generated__/graphql'; import { PaginationInput } from '@pocket-tools/apollo-utils'; +import { SavedItemModel } from '../models/SavedItem'; export const resolvers: Resolvers = { ...PocketDefaultScalars, @@ -20,6 +21,25 @@ export const resolvers: Resolvers = { } }, }, + SavedItem: { + notes(parent, { pagination, filter, sort }, context) { + // The GraphQL InputMaybe<> type is causing issues with + // strict nulls; so doing some manipulation here to + // make sure things are undefined vs. null + const _pagination: PaginationInput = { + ...(pagination?.after != null && { after: pagination.after }), + ...(pagination?.first != null && { first: pagination.first }), + ...(pagination?.last != null && { last: pagination.last }), + ...(pagination?.before != null && { before: pagination.before }), + }; + const opts = { + ...(pagination != null && { pagination: _pagination }), + ...(filter != null && { filter }), + ...(sort != null && { sort }), + }; + return new SavedItemModel(parent.url, context).notes(opts); + }, + }, Query: { note(root, { id }, context) { return context.NoteModel.load(id); diff --git a/servers/notes-api/src/datasources/NoteService.ts b/servers/notes-api/src/datasources/NoteService.ts index fdd77b17e..f5940965f 100644 --- a/servers/notes-api/src/datasources/NoteService.ts +++ b/servers/notes-api/src/datasources/NoteService.ts @@ -176,7 +176,9 @@ export class NotesService { * @param filters filters to apply to query * @returns SelectQueryBuilder with filters applied in where statement(s) */ - filterQuery(filters: NoteFilterInput | undefined) { + filterQuery( + filters: (NoteFilterInput & { sourceUrl?: string | undefined }) | undefined, + ) { let qb = this.context.db .selectFrom('Note') .selectAll() @@ -207,6 +209,11 @@ export class NotesService { case 'excludeDeleted': { return value === true ? eb('deleted', '=', false) : undefined; } + case 'sourceUrl': { + return value != null && typeof value === 'string' + ? eb('sourceUrl', '=', value) + : undefined; + } } }); return and(conditions.filter((_) => _ != null)); diff --git a/servers/notes-api/src/models/Note.ts b/servers/notes-api/src/models/Note.ts index e251a6773..f51d39b59 100644 --- a/servers/notes-api/src/models/Note.ts +++ b/servers/notes-api/src/models/Note.ts @@ -41,7 +41,10 @@ import { AllSelection } from 'kysely/dist/cjs/parser/select-parser'; type AllNote = AllSelection; -export type NoteConnectionModel = PaginationResult; +export type NoteConnectionModel = PaginationResult; +export type NoteResponse = Omit & { + savedItem: { url: string } | null; +}; /** * Model for retrieving and creating Notes @@ -64,7 +67,7 @@ export class NoteModel { * @param note * @returns */ - toGraphql(note: Selectable): Note { + toGraphql(note: Selectable): NoteResponse { const savedItem = note.sourceUrl != null ? { url: note.sourceUrl } : null; return { createdAt: note.createdAt, @@ -102,7 +105,7 @@ export class NoteModel { * Get multiple Notes by IDs. Prefer using `load` * unless you need to bypass cache behavior. */ - async getMany(ids: readonly string[]): Promise { + async getMany(ids: readonly string[]): Promise { const notes = await this.service.getMany(ids); return notes != null && notes.length > 0 ? notes.map((note) => this.toGraphql(note)) @@ -114,7 +117,7 @@ export class NoteModel { * behavior. Will return null if ID does not exist * or is inaccessible for the user. */ - async getOne(id: string): Promise { + async getOne(id: string): Promise { const note = await this.service.get(id); return note != null ? this.toGraphql(note) : null; } @@ -123,7 +126,7 @@ export class NoteModel { * Will return null if ID does not exist or is inaccessible * for the user. */ - async load(id: string): Promise { + async load(id: string): Promise { const note = await this.loader.load(id); return note != null ? this.toGraphql(note) : null; } @@ -300,7 +303,7 @@ export class NoteModel { */ async paginate(opts: { sort?: NoteSortInput; - filter?: NoteFilterInput; + filter?: NoteFilterInput & { sourceUrl?: string | undefined }; pagination?: PaginationInput; }): Promise { const { sort, filter, pagination } = opts; diff --git a/servers/notes-api/src/models/SavedItem.ts b/servers/notes-api/src/models/SavedItem.ts new file mode 100644 index 000000000..02f25fbfb --- /dev/null +++ b/servers/notes-api/src/models/SavedItem.ts @@ -0,0 +1,44 @@ +import { PaginationInput } from '@pocket-tools/apollo-utils'; +import { + NoteFilterInput, + NoteSortInput, + SavedItemNoteFilterInput, +} from '../__generated__/graphql'; +import { IContext } from '../apollo'; +import { NoteConnectionModel, NoteModel } from './Note'; + +/** + * Model for resolving note-related fields + * on a SavedItem entity. + */ +export class SavedItemModel { + constructor( + public readonly url: string, + public readonly context: IContext, + ) {} + /** + * Paginate over a note connection, filtered only to + * notes attached to thie save (plus other optional filters). + * @param opts pagination options + * @returns NoteConnectionModel + */ + async notes(opts: { + sort?: NoteSortInput; + filter?: SavedItemNoteFilterInput; + pagination?: PaginationInput; + }): Promise { + const noteModel = new NoteModel(this.context); + const filter: (NoteFilterInput & { sourceUrl: string }) | undefined = + opts.filter != null + ? { + ...opts.filter, + sourceUrl: this.url, + } + : { sourceUrl: this.url }; + return await noteModel.paginate({ + sort: opts.sort, + filter: filter, + pagination: opts.pagination, + }); + } +} diff --git a/servers/notes-api/src/test/operations/queries.ts b/servers/notes-api/src/test/operations/queries.ts index 738715c4c..3e37cdc82 100644 --- a/servers/notes-api/src/test/operations/queries.ts +++ b/servers/notes-api/src/test/operations/queries.ts @@ -58,3 +58,32 @@ export const GET_NOTES = print(gql` } } `); + +export const NOTES_FROM_SAVE = print(gql` + ${NoteFragment} + ${PageInfoFragment} + query NotesFromSave( + $sort: NoteSortInput + $filter: SavedItemNoteFilterInput + $pagination: PaginationInput + $url: String + ) { + _entities(representations: { url: $url, __typename: "SavedItem" }) { + ... on SavedItem { + url + notes(sort: $sort, filter: $filter, pagination: $pagination) { + pageInfo { + ...PageInfoFields + } + totalCount + edges { + cursor + node { + ...NoteFields + } + } + } + } + } + } +`); diff --git a/servers/notes-api/src/test/queries/notes.integration.ts b/servers/notes-api/src/test/queries/notes.integration.ts index 367d08d6b..30a008c09 100644 --- a/servers/notes-api/src/test/queries/notes.integration.ts +++ b/servers/notes-api/src/test/queries/notes.integration.ts @@ -241,7 +241,7 @@ describe('notes', () => { describe('filters', () => { it('returns only archived notes', async () => { const variables = { - pagination: { first: 10 }, + pagination: { first: 30 }, filter: { archived: true }, }; const res = await request(app) @@ -257,7 +257,7 @@ describe('notes', () => { }); it('returns only not-archived notes', async () => { const variables = { - pagination: { first: 10 }, + pagination: { first: 30 }, filter: { archived: false }, }; const res = await request(app) @@ -273,7 +273,7 @@ describe('notes', () => { }); it('returns notes after a timestamp', async () => { const variables = { - pagination: { first: 10 }, + pagination: { first: 30 }, filter: { since: new Date(now - 1) }, }; const res = await request(app) diff --git a/servers/notes-api/src/test/queries/savedItem.integration.ts b/servers/notes-api/src/test/queries/savedItem.integration.ts new file mode 100644 index 000000000..80b9551ce --- /dev/null +++ b/servers/notes-api/src/test/queries/savedItem.integration.ts @@ -0,0 +1,187 @@ +import { type ApolloServer } from '@apollo/server'; +import request from 'supertest'; +import { IContext, startServer } from '../../apollo'; +import { type Application } from 'express'; +import { NOTES_FROM_SAVE } from '../operations'; +import { db, roDb } from '../../datasources/db'; +import { Note as NoteFaker } from '../fakes/Note'; +import { Chance } from 'chance'; +import { sql } from 'kysely'; +import { NoteEdge } from '../../__generated__/graphql'; +let app: Application; +let server: ApolloServer; +let graphQLUrl: string; +const chance = new Chance(); +const userId = '1'; +const now = Date.now(); +const sourceUrl = + 'https://www.youtube.com/watch?v=bqloPw5wp48&pp=ygUMbmF0YWxpZSB3eW5u'; +// Seed random other notes but with only one user id +const chaff = [...Array(10).keys()].map((_) => ({ + ...NoteFaker(chance), + userId, +})); +// Seed notes for the test url +const seed = [...Array(26).keys()].map((_) => ({ + ...NoteFaker(chance), + userId, + sourceUrl, +})); +// Ensure at least one of the following cases: +// archived, at a certain time, deleted +seed.push( + { ...NoteFaker(chance), userId, archived: true, sourceUrl }, + { ...NoteFaker(chance), userId, archived: false, sourceUrl }, + { + ...NoteFaker(chance), + userId, + createdAt: new Date(now), + sourceUrl, + }, + { ...NoteFaker(chance), userId, deleted: true, sourceUrl }, +); + +beforeAll(async () => { + // port 0 tells express to dynamically assign an available port + ({ app, server, url: graphQLUrl } = await startServer(0)); + await sql`truncate table ${sql.table('Note')} CASCADE`.execute(db); + await db + .insertInto('Note') + .values(seed) + .returning(['noteId', 'userId']) + .execute(); + await db + .insertInto('Note') + .values(chaff) + .returning(['noteId', 'userId']) + .execute(); +}); +afterAll(async () => { + await sql`truncate table ${sql.table('Note')} CASCADE`.execute(db); + await server.stop(); + await db.destroy(); + await roDb.destroy(); +}); + +describe('resolving from SavedItem', () => { + it('returns paginated set for only notes on that SavedItem', async () => { + const variables = { url: sourceUrl, pagination: { first: 10 } }; + const res = await request(app) + .post(graphQLUrl) + .set({ userid: userId }) + .send({ query: NOTES_FROM_SAVE, variables }); + expect(res.body.errors).toBeUndefined(); + expect(res.body.data._entities).toBeArrayOfSize(1); + expect(res.body.data._entities[0]).toMatchObject({ + url: sourceUrl, + }); + const notes = res.body.data?._entities[0].notes; + expect(notes?.edges).toBeArrayOfSize(10); + expect(notes?.totalCount).toEqual(30); + expect(notes?.pageInfo.hasNextPage).toBeTrue(); + expect(notes?.pageInfo.hasPreviousPage).toBeFalse(); + expect(notes?.pageInfo.startCursor).toEqual(notes?.edges[0].cursor); + expect(notes?.pageInfo.endCursor).toEqual( + notes?.edges[notes.edges.length - 1].cursor, + ); + // Every note should have the same SavedItem url + const urls = notes.edges + .map((e: NoteEdge) => e.node?.savedItem?.url) + .filter((url: string | undefined) => url != null); + expect(urls).toBeArrayOfSize(10); + expect(Array.from(new Set(urls))).toBeArrayOfSize(1); + }); + it('returns only archived notes', async () => { + const variables = { + url: sourceUrl, + pagination: { first: 30 }, + filter: { archived: true }, + }; + const res = await request(app) + .post(graphQLUrl) + .set({ userid: userId }) + .send({ query: NOTES_FROM_SAVE, variables }); + expect(res.body.errors).toBeUndefined(); + expect(res.body.data._entities).toBeArrayOfSize(1); + expect(res.body.data._entities[0]).toMatchObject({ + url: sourceUrl, + }); + const notes = res.body.data?._entities[0].notes; + expect(notes.edges).toBeArray(); + expect(notes.edges.length).toBeGreaterThan(0); + const archivedCount = (notes.edges as Array) + .map((_) => +_.node!.archived) + .reduce((sum, current) => sum + current, 0); + expect(archivedCount).toEqual(notes.edges.length); + }); + it('returns only not-archived notes', async () => { + const variables = { + url: sourceUrl, + pagination: { first: 30 }, + filter: { archived: false }, + }; + const res = await request(app) + .post(graphQLUrl) + .set({ userid: userId }) + .send({ query: NOTES_FROM_SAVE, variables }); + expect(res.body.errors).toBeUndefined(); + expect(res.body.data._entities).toBeArrayOfSize(1); + expect(res.body.data._entities[0]).toMatchObject({ + url: sourceUrl, + }); + const notes = res.body.data?._entities[0].notes; + expect(notes.edges).toBeArray(); + expect(notes.edges.length).toBeGreaterThan(0); + const archivedCount = (notes.edges as Array) + .map((_) => +_.node!.archived) + .reduce((sum, current) => sum + current, 0); + expect(archivedCount).toEqual(0); + }); + it('returns only notes after a timestamp', async () => { + const variables = { + url: sourceUrl, + pagination: { first: 30 }, + filter: { since: new Date(now - 1) }, + }; + const res = await request(app) + .post(graphQLUrl) + .set({ userid: userId }) + .send({ query: NOTES_FROM_SAVE, variables }); + + expect(res.body.errors).toBeUndefined(); + expect(res.body.data._entities).toBeArrayOfSize(1); + expect(res.body.data._entities[0]).toMatchObject({ + url: sourceUrl, + }); + const notes = res.body.data?._entities[0].notes; + expect(notes?.edges).toBeArray(); + expect(notes.edges.length).toBeGreaterThan(0); + const isAfter = (notes.edges as Array).filter( + (_) => new Date(_.node!.updatedAt).getTime() > now - 1, + ); + expect(isAfter.length).toEqual(notes.edges.length); + }); + it('excludes deleted notes', async () => { + const variables = { + url: sourceUrl, + pagination: { first: 30 }, + filter: { excludeDeleted: true }, + }; + const res = await request(app) + .post(graphQLUrl) + .set({ userid: userId }) + .send({ query: NOTES_FROM_SAVE, variables }); + expect(res.body.errors).toBeUndefined(); + expect(res.body.data._entities).toBeArrayOfSize(1); + expect(res.body.data._entities[0]).toMatchObject({ + url: sourceUrl, + }); + const notes = res.body.data?._entities[0].notes; + expect(notes.edges).toBeArray(); + expect(notes.edges.length).toBeGreaterThan(0); + const deletedCount = (notes.edges as Array) + .map((_) => +_.node!.deleted) + .reduce((sum, current) => sum + current, 0); + expect(deletedCount).toEqual(0); + }); +});