diff --git a/howdju-common/lib/apiModels.ts b/howdju-common/lib/apiModels.ts index 7349da8c..b351383e 100644 --- a/howdju-common/lib/apiModels.ts +++ b/howdju-common/lib/apiModels.ts @@ -145,6 +145,14 @@ export const MediaExcerptSearchFilterKeys = [ "creatorUserId", "speakerPersorgId", "sourceId", + "domain", + /** + * Returns MediaExcerpts having URLs matching url. + * + * Matching means that the two are equal after removing the query parameters and fragment and + * ignoring the trailing slash. Both the `url` and `canonical_url` are considered. + */ + "url", ] as const; export type MediaExcerptSearchFilter = ToFilter< typeof MediaExcerptSearchFilterKeys diff --git a/howdju-common/lib/zodSchemas.ts b/howdju-common/lib/zodSchemas.ts index df2f9ebc..b7406f6f 100644 --- a/howdju-common/lib/zodSchemas.ts +++ b/howdju-common/lib/zodSchemas.ts @@ -395,8 +395,11 @@ export const CreateUrlInput = CreateUrl; export type CreateUrlInput = z.infer; export const UrlLocator = Entity.extend({ + mediaExcerptId: z.string(), url: Url, anchors: z.array(DomAnchor).optional(), + created: momentObject, + creatorUserId: z.string(), }); export type UrlLocator = z.output; @@ -449,7 +452,12 @@ export type SourceExcerptType = SourceExcerpt["type"]; /** @deprecated See SourceExcerpt */ export const SourceExcerptTypes = sourceExcerptTypes.Enum; -export const CreateUrlLocator = UrlLocator.omit({ id: true }).extend({ +export const CreateUrlLocator = UrlLocator.omit({ + id: true, + mediaExcerptId: true, + created: true, + creatorUserId: true, +}).extend({ url: CreateUrl, anchors: z.array(CreateDomAnchor).optional(), }); @@ -549,6 +557,23 @@ export type CreateMediaExcerptCitationInput = z.output< typeof CreateMediaExcerptCitationInput >; +/** + * A model identifying MediaExcerptCitations for deletion. + * + * Since MediaExcerptCitation is a relation and not an Entity (and so has no singular unique ID), we + * need a model to uniquely identify it for deletion. + */ +export const DeleteMediaExcerptCitation = z.object({ + mediaExcerptId: z.string(), + source: z.object({ + id: z.string(), + }), + normalPincite: z.string().optional(), +}); +export type DeleteMediaExcerptCitation = z.output< + typeof DeleteMediaExcerptCitation +>; + /** * A representation of an excerpt of some fixed media conveying speech. * * @@ -634,6 +659,9 @@ export const MediaExcerpt = Entity.extend({ citations: z.array(MediaExcerptCitation), /** Persorgs to whom users have attributed the speech in the source excerpt. */ speakers: z.array(Persorg), + created: momentObject, + creatorUserId: z.string(), + creator: UserBlurb, }); export type MediaExcerpt = z.output; @@ -1059,7 +1087,12 @@ export const CreateSourceExcerpt = z.discriminatedUnion("type", [ /** @deprecated */ export type CreateSourceExcerpt = z.infer; -const CreateMediaExcerptBase = MediaExcerpt.omit({ id: true }) +const CreateMediaExcerptBase = MediaExcerpt.omit({ + id: true, + created: true, + creatorUserId: true, + creator: true, +}) .merge(CreateModel) .extend({ localRep: MediaExcerpt.shape.localRep.omit({ normalQuotation: true }), diff --git a/howdju-service-common/lib/daos/MediaExcerptsDao.test.ts b/howdju-service-common/lib/daos/MediaExcerptsDao.test.ts index 0ea9d98c..a971af16 100644 --- a/howdju-service-common/lib/daos/MediaExcerptsDao.test.ts +++ b/howdju-service-common/lib/daos/MediaExcerptsDao.test.ts @@ -6,7 +6,7 @@ import { expectToBeSameMomentDeep, mockLogger } from "howdju-test-common"; import { endPoolAndDropDb, initDb, makeTestDbConfig } from "@/util/testUtil"; import { Database, makePool } from "../database"; -import { MediaExcerptsDao } from "../daos"; +import { MediaExcerptsDao, SourcesDao, UrlsDao } from "../daos"; import { makeTestProvider } from "@/initializers/TestProvider"; import TestHelper from "@/initializers/TestHelper"; import { MediaExcerptsService } from ".."; @@ -20,6 +20,8 @@ describe("MediaExcerptsDao", () => { let dao: MediaExcerptsDao; let mediaExcerptsService: MediaExcerptsService; + let sourcesDao: SourcesDao; + let urlsDao: UrlsDao; let testHelper: TestHelper; beforeEach(async () => { dbName = await initDb(dbConfig); @@ -31,6 +33,8 @@ describe("MediaExcerptsDao", () => { dao = provider.mediaExcerptsDao; mediaExcerptsService = provider.mediaExcerptsService; + sourcesDao = provider.sourcesDao; + urlsDao = provider.urlsDao; testHelper = provider.testHelper; }); afterEach(async () => { @@ -38,7 +42,7 @@ describe("MediaExcerptsDao", () => { }); describe("readMediaExcerptForId", () => { - test("reads a media excerpt for an ID", async () => { + test("reads a MediaExcerpt for an ID", async () => { const { authToken, user } = await testHelper.makeUser(); const mediaExcerpt = await testHelper.makeMediaExcerpt({ authToken }); @@ -53,6 +57,10 @@ describe("MediaExcerptsDao", () => { expect(readMediaExcerpt).toEqual( expectToBeSameMomentDeep( merge({}, mediaExcerpt, { + creator: { + id: user.id, + longName: user.longName, + }, localRep: { normalQuotation: "the text quote", }, @@ -103,8 +111,73 @@ describe("MediaExcerptsDao", () => { expect(readMediaExcerpt).toBeUndefined(); }); - test.todo("doesn't read a deleted media excerpt"); - test.todo("allows recreating a deleted media excerpt"); + test("doesn't read a deleted MediaExcerpt", async () => { + const { authToken } = await testHelper.makeUser(); + const mediaExcerpt = await testHelper.makeMediaExcerpt({ authToken }); + const deletedAt = utcNow(); + await dao.deleteMediaExcerpt(mediaExcerpt.id, deletedAt); + + const readMediaExcerpt = await dao.readMediaExcerptForId(mediaExcerpt.id); + + expect(readMediaExcerpt).toBeUndefined(); + }); + + test("reads a MediaExcerpt after deleting one of its Citations", async () => { + const { authToken } = await testHelper.makeUser(); + const mediaExcerpt = await testHelper.makeMediaExcerpt({ authToken }); + const deletedAt = utcNow(); + await dao.deleteMediaExcerptCitation( + mediaExcerpt.citations[0], + deletedAt + ); + + const readMediaExcerpt = await dao.readMediaExcerptForId(mediaExcerpt.id); + + expect(readMediaExcerpt).toBeDefined(); + }); + + test("reads a MediaExcerpt after deleting one of its Sources", async () => { + const { authToken } = await testHelper.makeUser(); + const mediaExcerpt = await testHelper.makeMediaExcerpt({ authToken }); + const deletedAt = utcNow(); + await sourcesDao.deleteSourceForId( + mediaExcerpt.citations[0].source.id, + deletedAt + ); + + // Act + const readMediaExcerpt = await dao.readMediaExcerptForId(mediaExcerpt.id); + + expect(readMediaExcerpt).toBeDefined(); + }); + test("reads a MediaExcerpt after deleting one of its UrlLocators", async () => { + const { authToken } = await testHelper.makeUser(); + const mediaExcerpt = await testHelper.makeMediaExcerpt({ authToken }); + const deletedAt = utcNow(); + await dao.deleteUrlLocatorForId( + mediaExcerpt.locators.urlLocators[0].id, + deletedAt + ); + + // Act + const readMediaExcerpt = await dao.readMediaExcerptForId(mediaExcerpt.id); + + expect(readMediaExcerpt).toBeDefined(); + }); + test("reads a MediaExcerpt after deleting one of its Urls", async () => { + const { authToken } = await testHelper.makeUser(); + const mediaExcerpt = await testHelper.makeMediaExcerpt({ authToken }); + const deletedAt = utcNow(); + await urlsDao.deleteUrlForId( + mediaExcerpt.locators.urlLocators[0].url.id, + deletedAt + ); + + // Act + const readMediaExcerpt = await dao.readMediaExcerptForId(mediaExcerpt.id); + + expect(readMediaExcerpt).toBeDefined(); + }); }); describe("readEquivalentUrlLocator", () => { @@ -300,6 +373,10 @@ describe("MediaExcerptsDao", () => { citations: [expect.objectContaining(citations[0])], created, creatorUserId, + creator: { + id: creator.id, + longName: creator.longName, + }, }) ); }); @@ -400,8 +477,12 @@ describe("MediaExcerptsDao", () => { ); // Act - const readMediaExcerpts = await dao.readMediaExcerptsMatchingUrl( - "https://www.example.com/the-path?otherKey=otherValue#other-fragment" + const readMediaExcerpts = await dao.readMediaExcerpts( + { + url: "https://www.example.com/the-path?otherKey=otherValue#other-fragment", + }, + [], + 5 ); // Assert @@ -460,8 +541,12 @@ describe("MediaExcerptsDao", () => { ); // Act - const readMediaExcerpts = await dao.readMediaExcerptsMatchingUrl( - "https://www.example.com/the-path?otherKey=otherValue#other-fragment" + const readMediaExcerpts = await dao.readMediaExcerpts( + { + url: "https://www.example.com/the-path?otherKey=otherValue#other-fragment", + }, + [], + 5 ); // Assert @@ -518,8 +603,10 @@ describe("MediaExcerptsDao", () => { ); // Act - const readMediaExcerpts = await dao.readMediaExcerptsMatchingDomain( - "www.example.com" + const readMediaExcerpts = await dao.readMediaExcerpts( + { domain: "www.example.com" }, + [], + 5 ); // Assert @@ -572,8 +659,10 @@ describe("MediaExcerptsDao", () => { ); // Act - const readMediaExcerpts = await dao.readMediaExcerptsMatchingDomain( - "www.example.com" + const readMediaExcerpts = await dao.readMediaExcerpts( + { domain: "www.example.com" }, + [], + 5 ); // Assert diff --git a/howdju-service-common/lib/daos/MediaExcerptsDao.ts b/howdju-service-common/lib/daos/MediaExcerptsDao.ts index b8744f3d..b4142278 100644 --- a/howdju-service-common/lib/daos/MediaExcerptsDao.ts +++ b/howdju-service-common/lib/daos/MediaExcerptsDao.ts @@ -28,6 +28,8 @@ import { removeQueryParamsAndFragment, MediaExcerptCitationOut, SourceRef, + DeleteMediaExcerptCitation, + toJson, } from "howdju-common"; import { Database, TxnClient } from "../database"; @@ -35,9 +37,9 @@ import { PersorgsDao } from "./PersorgsDao"; import { SourcesDao } from "./SourcesDao"; import { UrlsDao } from "./UrlsDao"; import { toDbDirection } from "./daoModels"; -import { EntityNotFoundError, InvalidRequestError } from ".."; +import { EntityNotFoundError, InvalidRequestError, UsersDao } from ".."; import { SqlClause } from "./daoTypes"; -import { renumberSqlArgs } from "./daosUtil"; +import { renumberSqlArgs, toIdString } from "./daosUtil"; export type CreateMediaExcerptDataIn = Pick< PartialPersist, @@ -45,25 +47,14 @@ export type CreateMediaExcerptDataIn = Pick< >; export class MediaExcerptsDao { - private logger: Logger; - private database: Database; - private urlsDao: UrlsDao; - private sourcesDao: SourcesDao; - private persorgsDao: PersorgsDao; - constructor( - logger: Logger, - database: Database, - urlsDao: UrlsDao, - sourcesDao: SourcesDao, - persorgsDao: PersorgsDao - ) { - this.logger = logger; - this.database = database; - this.urlsDao = urlsDao; - this.sourcesDao = sourcesDao; - this.persorgsDao = persorgsDao; - } + private logger: Logger, + private database: Database, + private urlsDao: UrlsDao, + private sourcesDao: SourcesDao, + private persorgsDao: PersorgsDao, + private usersDao: UsersDao + ) {} async readMediaExcerptsForIds(ids: EntityId[]) { return await Promise.all(ids.map((id) => this.readMediaExcerptForId(id))); @@ -76,16 +67,23 @@ export class MediaExcerptsDao { rows: [row], } = await this.database.query( "readMediaExceptForId", - `select * from media_excerpts where media_excerpt_id = $1`, + ` + select * + from media_excerpts + where + media_excerpt_id = $1 + and deleted is null + `, [mediaExcerptId] ); if (!row) { return undefined; } - const [urlLocators, citations, speakers] = await Promise.all([ + const [urlLocators, citations, speakers, creator] = await Promise.all([ this.readUrlLocatorsForMediaExcerptId(mediaExcerptId), this.readCitationsForMediaExcerptId(mediaExcerptId), this.readSpeakersForMediaExcerptId(mediaExcerptId), + this.usersDao.readUserBlurbForId(row.creator_user_id), ]); return brandedParse(MediaExcerptRef, { id: row.media_excerpt_id, @@ -100,13 +98,23 @@ export class MediaExcerptsDao { speakers, created: row.created, creatorUserId: row.creator_user_id, + creator, }); } private async readSpeakersForMediaExcerptId(mediaExcerptId: EntityId) { const { rows } = await this.database.query( "readSpeakersForMediaExcerptId", - `select * from media_excerpt_speakers where media_excerpt_id = $1 and deleted is null`, + ` + select mes.* + from + media_excerpt_speakers mes + join persorgs p on mes.speaker_persorg_id = p.persorg_id + where + media_excerpt_id = $1 + and mes.deleted is null + and p.deleted is null + `, [mediaExcerptId] ); return await Promise.all( @@ -120,10 +128,18 @@ export class MediaExcerptsDao { const { rows } = await this.database.query( "readCitationsForMediaExcerptId", ` - select * - from media_excerpt_citations - where media_excerpt_id = $1 and deleted is null - order by media_excerpt_id asc, source_id asc, normal_pincite asc`, + select mec.* + from + media_excerpt_citations mec + join sources s using (source_id) + where + mec.media_excerpt_id = $1 + and mec.deleted is null + and s.deleted is null + order by + media_excerpt_id asc + , source_id asc + , normal_pincite asc`, [mediaExcerptId] ); return await Promise.all( @@ -149,7 +165,13 @@ export class MediaExcerptsDao { ): Promise { const { rows } = await this.database.query( "readUrlLocatorsForMediaExcerptId", - `select * from url_locators where media_excerpt_id = $1 and deleted is null`, + ` + select ul.* + from url_locators ul + join urls u using (url_id) + where media_excerpt_id = $1 + and ul.deleted is null + and u.deleted is null`, [mediaExcerptId] ); return await Promise.all( @@ -162,11 +184,12 @@ export class MediaExcerptsDao { row.url_locator_id ); return brandedParse(UrlLocatorRef, { - id: row.url_locator_id, + id: toIdString(row.url_locator_id), + mediaExcerptId: toIdString(row.media_excerpt_id), url, anchors, created: row.created, - creatorUserId: row.creator_user_id, + creatorUserId: toIdString(row.creator_user_id), }); }) ); @@ -320,29 +343,31 @@ export class MediaExcerptsDao { // Creating the UrlLocators and Citations in the serializable transaction will conflict // with having read them in the readEquivalentMediaExcerpts query above. - const urlLocators = await Promise.all( - createUrlLocators.map((createUrlLocator) => - this.createUrlLocator({ - client, - creatorUserId, - mediaExcerpt, - createUrlLocator, - created, - }) - ) - ); - - const citations = await Promise.all( - createCitations.map((createCitation) => - this.createMediaExcerptCitation({ - client, - creatorUserId, - mediaExcerpt, - createCitation, - created, - }) - ) - ); + const [urlLocators, citations, creator] = await Promise.all([ + await Promise.all( + createUrlLocators.map((createUrlLocator) => + this.createUrlLocator({ + client, + creatorUserId, + mediaExcerpt, + createUrlLocator, + created, + }) + ) + ), + await Promise.all( + createCitations.map((createCitation) => + this.createMediaExcerptCitation({ + client, + creatorUserId, + mediaExcerpt, + createCitation, + created, + }) + ) + ), + await this.usersDao.readUserBlurbForId(creatorUserId), + ]); return { mediaExcerpt: merge({}, mediaExcerpt, { @@ -350,6 +375,7 @@ export class MediaExcerptsDao { urlLocators, }, citations, + creator, }), isExtant: false, }; @@ -528,7 +554,8 @@ export class MediaExcerptsDao { return brandedParse( UrlLocatorRef, merge({}, createUrlLocator, { - id: row.url_locator_id, + id: toIdString(row.url_locator_id), + mediaExcerptId: mediaExcerpt.id, created: row.created, creatorUserId: row.creator_user_id, anchors, @@ -573,6 +600,7 @@ export class MediaExcerptsDao { return brandedParse(UrlLocatorRef, { ...createUrlLocator, id, + mediaExcerptId: mediaExcerpt.id, anchors, created, creatorUserId, @@ -836,7 +864,7 @@ export class MediaExcerptsDao { orderBySqls.push(`${columnName} ${direction}`); }); - const filterSubselects = makeFilterSubselects(filters); + const filterSubselects = this.makeFilterSubselects(filters); filterSubselects.forEach(({ sql, args: subselectArgs }) => { const renumberedSql = renumberSqlArgs(sql, args.length); whereSqls.push(`media_excerpt_id in (${renumberedSql})`); @@ -896,7 +924,7 @@ export class MediaExcerptsDao { orderBySqls.push(`${columnName} ${direction}`); }); - const filterSubselects = makeFilterSubselects(filters); + const filterSubselects = this.makeFilterSubselects(filters); const filterWhereSqls: string[] = []; filterSubselects.forEach(({ sql, args: subselectArgs }) => { const renumberedSql = renumberSqlArgs(sql, args.length); @@ -934,63 +962,46 @@ export class MediaExcerptsDao { return mediaExcerpts.filter(isDefined); } - /** - * Returns MediaExcerpts having URLs matching url. - * - * Matching means that the two are equal after removing the query parameters and fragment and - * ignoring the trailing slash. Both the `url` and `canonical_url` are considered. - */ - async readMediaExcerptsMatchingUrl(url: string) { - url = removeQueryParamsAndFragment(url); - url = url.endsWith("/") ? url.slice(0, -1) : url; - const { rows } = await this.database.query( - "readMediaExcerptsMatchingUrl", - ` - select - distinct media_excerpt_id - from - media_excerpts me - join url_locators ul using (media_excerpt_id) - join urls u using (url_id), - trim(trailing '/' from substring(u.url from '([^?#]*)')) origin_and_path, - trim(trailing '/' from substring(u.url from '([^?#]*)')) canonical_origin_and_path - where - me.deleted is null - and ul.deleted is null - and u.deleted is null - and (origin_and_path = $1 or canonical_origin_and_path = $1) - `, - [url] - ); - const mediaExcerpts = await Promise.all( - rows.map((row) => this.readMediaExcerptForId(row.media_excerpt_id)) + async deleteMediaExcerpt(mediaExcerptId: string, deletedAt: Moment) { + await this.database.query( + "deleteMediaExcerpt", + `update media_excerpts + set deleted = $1 + where media_excerpt_id = $2 and deleted is null`, + [deletedAt, mediaExcerptId] ); - return mediaExcerpts.filter(isDefined); } - async readMediaExcerptsMatchingDomain(domain: string) { - const { rows } = await this.database.query( - "readMediaExcerptsHavingDomain", + deleteMediaExcerptCitation( + { mediaExcerptId, source, normalPincite }: DeleteMediaExcerptCitation, + deletedAt: Moment + ) { + const args = [deletedAt, mediaExcerptId, source.id]; + if (normalPincite) { + args.push(normalPincite); + } + return this.database.query( + "deleteMediaExcerptCitation", ` - select - distinct media_excerpt_id - from - media_excerpts me - join url_locators ul using (media_excerpt_id) - join urls u using (url_id), - substring(u.url from '(?:.*://)?([^/?]*)') domain + update media_excerpt_citations + set deleted = $1 where - me.deleted is null - and ul.deleted is null - and u.deleted is null - and (domain = $1 or domain ilike '%.' || $1) - `, - [domain] + media_excerpt_id = $2 + and source_id = $3 + and ${normalPincite ? `normal_pincite = $4` : `normal_pincite is null`} + and deleted is null`, + args ); - const mediaExcerpts = await Promise.all( - rows.map((row) => this.readMediaExcerptForId(row.media_excerpt_id)) + } + + deleteUrlLocatorForId(urlLocatorId: string, deletedAt: Moment) { + return this.database.query( + "deleteUrlLocatorForId", + `update url_locators + set deleted = $1 + where url_locator_id = $2 and deleted is null`, + [deletedAt, urlLocatorId] ); - return mediaExcerpts.filter(isDefined); } async deleteMediaExcerptCitationsForSourceId( @@ -1005,48 +1016,109 @@ export class MediaExcerptsDao { [deletedAt, sourceId] ); } -} -function makeFilterSubselects(filters: MediaExcerptSearchFilter | undefined) { - const filterSubselects: SqlClause[] = []; - if (!filters) { - return filterSubselects; - } - let filterName: keyof MediaExcerptSearchFilter; - for (filterName in filters) { - const value = filters[filterName]; - switch (filterName) { - case "creatorUserId": { - const sql = ` - select media_excerpt_id + private makeFilterSubselects(filters: MediaExcerptSearchFilter | undefined) { + const filterSubselects: SqlClause[] = []; + if (!filters) { + return filterSubselects; + } + let filterName: keyof MediaExcerptSearchFilter; + for (filterName in filters) { + const value = filters[filterName]; + if (!value) { + this.logger.error( + `makeFilterSubselects: filter value was mising for ${filterName} (filters ${toJson( + filters + )})` + ); + continue; + } + switch (filterName) { + case "creatorUserId": { + const sql = ` + select distinct me.media_excerpt_id + from + media_excerpts me + join users u on me.creator_user_id = u.user_id + where + me.creator_user_id = $1 + and me.deleted is null + and u.deleted is null + `; + const args = [value]; + filterSubselects.push({ sql, args }); + break; + } + case "speakerPersorgId": { + const sql = ` + select distinct media_excerpt_id from media_excerpts - where creator_user_id = $1 and deleted is null + join media_excerpt_speakers mes using (media_excerpt_id) + join persorgs p on mes.speaker_persorg_id = p.persorg_id + where speaker_persorg_id = $1 and mes.deleted is null and p.deleted is null `; - const args = [value]; - filterSubselects.push({ sql, args }); - break; - } - case "speakerPersorgId": { - const sql = ` - select media_excerpt_id - from media_excerpts join media_excerpt_speakers mes using (media_excerpt_id) - where speaker_persorg_id = $1 and mes.deleted is null + const args = [value]; + filterSubselects.push({ sql, args }); + break; + } + case "sourceId": { + const sql = ` + select distinct media_excerpt_id + from media_excerpts + join media_excerpt_citations mec using (media_excerpt_id) + join sources s using (source_id) + where + mec.source_id = $1 + and mec.deleted is null + and s.deleted is null `; - const args = [value]; - filterSubselects.push({ sql, args }); - break; - } - case "sourceId": { - const sql = ` - select media_excerpt_id - from media_excerpts join media_excerpt_citations mec using (media_excerpt_id) - where mec.source_id = $1 and mec.deleted is null + const args = [value]; + filterSubselects.push({ sql, args }); + break; + } + case "domain": { + const sql = ` + select + distinct media_excerpt_id + from + media_excerpts me + join url_locators ul using (media_excerpt_id) + join urls u using (url_id), + substring(u.url from '(?:.*://)?([^/?]*)') domain + where + me.deleted is null + and ul.deleted is null + and u.deleted is null + and (domain = $1 or domain ilike '%.' || $1) + `; + const args = [value]; + filterSubselects.push({ sql, args }); + break; + } + case "url": { + let url = removeQueryParamsAndFragment(value); + url = url.endsWith("/") ? url.slice(0, -1) : url; + const sql = ` + select + distinct media_excerpt_id + from + media_excerpts me + join url_locators ul using (media_excerpt_id) + join urls u using (url_id), + trim(trailing '/' from substring(u.url from '([^?#]*)')) origin_and_path, + trim(trailing '/' from substring(u.url from '([^?#]*)')) canonical_origin_and_path + where + me.deleted is null + and ul.deleted is null + and u.deleted is null + and (origin_and_path = $1 or canonical_origin_and_path = $1) `; - const args = [value]; - filterSubselects.push({ sql, args }); - break; + const args = [url]; + filterSubselects.push({ sql, args }); + break; + } } } + return filterSubselects; } - return filterSubselects; } diff --git a/howdju-service-common/lib/daos/SourcesDao.ts b/howdju-service-common/lib/daos/SourcesDao.ts index 42061660..1fb19145 100644 --- a/howdju-service-common/lib/daos/SourcesDao.ts +++ b/howdju-service-common/lib/daos/SourcesDao.ts @@ -112,7 +112,9 @@ export class SourcesDao { await this.db.query( "deleteSourceForId", ` - update sources set deleted = $2 where source_id = $1 + update sources + set deleted = $2 + where source_id = $1 and deleted is null `, [sourceId, deletedAt] ); diff --git a/howdju-service-common/lib/daos/UrlsDao.ts b/howdju-service-common/lib/daos/UrlsDao.ts index b5d7c187..cd4e5e77 100644 --- a/howdju-service-common/lib/daos/UrlsDao.ts +++ b/howdju-service-common/lib/daos/UrlsDao.ts @@ -173,4 +173,16 @@ export class UrlsDao { ) .then(({ rows: [row] }) => toUrl(row)); } + + deleteUrlForId(urlId: EntityId, deletedAt: Moment) { + return this.database.query( + "deleteUrlForId", + ` + update urls + set deleted = $2 + where url_id = $1 and deleted is null + `, + [urlId, deletedAt] + ); + } } diff --git a/howdju-service-common/lib/initializers/daosInit.ts b/howdju-service-common/lib/initializers/daosInit.ts index 6e2deac2..1d4615ae 100644 --- a/howdju-service-common/lib/initializers/daosInit.ts +++ b/howdju-service-common/lib/initializers/daosInit.ts @@ -91,7 +91,8 @@ export function daosInitializer(provider: DatabaseProvider) { database, urlsDao, sourcesDao, - persorgsDao + persorgsDao, + usersDao ); const justificationsDao = new JustificationsDao( logger, diff --git a/howdju-service-common/lib/initializers/servicesInit.ts b/howdju-service-common/lib/initializers/servicesInit.ts index ca7ec5d8..1e2670b6 100644 --- a/howdju-service-common/lib/initializers/servicesInit.ts +++ b/howdju-service-common/lib/initializers/servicesInit.ts @@ -98,8 +98,7 @@ export function servicesInitializer(provider: AwsProvider) { provider.appConfig, authService, permissionsService, - provider.persorgsDao, - provider.mediaExcerptsDao + provider.persorgsDao ); const statementsService = new StatementsService( @@ -147,12 +146,13 @@ export function servicesInitializer(provider: AwsProvider) { provider.appConfig, authService, permissionsService, - provider.sourcesDao, - provider.mediaExcerptsDao + provider.sourcesDao ); const mediaExcerptsService = new MediaExcerptsService( + provider.appConfig, authService, + permissionsService, provider.mediaExcerptsDao, sourcesService, persorgsService, diff --git a/howdju-service-common/lib/services/MainSearchService.ts b/howdju-service-common/lib/services/MainSearchService.ts index 2d800f32..b5eeb462 100644 --- a/howdju-service-common/lib/services/MainSearchService.ts +++ b/howdju-service-common/lib/services/MainSearchService.ts @@ -13,6 +13,8 @@ import { import { MediaExcerptsService } from "./MediaExcerptsService"; import { isDomain, isUrl } from "howdju-common"; +const MAX_EXCERPT_COUNT = 50; + export class MainSearchService { constructor( private tagsService: TagsService, @@ -44,9 +46,19 @@ export class MainSearchService { const isUrlSearch = isUrl(searchText); const isDomainSearch = isDomain(searchText); const mediaExcerpts = isDomainSearch - ? this.mediaExcerptsService.readMediaExcerptsMatchingDomain(searchText) + ? this.mediaExcerptsService.readMediaExcerpts( + { domain: searchText }, + [], + undefined, + MAX_EXCERPT_COUNT + ) : isUrlSearch - ? this.mediaExcerptsService.readMediaExcerptsMatchingUrl(searchText) + ? this.mediaExcerptsService.readMediaExcerpts( + { url: searchText }, + [], + undefined, + MAX_EXCERPT_COUNT + ) : this.mediaExcerptsSearcher.search(searchText); // TODO(466) unify entities into a singular search result. return Bluebird.props({ diff --git a/howdju-service-common/lib/services/MediaExcerptsService.test.ts b/howdju-service-common/lib/services/MediaExcerptsService.test.ts index c0f4f805..a380a413 100644 --- a/howdju-service-common/lib/services/MediaExcerptsService.test.ts +++ b/howdju-service-common/lib/services/MediaExcerptsService.test.ts @@ -3,10 +3,12 @@ import { merge, toNumber } from "lodash"; import { CreateMediaExcerpt, + MediaExcerptOut, MediaExcerptSearchFilter, MomentConstructor, sleep, SortDescription, + utcNow, } from "howdju-common"; import { expectToBeSameMomentDeep, mockLogger } from "howdju-test-common"; @@ -15,6 +17,7 @@ import { Database, makePool } from "../database"; import { makeTestProvider } from "@/initializers/TestProvider"; import TestHelper from "@/initializers/TestHelper"; import { MediaExcerptsService } from "./MediaExcerptsService"; +import { SourcesDao, UrlsDao } from "../daos"; const dbConfig = makeTestDbConfig(); @@ -23,6 +26,8 @@ describe("MediaExcerptsService", () => { let pool: Pool; let service: MediaExcerptsService; + let sourcesDao: SourcesDao; + let urlsDao: UrlsDao; let testHelper: TestHelper; beforeEach(async () => { dbName = await initDb(dbConfig); @@ -33,6 +38,8 @@ describe("MediaExcerptsService", () => { const provider = makeTestProvider(database); service = provider.mediaExcerptsService; + sourcesDao = provider.sourcesDao; + urlsDao = provider.urlsDao; testHelper = provider.testHelper; }); afterEach(async () => { @@ -81,13 +88,18 @@ describe("MediaExcerptsService", () => { const creatorInfo = { creatorUserId: user.id, - created: expect.any(MomentConstructor), + created: mediaExcerpt.created, }; expect(isExtant).toBe(false); expect(mediaExcerpt).toEqual( expectToBeSameMomentDeep( merge({}, createMediaExcerpt, { id: expect.any(String), + ...creatorInfo, + creator: { + id: user.id, + longName: user.longName, + }, localRep: { normalQuotation: "the text quote", }, @@ -95,6 +107,7 @@ describe("MediaExcerptsService", () => { urlLocators: [ { id: expect.any(String), + mediaExcerptId: mediaExcerpt.id, url: { id: expect.any(String), canonicalUrl: url.url, @@ -132,7 +145,6 @@ describe("MediaExcerptsService", () => { normalName: "the speaker", }, ], - ...creatorInfo, }) ) ); @@ -404,7 +416,7 @@ describe("MediaExcerptsService", () => { }); test("re-uses related entities for concurrent attempts", async () => { // Arrange - const count = 10; + const count = 8; const users = await Promise.all( Array.from({ length: count }).map((_, i) => testHelper.makeUser({ @@ -563,6 +575,85 @@ describe("MediaExcerptsService", () => { ); } }); + test("allows recreating a deleted MediaExcerpt", async () => { + const { authToken } = await testHelper.makeUser(); + const url = { url: "https://www.example.com" }; + const createMediaExcerpt: CreateMediaExcerpt = { + localRep: { + quotation: "the text quote", + }, + locators: { + urlLocators: [ + { + url, + anchors: [ + { + exactText: "exact text", + prefixText: "prefix text", + suffixText: "suffix text", + startOffset: 0, + endOffset: 1, + }, + ], + }, + ], + }, + citations: [ + { + source: { + description: "the source description", + }, + pincite: "the pincite", + }, + ], + speakers: [{ name: "the speaker", isOrganization: false }], + }; + + const { mediaExcerpt } = await service.readOrCreateMediaExcerpt( + { authToken }, + createMediaExcerpt + ); + + await service.deleteMediaExcerpt({ authToken }, mediaExcerpt.id); + + // Act + const { isExtant, mediaExcerpt: recreatedMediaExcerpt } = + await service.readOrCreateMediaExcerpt( + { authToken }, + createMediaExcerpt + ); + + // Assert + expect(isExtant).toBe(false); + expect(recreatedMediaExcerpt).toEqual( + expectToBeSameMomentDeep( + merge({}, mediaExcerpt, { + id: expect.any(String), + created: recreatedMediaExcerpt.created, + citations: mediaExcerpt.citations.map((citation) => ({ + ...citation, + mediaExcerptId: recreatedMediaExcerpt.id, + created: recreatedMediaExcerpt.created, + })), + locators: { + urlLocators: mediaExcerpt.locators.urlLocators.map( + (urlLocator) => ({ + ...urlLocator, + id: expect.any(String), + mediaExcerptId: recreatedMediaExcerpt.id, + created: recreatedMediaExcerpt.created, + anchors: urlLocator.anchors?.map((anchor) => ({ + ...anchor, + urlLocatorId: expect.any(String), + created: recreatedMediaExcerpt.created, + })), + }) + ), + }, + }) + ) + ); + }); }); describe("readMediaExcerptForId", () => { @@ -613,13 +704,8 @@ describe("MediaExcerptsService", () => { const count = 3; const continuationToken = undefined; - mediaExcerpts.sort((a, b) => toNumber(a.id) - toNumber(b.id)); - const expectedMediaExcerpts = mediaExcerpts - .filter((me) => me.speakers[0].id === speaker1.id) - .slice(0, count); - // Act - const { mediaExcerpts: mediaExcerptsOut } = + const { mediaExcerpts: initialMediaExcerpts } = await service.readMediaExcerpts( filters, sorts, @@ -628,7 +714,11 @@ describe("MediaExcerptsService", () => { ); // Assert - expect(mediaExcerptsOut).toIncludeSameMembers( + const expectedMediaExcerpts = mediaExcerpts + .sort((a, b) => toNumber(a.id) - toNumber(b.id)) + .filter((me) => me.speakers[0].id === speaker1.id) + .slice(0, count); + expect(initialMediaExcerpts).toIncludeSameMembers( expectToBeSameMomentDeep(expectedMediaExcerpts) ); }); @@ -677,7 +767,7 @@ describe("MediaExcerptsService", () => { .slice(count, 2 * count); // Act - const { mediaExcerpts: mediaExcerptsOut } = + const { mediaExcerpts: moreMediaExcerpts } = await service.readMediaExcerpts( filters, sorts, @@ -686,9 +776,198 @@ describe("MediaExcerptsService", () => { ); // Assert - expect(mediaExcerptsOut).toIncludeSameMembers( + expect(moreMediaExcerpts).toIncludeSameMembers( expectToBeSameMomentDeep(expectedMediaExcerpts) ); }); + + test("doesn't read deleted MediaExcerpts", async () => { + const { authToken } = await testHelper.makeUser(); + const mediaExcerpts = await Promise.all( + Array.from({ length: 10 }).map(async (_, i) => { + await sleep(Math.random() * 1000); + return await testHelper.makeMediaExcerpt( + { authToken }, + { + localRep: { + // Make quotaiton unique to avoid any equivalence. + quotation: `The most magical thing ${i}`, + }, + speakers: [{ name: "Name 1", isOrganization: false }], + } + ); + }) + ); + // Media excerpts are created out of ID order above, but will be returned sorted by ID + const filters: MediaExcerptSearchFilter | undefined = undefined; + const sorts: SortDescription[] = []; + const count = 3; + + const { extant: expectedMediaExcerpts, toDelete } = mediaExcerpts + .sort((a, b) => toNumber(a.id) - toNumber(b.id)) + .reduce( + (acc, me) => { + if (toNumber(me.id) % 2 === 0) { + acc.extant.push(me); + } else { + acc.toDelete.push(me); + } + return acc; + }, + { extant: [] as MediaExcerptOut[], toDelete: [] as MediaExcerptOut[] } + ); + await Promise.all( + toDelete.map((me) => service.deleteMediaExcerpt({ authToken }, me.id)) + ); + + // Act + const { mediaExcerpts: initialMediaExcerpts, continuationToken } = + await service.readMediaExcerpts( + filters, + sorts, + /*continuationToken=*/ undefined, + count + ); + const { mediaExcerpts: moreMediaExcerpts } = + await service.readMediaExcerpts( + undefined, + [], + continuationToken, + count + ); + + // Assert + expect(initialMediaExcerpts).toIncludeSameMembers( + expectToBeSameMomentDeep(expectedMediaExcerpts.slice(0, count)) + ); + expect(moreMediaExcerpts).toIncludeSameMembers( + expectToBeSameMomentDeep(expectedMediaExcerpts.slice(count, 2 * count)) + ); + }); + test("doesn't read MediaExcerpts for a deleted Citation", async () => { + const { authToken } = await testHelper.makeUser(); + const mediaExcerpt = await testHelper.makeMediaExcerpt( + { authToken }, + { + localRep: { + // Make quotaiton unique to avoid any equivalence. + quotation: `The most important thing is to remember the most important thing`, + }, + speakers: [{ name: "Name 1", isOrganization: false }], + } + ); + + await Promise.all( + mediaExcerpt.citations.map((citation) => + service.deleteCitation({ authToken }, citation) + ) + ); + + const sourceId = mediaExcerpt.citations[0].source.id; + const filters: MediaExcerptSearchFilter = { sourceId }; + const sorts: SortDescription[] = []; + const count = 3; + + // Act + const { mediaExcerpts } = await service.readMediaExcerpts( + filters, + sorts, + /*continuationToken=*/ undefined, + count + ); + + // Assert + expect(mediaExcerpts).toBeEmpty(); + }); + test("doesn't read MediaExcerpts for a deleted Source", async () => { + const { authToken } = await testHelper.makeUser(); + const mediaExcerpt = await testHelper.makeMediaExcerpt( + { authToken }, + { + localRep: { + // Make quotaiton unique to avoid any equivalence. + quotation: `The most important thing is to remember the most important thing`, + }, + speakers: [{ name: "Name 1", isOrganization: false }], + } + ); + + const deletedAt = utcNow(); + await Promise.all( + mediaExcerpt.citations.map((citation) => + sourcesDao.deleteSourceForId(citation.source.id, deletedAt) + ) + ); + + const sourceId = mediaExcerpt.citations[0].source.id; + const filters: MediaExcerptSearchFilter = { sourceId }; + const sorts: SortDescription[] = []; + const count = 3; + + // Act + const { mediaExcerpts } = await service.readMediaExcerpts( + filters, + sorts, + /*continuationToken=*/ undefined, + count + ); + + // Assert + expect(mediaExcerpts).toBeEmpty(); + }); + + test("doesn't read MediaExcerpts for a deleted UrlLocator", async () => { + const { authToken } = await testHelper.makeUser(); + const mediaExcerpt = await testHelper.makeMediaExcerpt({ authToken }); + + await Promise.all( + mediaExcerpt.locators.urlLocators.map((urlLocator) => + service.deleteUrlLocator({ authToken }, urlLocator) + ) + ); + + const url = mediaExcerpt.locators.urlLocators[0].url.url; + const filters: MediaExcerptSearchFilter = { url }; + const sorts: SortDescription[] = []; + const count = 3; + + // Act + const { mediaExcerpts } = await service.readMediaExcerpts( + filters, + sorts, + /*continuationToken=*/ undefined, + count + ); + + // Assert + expect(mediaExcerpts).toBeEmpty(); + }); + test("doesn't read MediaExcerpts for a deleted Url", async () => { + const { authToken } = await testHelper.makeUser(); + const mediaExcerpt = await testHelper.makeMediaExcerpt({ authToken }); + + const deletedAt = utcNow(); + await Promise.all( + mediaExcerpt.locators.urlLocators.map((urlLocator) => + urlsDao.deleteUrlForId(urlLocator.url.id, deletedAt) + ) + ); + + const url = mediaExcerpt.locators.urlLocators[0].url.url; + const filters: MediaExcerptSearchFilter = { url }; + const sorts: SortDescription[] = []; + const count = 3; + + // Act + const { mediaExcerpts } = await service.readMediaExcerpts( + filters, + sorts, + /*continuationToken=*/ undefined, + count + ); + + // Assert + expect(mediaExcerpts).toBeEmpty(); + }); }); }); diff --git a/howdju-service-common/lib/services/MediaExcerptsService.ts b/howdju-service-common/lib/services/MediaExcerptsService.ts index d148c6c5..d6f435c3 100644 --- a/howdju-service-common/lib/services/MediaExcerptsService.ts +++ b/howdju-service-common/lib/services/MediaExcerptsService.ts @@ -6,17 +6,21 @@ import { CreateMediaExcerpt, CreateMediaExcerptCitation, CreateUrlLocator, + DeleteMediaExcerptCitation, EntityId, + makeModelErrors, MediaExcerpt, MediaExcerptCitationOut, MediaExcerptOut, MediaExcerptRef, MediaExcerptSearchFilter, + momentAdd, newImpossibleError, PartialPersist, PersorgOut, SortDescription, SourceOut, + UrlLocatorOut, UrlOut, utcNow, } from "howdju-common"; @@ -27,7 +31,14 @@ import { MediaExcerptsDao } from "../daos"; import { PersorgsService } from "./PersorgsService"; import { UrlsService } from "./UrlsService"; import { UserIdent } from "./types"; -import { EntityNotFoundError, RequestValidationError } from ".."; +import { + ApiConfig, + AuthorizationError, + EntityNotFoundError, + EntityTooOldToModifyError, + PermissionsService, + RequestValidationError, +} from ".."; import { createContinuationToken, createNextContinuationToken, @@ -37,10 +48,14 @@ import { retryTransaction } from "./patterns"; // Must be greater than the number MediaExcerpts targeting the same URL or Source that we want // to succeed creating in parallel. -const MAX_SUPPORTED_CONCURRENT_EQUIVALENT_MEDIA_EXCERPT_CREATIONS = 10; +const CREATE_MEDIA_EXCERPT_RETRIES = 10; +const CREATE_URL_LOCATOR_RETRIES = 3; +const CREATE_CITATION_RETRIES = 3; export class MediaExcerptsService { constructor( + private config: ApiConfig, private authService: AuthService, + private permissionsService: PermissionsService, private mediaExcerptsDao: MediaExcerptsDao, private sourcesService: SourcesService, private persorgsService: PersorgsService, @@ -60,7 +75,7 @@ export class MediaExcerptsService { createMediaExcerpt: CreateMediaExcerpt ): Promise<{ isExtant: boolean; mediaExcerpt: MediaExcerptOut }> { const userId = await this.authService.readUserIdForUserIdent(userIdent); - const now = utcNow(); + const createdAt = utcNow(); const createCitations = createMediaExcerpt.citations ?? []; // Create entities that don't depend on the media excerpt's ID @@ -75,17 +90,17 @@ export class MediaExcerptsService { this.sourcesService.readOrCreateSources( userId, createCitations.map((c) => c.source), - now + createdAt ), createMediaExcerpt.speakers ? this.persorgsService.readOrCreatePersorgs( userId, createMediaExcerpt.speakers, - now + createdAt ) : { persorgs: [], isExtant: true }, createUrls - ? this.urlsService.readOrCreateUrlsAsUser(createUrls, userId, now) + ? this.urlsService.readOrCreateUrlsAsUser(createUrls, userId, createdAt) : [], ]); @@ -120,7 +135,7 @@ export class MediaExcerptsService { await this.readOrCreateMediaExcerptWithRetry( createMediaExcerpt, userId, - now, + createdAt, createUrlLocatorsWithUrl, createCitationsWithSource ); @@ -135,7 +150,7 @@ export class MediaExcerptsService { userId, mediaExcerpt, createUrlLocatorsWithUrl, - now + createdAt ).then(({ urlLocators, isExtant }) => ({ urlLocators: uniqBy( [...mediaExcerpt.locators.urlLocators, ...urlLocators], @@ -149,7 +164,7 @@ export class MediaExcerptsService { userId, mediaExcerpt, createCitationsWithSource, - now + createdAt ).then(({ citations, isExtant }) => ({ citations: uniqWith( [...mediaExcerpt.citations, ...citations], @@ -160,7 +175,12 @@ export class MediaExcerptsService { isExtant, })) : { citations: mediaExcerpt.citations, isExtant: false }, - this.ensureMediaExcerptSpeakers(userId, mediaExcerpt, speakers, now), + this.ensureMediaExcerptSpeakers( + userId, + mediaExcerpt, + speakers, + createdAt + ), ]); const isExtant = @@ -183,20 +203,18 @@ export class MediaExcerptsService { private async readOrCreateMediaExcerptWithRetry( createMediaExcerpt: CreateMediaExcerpt, userId: EntityId, - created: Moment, + createdAt: Moment, createUrlLocators: (CreateUrlLocator & { url: UrlOut })[], createCitations: (CreateMediaExcerptCitation & { source: SourceOut })[] ) { - return retryTransaction( - MAX_SUPPORTED_CONCURRENT_EQUIVALENT_MEDIA_EXCERPT_CREATIONS, - () => - this.mediaExcerptsDao.readOrCreateMediaExcerpt( - createMediaExcerpt, - userId, - created, - createUrlLocators, - createCitations - ) + return retryTransaction(CREATE_MEDIA_EXCERPT_RETRIES, () => + this.mediaExcerptsDao.readOrCreateMediaExcerpt( + createMediaExcerpt, + userId, + createdAt, + createUrlLocators, + createCitations + ) ); } @@ -223,7 +241,7 @@ export class MediaExcerptsService { createUrlLocator: PartialPersist, created: Moment ) { - return retryTransaction(3, () => + return retryTransaction(CREATE_URL_LOCATOR_RETRIES, () => this.mediaExcerptsDao.readOrCreateUrlLocator( mediaExcerpt, createUrlLocator, @@ -258,7 +276,7 @@ export class MediaExcerptsService { createCitation: PartialPersist, created: Moment ): Promise<{ citation: MediaExcerptCitationOut; isExtant: boolean }> { - return retryTransaction(3, () => + return retryTransaction(CREATE_CITATION_RETRIES, () => this.mediaExcerptsDao.readOrCreateMediaExcerptCitation( mediaExcerpt, createCitation, @@ -376,11 +394,90 @@ export class MediaExcerptsService { }; } - readMediaExcerptsMatchingUrl(url: string) { - return this.mediaExcerptsDao.readMediaExcerptsMatchingUrl(url.trim()); + async deleteMediaExcerpt( + userIdent: UserIdent, + mediaExcerptId: EntityId + ): Promise { + const userId = await this.authService.readUserIdForUserIdent(userIdent); + const mediaExcerpt = await this.mediaExcerptsDao.readMediaExcerptForId( + mediaExcerptId + ); + if (!mediaExcerpt) { + throw new EntityNotFoundError("MEDIA_EXCERPT", mediaExcerptId); + } + + await this.checkModifyPermission(userId, mediaExcerpt); + + const deletedAt = utcNow(); + await this.mediaExcerptsDao.deleteMediaExcerpt(mediaExcerptId, deletedAt); + return mediaExcerpt; + } + + async deleteCitation( + userIdent: UserIdent, + deleteMediaExcerptCitation: DeleteMediaExcerptCitation + ) { + const userId = await this.authService.readUserIdForUserIdent(userIdent); + const mediaExcerpt = await this.mediaExcerptsDao.readMediaExcerptForId( + deleteMediaExcerptCitation.mediaExcerptId + ); + if (!mediaExcerpt) { + throw new EntityNotFoundError( + "MEDIA_EXCERPT", + deleteMediaExcerptCitation.mediaExcerptId + ); + } + + await this.checkModifyPermission(userId, mediaExcerpt); + + const deletedAt = utcNow(); + await this.mediaExcerptsDao.deleteMediaExcerptCitation( + deleteMediaExcerptCitation, + deletedAt + ); + } + + async deleteUrlLocator(userIdent: UserIdent, urlLocator: UrlLocatorOut) { + const userId = await this.authService.readUserIdForUserIdent(userIdent); + const mediaExcerpt = await this.mediaExcerptsDao.readMediaExcerptForId( + urlLocator.mediaExcerptId + ); + if (!mediaExcerpt) { + throw new EntityNotFoundError("MEDIA_EXCERPT", urlLocator.mediaExcerptId); + } + + await this.checkModifyPermission(userId, mediaExcerpt); + + const deletedAt = utcNow(); + await this.mediaExcerptsDao.deleteUrlLocatorForId(urlLocator.id, deletedAt); } - readMediaExcerptsMatchingDomain(domain: string) { - return this.mediaExcerptsDao.readMediaExcerptsMatchingDomain(domain.trim()); + private async checkModifyPermission( + userId: EntityId, + mediaExcerpt: MediaExcerptOut + ) { + const hasEditPermission = await this.permissionsService.userHasPermission( + userId, + "EDIT_ANY_ENTITY" + ); + if (hasEditPermission) { + return; + } + + if (mediaExcerpt.creatorUserId !== userId) { + throw new AuthorizationError( + makeModelErrors((e) => + e("Only a Media Excerpt's creator may edit it.") + ) + ); + } + // TODO(473) disallow deletes if other users have already depended on it. + if ( + utcNow().isAfter( + momentAdd(mediaExcerpt.created, this.config.modifyEntityGracePeriod) + ) + ) { + throw new EntityTooOldToModifyError(this.config.modifyEntityGracePeriod); + } } } diff --git a/howdju-service-common/lib/services/PersorgsService.ts b/howdju-service-common/lib/services/PersorgsService.ts index 134fa041..a0bf8670 100644 --- a/howdju-service-common/lib/services/PersorgsService.ts +++ b/howdju-service-common/lib/services/PersorgsService.ts @@ -21,7 +21,7 @@ import { persorgSchema } from "./validationSchemas"; import { PermissionsService } from "./PermissionsService"; import { AuthService } from "./AuthService"; import { EntityService } from "./EntityService"; -import { MediaExcerptsDao, PersorgData, PersorgsDao } from "../daos"; +import { PersorgData, PersorgsDao } from "../daos"; import { readWriteReread, updateHandlingConstraints } from "./patterns"; import { UserIdent } from "./types"; import { ApiConfig } from ".."; @@ -37,8 +37,7 @@ export class PersorgsService extends EntityService< private config: ApiConfig, authService: AuthService, private permissionsService: PermissionsService, - private persorgsDao: PersorgsDao, - private mediaExcerptsDao: MediaExcerptsDao + private persorgsDao: PersorgsDao ) { super(persorgSchema, authService); } @@ -154,10 +153,6 @@ export class PersorgsService extends EntityService< await this.checkModifyPermission(userId, source); const deletedAt = utcNow(); - await this.mediaExcerptsDao.deleteMediaExcerptSpeakersForPersorgId( - persorgId, - deletedAt - ); await this.persorgsDao.deletePersorgForId(persorgId, deletedAt); } diff --git a/howdju-service-common/lib/services/SourcesService.ts b/howdju-service-common/lib/services/SourcesService.ts index c8dcc5d2..785b85da 100644 --- a/howdju-service-common/lib/services/SourcesService.ts +++ b/howdju-service-common/lib/services/SourcesService.ts @@ -11,7 +11,7 @@ import { utcNow, } from "howdju-common"; -import { MediaExcerptsDao, SourcesDao } from "../daos"; +import { SourcesDao } from "../daos"; import { EntityWrapper } from "../types"; import { readWriteReread, updateHandlingConstraints } from "./patterns"; import { @@ -29,8 +29,7 @@ export class SourcesService { private config: ApiConfig, private authService: AuthService, private permissionsService: PermissionsService, - private sourcesDao: SourcesDao, - private mediaExcerptsDao: MediaExcerptsDao + private sourcesDao: SourcesDao ) {} async readSourceForId(sourceId: EntityId): Promise { @@ -106,10 +105,6 @@ export class SourcesService { await this.checkModifyPermission(userId, source); const deletedAt = utcNow(); - await this.mediaExcerptsDao.deleteMediaExcerptCitationsForSourceId( - sourceId, - deletedAt - ); await this.sourcesDao.deleteSourceForId(sourceId, deletedAt); } diff --git a/howdju-service-routes/lib/routes.ts b/howdju-service-routes/lib/routes.ts index a4b3a589..44b74ad7 100644 --- a/howdju-service-routes/lib/routes.ts +++ b/howdju-service-routes/lib/routes.ts @@ -956,6 +956,22 @@ export const serviceRoutes = { } ), }, + deleteMediaExcerpt: { + path: "media-excerpts/:mediaExcerptId", + method: httpMethods.DELETE, + request: handler( + Authed.merge(PathParams("mediaExcerptId")), + async ( + appProvider: ServicesProvider, + { authToken, pathParams: { mediaExcerptId } } + ) => { + await appProvider.mediaExcerptsService.deleteMediaExcerpt( + { authToken }, + mediaExcerptId + ); + } + ), + }, /* * Auth diff --git a/premiser-ui/src/apiActions.ts b/premiser-ui/src/apiActions.ts index 3b942ed5..0dc0e163 100644 --- a/premiser-ui/src/apiActions.ts +++ b/premiser-ui/src/apiActions.ts @@ -465,6 +465,13 @@ export const api = { normalizationSchema: { mediaExcerpt: mediaExcerptSchema }, }) ), + deleteMediaExcerpt: apiActionCreator( + "DELETE_MEDIA_EXCERPT", + serviceRoutes.deleteMediaExcerpt, + (mediaExcerptId: EntityId) => ({ + pathParams: { mediaExcerptId }, + }) + ), /** @deprecated */ fetchSourceExcerptParaphrase: apiActionCreator( diff --git a/premiser-ui/src/components/mediaExcerpts/MediaExcerptViewer.tsx b/premiser-ui/src/components/mediaExcerpts/MediaExcerptViewer.tsx index 5147b371..e19e066b 100644 --- a/premiser-ui/src/components/mediaExcerpts/MediaExcerptViewer.tsx +++ b/premiser-ui/src/components/mediaExcerpts/MediaExcerptViewer.tsx @@ -14,6 +14,7 @@ import MediaExcerptCitationViewer from "./MediaExcerptCitationViewer"; import "./MediaExcerptViewer.scss"; import paths from "@/paths"; import Link from "@/Link"; +import CreationInfo from "../creationInfo/CreationInfo"; interface Props { mediaExcerpt: MediaExcerptView; @@ -26,6 +27,10 @@ export default function MediaExcerptViewer({ mediaExcerpt }: Props) { className="quotation" text={mediaExcerpt.localRep.quotation} /> +
    {mediaExcerpt.locators.urlLocators.map((urlLocator: UrlLocatorView) => (
  • diff --git a/premiser-ui/src/pages/MediaExcerptPage.tsx b/premiser-ui/src/pages/mediaExcerpt/MediaExcerptPage.tsx similarity index 76% rename from premiser-ui/src/pages/MediaExcerptPage.tsx rename to premiser-ui/src/pages/mediaExcerpt/MediaExcerptPage.tsx index a3fceca3..9f91fcbd 100644 --- a/premiser-ui/src/pages/MediaExcerptPage.tsx +++ b/premiser-ui/src/pages/mediaExcerpt/MediaExcerptPage.tsx @@ -1,6 +1,5 @@ import React, { useEffect } from "react"; import { RouteComponentProps } from "react-router"; -import { denormalize } from "normalizr"; import { CircularProgress, Divider, @@ -9,17 +8,20 @@ import { MenuButton, } from "react-md"; import { MaterialSymbol } from "react-material-symbols"; +import { Link } from "react-router-dom"; +import { push } from "connected-react-router"; -import { EntityId, MediaExcerptView } from "howdju-common"; +import { EntityId, newUnimplementedError } from "howdju-common"; -import { useAppDispatch, useAppSelector } from "@/hooks"; +import { useAppDispatch, useAppEntitySelector, useAppSelector } from "@/hooks"; import { api } from "@/apiActions"; import { mediaExcerptSchema } from "@/normalizationSchemas"; import { combineIds } from "@/viewModels"; import MediaExcerptCard from "@/components/mediaExcerpts/MediaExcerptCard"; import HowdjuHelmet from "@/Helmet"; import paths from "@/paths"; -import { Link } from "react-router-dom"; +import { flows } from "@/actions"; +import app from "@/app/appSlice"; interface MatchParams { mediaExcerptId: EntityId; @@ -35,10 +37,20 @@ export default function MediaExcerptPage(props: Props) { dispatch(api.fetchMediaExcerpt(mediaExcerptId)); }, [dispatch, mediaExcerptId]); - // eslint-disable-next-line @typescript-eslint/no-empty-function -- TODO(20) remove this. - function useInAppearance() {} - // eslint-disable-next-line @typescript-eslint/no-empty-function -- TODO(38) remove this. - function deleteMediaExcerpt() {} + function useInAppearance() { + throw newUnimplementedError("useInAppearance"); + } + function deleteMediaExcerpt() { + dispatch( + flows.apiActionOnSuccess( + api.deleteMediaExcerpt(mediaExcerptId), + app.addToast("Deleted Media Excerpt."), + push(paths.home()) + ) + ); + } + + const { isFetching } = useAppSelector((state) => state.mediaExcerptPage); // TODO(17): pass props directly after upgrading react-md to a version with correct types const menuClassNameProps = { menuClassName: "context-menu" } as any; @@ -83,17 +95,15 @@ export default function MediaExcerptPage(props: Props) { /> ); - const mediaExcerpt = useAppSelector( - (state) => - denormalize(mediaExcerptId, mediaExcerptSchema, state.entities) as - | MediaExcerptView - | undefined - ); + const mediaExcerpt = useAppEntitySelector(mediaExcerptId, mediaExcerptSchema); const title = `Media Excerpt ${mediaExcerptId}`; if (!mediaExcerpt) { - return ; + if (isFetching) { + return ; + } + return
    Media Excerpt not found.
    ; } return (
    diff --git a/premiser-ui/src/pages/mediaExcerpt/mediaExcerptPageSlice.ts b/premiser-ui/src/pages/mediaExcerpt/mediaExcerptPageSlice.ts new file mode 100644 index 00000000..74482ee0 --- /dev/null +++ b/premiser-ui/src/pages/mediaExcerpt/mediaExcerptPageSlice.ts @@ -0,0 +1,24 @@ +import { createSlice } from "@reduxjs/toolkit"; + +import { api } from "@/apiActions"; + +const initialState = { + isFetching: false, +}; + +export const mediaExcerptPageSlice = createSlice({ + name: "mediaExcerptPage", + initialState, + reducers: {}, + extraReducers(builder) { + builder.addCase(api.fetchMediaExcerpt, (state) => { + state.isFetching = true; + }); + builder.addCase(api.fetchMediaExcerpt.response, (state) => { + state.isFetching = false; + }); + }, +}); + +export default mediaExcerptPageSlice.actions; +export const mediaExcerptPage = mediaExcerptPageSlice.reducer; diff --git a/premiser-ui/src/reducers/index.ts b/premiser-ui/src/reducers/index.ts index f1da8868..b2e3e1ea 100644 --- a/premiser-ui/src/reducers/index.ts +++ b/premiser-ui/src/reducers/index.ts @@ -17,6 +17,7 @@ import { justificationsSearchPage } from "@/pages/justificationsSearch/justifica import { tagPage } from "@/pages/tag/tagPageSlice"; import { persorgPage } from "@/pages/persorg/persorgPageSlice"; import { sourcePage } from "@/pages/source/sourcePageSlice"; +import { mediaExcerptPage } from "@/pages/mediaExcerpt/mediaExcerptPageSlice"; import { justificationsPage } from "@/pages/justifications/justificationsPageSlice"; import { accountSettingsPage } from "@/pages/accountSettings/accountSettingsPageSlice"; import { propositionUsagesPage } from "@/pages/propositionUsages/propositionUsagesPageSlice"; @@ -37,6 +38,7 @@ export default (history: History) => justificationsSearchPage, mainSearch, mainSearchPage, + mediaExcerptPage, persorgPage, primaryContextTrail, privacyConsent, diff --git a/premiser-ui/src/routes.tsx b/premiser-ui/src/routes.tsx index 24fa5fe5..89853534 100644 --- a/premiser-ui/src/routes.tsx +++ b/premiser-ui/src/routes.tsx @@ -46,7 +46,7 @@ import { map } from "lodash"; import { CreatePropositionPageMode } from "./types"; import { Location, LocationState } from "history"; import { PrimaryContextTrailProvider } from "./components/contextTrail/PrimaryContextTrailProvider"; -import MediaExcerptPage from "./pages/MediaExcerptPage"; +import MediaExcerptPage from "./pages/mediaExcerpt/MediaExcerptPage"; import MediaExcerptUsagesPage from "./pages/mediaExcerptUsages/MediaExcerptUsagesPage"; const renderHomePath = (props: RouteProps) => {