diff --git a/shared/shared.ts b/shared/shared.ts index 91eb35bc3..20facf932 100644 --- a/shared/shared.ts +++ b/shared/shared.ts @@ -106,20 +106,23 @@ export function getDocumentPath(userId: string, documentKey: string, network?: s return documentPath; } -export interface IDocumentMetadata { +export interface IDocumentMetadataBase { uid: string; type: string; key: string; + title?: string|null; + visibility?: string; + investigation?: string|null; + problem?: string|null; + unit?: string|null; +} + +export interface IDocumentMetadata extends IDocumentMetadataBase { createdAt?: number; - title?: string; - originDoc?: string; + originDoc?: string|null; properties?: Record; tools?: string[]; strategies?: string[]; - investigation?: string; - problem?: string; - unit?: string|null; - visibility?: string; } export function isDocumentMetadata(o: any): o is IDocumentMetadata { return !!o.uid && !!o.type && !!o.key; diff --git a/src/components/document/document-group.tsx b/src/components/document/document-group.tsx index 40cf6830e..6c9cb1679 100644 --- a/src/components/document/document-group.tsx +++ b/src/components/document/document-group.tsx @@ -1,9 +1,8 @@ import React, { useEffect, useRef, useState } from "react"; import { observer } from "mobx-react-lite"; -import { DocumentModelType } from "../../models/document/document"; import { SimpleDocumentItem } from "../thumbnail/simple-document-item"; -import { IDocumentMetadata } from "../../../shared/shared"; import { DocumentGroup } from "../../models/stores/document-group"; +import { IDocumentMetadataModel } from "../../models/stores/sorted-documents"; import ScrollArrowIcon from "../../assets/workspace-instance-scroll.svg"; @@ -12,7 +11,7 @@ import "./document-group.scss"; interface IProps { documentGroup: DocumentGroup; secondarySort: string; - onSelectDocument: (document: DocumentModelType | IDocumentMetadata) => void; + onSelectDocument: (document: IDocumentMetadataModel) => void; } export const DocumentGroupComponent = observer(function DocumentGroupComponent(props: IProps) { @@ -119,13 +118,11 @@ export const DocumentGroupComponent = observer(function DocumentGroupComponent(p } {visibleCount < docCount && renderScrollButton("left", leftArrowDisabled)}
- {documentGroup.documents?.map((doc: any) => { + {documentGroup.documents?.map((doc) => { return ( ); diff --git a/src/components/document/document.tsx b/src/components/document/document.tsx index a94c54bd7..e84360ee1 100644 --- a/src/components/document/document.tsx +++ b/src/components/document/document.tsx @@ -11,6 +11,7 @@ import { logDocumentEvent, logDocumentViewEvent } from "../../models/document/lo import { IToolbarModel } from "../../models/stores/problem-configuration"; import { SupportType, TeacherSupportModelType, AudienceEnum } from "../../models/stores/supports"; import { WorkspaceModelType } from "../../models/stores/workspace"; +import { getDocumentTitleWithTimestamp } from "../../models/document/document-utils"; import { ENavTab } from "../../models/view/nav-tabs"; import { IconButton } from "../utilities/icon-button"; import ToggleControl from "../utilities/toggle-control"; @@ -384,7 +385,9 @@ export class DocumentComponent extends BaseComponent { { !hideButtons && }
:
- + { !hideButtons && }
} diff --git a/src/components/document/sort-work-document-area.tsx b/src/components/document/sort-work-document-area.tsx index 512af6a36..50944ccd6 100644 --- a/src/components/document/sort-work-document-area.tsx +++ b/src/components/document/sort-work-document-area.tsx @@ -1,8 +1,7 @@ import React, { useState } from "react"; import classNames from "classnames"; import { observer } from "mobx-react"; -import { useAppConfig, useProblemStore, - usePersistentUIStore, useUserStore, useClassStore, useUIStore, useStores } from "../../hooks/use-stores"; +import { useStores } from "../../hooks/use-stores"; import { DocumentModelType } from "../../models/document/document"; import { EditableDocumentContent } from "./editable-document-content"; import { getDocumentDisplayTitle } from "../../models/document/document-utils"; @@ -24,13 +23,8 @@ interface IProps { export const SortWorkDocumentArea: React.FC = observer(function SortWorkDocumentArea(props: IProps) { const { openDocumentsGroup } = props; - const store = useStores(); - const ui = useUIStore(); - const persistentUI = usePersistentUIStore(); - const user = useUserStore(); - const classStore = useClassStore(); - const problemStore = useProblemStore(); - const appConfigStore = useAppConfig(); + const {appConfig, class: classStore, documents, networkDocuments, + persistentUI, sortedDocuments, ui, unit, user} = useStores(); const tabState = persistentUI.tabs.get(ENavTab.kSortWork); const openDocumentKey = tabState?.openSubTab && tabState?.openDocuments.get(tabState.openSubTab); const showScroller = persistentUI.showDocumentScroller; @@ -43,24 +37,24 @@ export const SortWorkDocumentArea: React.FC = observer(function SortWork } const getOpenDocument = () => { - const openDoc = store.documents.getDocument(openDocumentKey) || - store.networkDocuments.getDocument(openDocumentKey); + const openDoc = documents.getDocument(openDocumentKey) || + networkDocuments.getDocument(openDocumentKey); if (openDoc) { return openDoc; } // Calling `fetchFullDocument` will update the `documents` store with the full document, // triggering a re-render of this component since it's an observer. - store.sortedDocuments.fetchFullDocument(openDocumentKey); + sortedDocuments.fetchFullDocument(openDocumentKey); }; const openDocument = getOpenDocument(); - const isVisible = openDocument?.isAccessibleToUser(user, store.documents); - const showPlayback = user.type && appConfigStore.enableHistoryRoles.includes(user.type); + const isVisible = openDocument?.isAccessibleToUser(user, documents); + const showPlayback = user.type && appConfig.enableHistoryRoles.includes(user.type); const showExemplarShare = user.type === "teacher" && openDocument && isExemplarType(openDocument.type); const getDisplayTitle = (document: DocumentModelType) => { const documentOwner = classStore.users.get(document.uid); - const documentTitle = getDocumentDisplayTitle(document, appConfigStore, problemStore, store.unit.code); + const documentTitle = getDocumentDisplayTitle(unit, document, appConfig); return {owner: documentOwner ? documentOwner.fullName : "", title: documentTitle}; }; const displayTitle = openDocument && getDisplayTitle(openDocument); diff --git a/src/components/document/sorted-section.tsx b/src/components/document/sorted-section.tsx index ee3017bd7..24d99214e 100644 --- a/src/components/document/sorted-section.tsx +++ b/src/components/document/sorted-section.tsx @@ -5,7 +5,6 @@ import classNames from "classnames"; import { DocumentModelType } from "../../models/document/document"; import { useStores } from "../../hooks/use-stores"; import { DocFilterType, SecondarySortType } from "../../models/stores/ui-types"; -import { IDocumentMetadata } from "../../../shared/shared"; import { DocumentGroup } from "../../models/stores/document-group"; import { DocumentGroupComponent } from "./document-group"; import { logDocumentViewEvent } from "../../models/document/log-document-event"; @@ -53,7 +52,7 @@ export const SortedSection: React.FC = observer(function SortedSection(p : { primaryLabel: documentGroup.label, primaryType: documentGroup.sortType, secondaryLabel: label, secondaryType: sortType }; - return async (document: DocumentModelType | IDocumentMetadata) => { + return async (document: DocumentModelType | IDocumentMetadataModel) => { const openSubTab = JSON.stringify(openSubTabMetadata); persistentUI.openSubTabDocument(ENavTab.kSortWork, openSubTab, document.key); logDocumentViewEvent(document); diff --git a/src/components/navigation/document-view.tsx b/src/components/navigation/document-view.tsx index 09e953d53..5af8fc6db 100644 --- a/src/components/navigation/document-view.tsx +++ b/src/components/navigation/document-view.tsx @@ -2,8 +2,8 @@ import React from "react"; import { observer } from "mobx-react"; import { useQueryClient } from "react-query"; import classNames from "classnames"; -import { useAppConfig, useLocalDocuments, useProblemStore, useStores, - usePersistentUIStore, useUserStore, useClassStore, useUIStore } from "../../hooks/use-stores"; +import { useAppConfig, useLocalDocuments, useStores, + usePersistentUIStore } from "../../hooks/use-stores"; import { useUserContext } from "../../hooks/use-user-context"; import { ISubTabSpec, NavTabModelType, kBookmarksTabTitle } from "../../models/view/nav-tabs"; import { DocumentType } from "../../models/document/document-types"; @@ -202,18 +202,14 @@ interface IDocumentAreaProps { } const DocumentArea = ({openDocument, subTab, tab, sectionClass, isSecondaryDocument, - hasSecondaryDocument, hideLeftFlipper, hideRightFlipper, onChangeDocument}: IDocumentAreaProps) => { - const ui = useUIStore(); - const persistentUI = usePersistentUIStore(); - const user = useUserStore(); - const appConfig = useAppConfig(); - const classStore = useClassStore(); - const problemStore = useProblemStore(); + hasSecondaryDocument, hideLeftFlipper, hideRightFlipper, onChangeDocument}: IDocumentAreaProps +) => { + const {appConfig, class: classStore, persistentUI, ui, unit, user} = useStores(); const showPlayback = user.type && !openDocument?.isPublished ? appConfig.enableHistoryRoles.includes(user.type) : false; const getDisplayTitle = (document: DocumentModelType) => { const documentOwner = classStore.users.get(document.uid); - const documentTitle = getDocumentDisplayTitle(document, appConfig, problemStore); + const documentTitle = getDocumentDisplayTitle(unit, document, appConfig); return {owner: documentOwner ? documentOwner.fullName : "", title: documentTitle}; }; const displayTitle = getDisplayTitle(openDocument); diff --git a/src/components/thumbnail/simple-document-item.tsx b/src/components/thumbnail/simple-document-item.tsx index 6a3afb0ea..5e499c9c3 100644 --- a/src/components/thumbnail/simple-document-item.tsx +++ b/src/components/thumbnail/simple-document-item.tsx @@ -1,30 +1,24 @@ import { observer } from "mobx-react"; import React from "react"; -import { IDocumentMetadata } from "../../../shared/shared"; +import { IDocumentMetadataModel } from "../../models/stores/sorted-documents"; import { useStores } from "../../hooks/use-stores"; -import { isDocumentAccessibleToUser } from "../../models/document/document-utils"; +import { getDocumentDisplayTitle, isDocumentAccessibleToUser } from "../../models/document/document-utils"; import "./simple-document-item.scss"; interface IProps { - document: IDocumentMetadata; - investigationOrdinal: string; - problemOrdinal: string; - onSelectDocument: (document: IDocumentMetadata) => void; + document: IDocumentMetadataModel; + onSelectDocument: (document: IDocumentMetadataModel) => void; } export const SimpleDocumentItem = observer(function SimpleDocumentItem( - { document, investigationOrdinal, onSelectDocument, problemOrdinal }: IProps + { document, onSelectDocument }: IProps ) { - const { documents, class: classStore, unit, user } = useStores(); + const { appConfig, documents, class: classStore, unit, user } = useStores(); const { uid } = document; const userName = classStore.getUserById(uid)?.displayName; - // TODO: Make it so we don't have to convert investigationOrdinal and problemOrdinal to numbers here? We do so - // because the values originate as strings. Changing their types to numbers in the model would make this unnecessary, - // but doing that causes errors elsewhere when trying to load documents that aren't associated with a problem. - const investigation = unit.getInvestigation(Number(investigationOrdinal)); - const problem = investigation?.getProblem(Number(problemOrdinal)); - const title = document.title ? `${userName}: ${document.title}` : `${userName}: ${problem?.title ?? "unknown title"}`; + const title = getDocumentDisplayTitle(unit, document, appConfig); + const titleWithUser = `${userName}: ${title}`; const isPrivate = !isDocumentAccessibleToUser(document, user, documents); const handleClick = () => { @@ -35,7 +29,7 @@ export const SimpleDocumentItem = observer(function SimpleDocumentItem(
diff --git a/src/hooks/use-document-caption.tsx b/src/hooks/use-document-caption.tsx index d7e955203..7d73c26de 100644 --- a/src/hooks/use-document-caption.tsx +++ b/src/hooks/use-document-caption.tsx @@ -1,14 +1,11 @@ import { useFirestoreTeacher } from "./firestore-hooks"; -import { useAppConfig, useClassStore, useProblemStore, useUserStore } from "./use-stores"; import { DocumentModelType } from "../models/document/document"; import { ExemplarDocument, isPublishedType, isUnpublishedType } from "../models/document/document-types"; import { getDocumentDisplayTitle } from "../models/document/document-utils"; +import { useStores } from "./use-stores"; export function useDocumentCaption(document: DocumentModelType, isStudentWorkspaceDoc?: boolean) { - const appConfig = useAppConfig(); - const problem = useProblemStore(); - const classStore = useClassStore(); - const user = useUserStore(); + const {appConfig, class: classStore, unit, user} = useStores(); const { type, uid } = document; const pubVersion = document.pubVersion; const teacher = useFirestoreTeacher(uid, user.network || ""); @@ -29,6 +26,6 @@ export function useDocumentCaption(document: DocumentModelType, isStudentWorkspa : isPublishedType(type) && pubVersion ? ` v${pubVersion}` : ""; - const title = getDocumentDisplayTitle(document, appConfig, problem); + const title = getDocumentDisplayTitle(unit, document, appConfig); return `${namePrefix}${title}${dateSuffix}`; } diff --git a/src/lib/db.ts b/src/lib/db.ts index f24ff93fb..cf5cf03bd 100644 --- a/src/lib/db.ts +++ b/src/lib/db.ts @@ -426,6 +426,12 @@ export class DB { if (!docSnapshot.exists) { const { classHash, self, version, ...cleanedMetadata } = metadata as DBDocumentMetadata & { classHash: string }; + + let problemInfo: {unit:string|null, investigation?: string, problem?: string} = {unit: null}; + if ("offeringId" in metadata && metadata.offeringId != null) { + problemInfo = this.currentProblemInfo; + } + const firestoreMetadata: IDocumentMetadata & { contextId: string } = { ...cleanedMetadata, // The validateCommentableDocument firebase function currently deployed to production is out of date. @@ -434,17 +440,8 @@ export class DB { key: documentKey, properties: {}, uid: userContext.uid, - unit: null + ...problemInfo }; - if ("offeringId" in metadata && metadata.offeringId != null) { - const { investigation, problem, unit } = this.stores; - const investigationOrdinal = String(investigation.ordinal); - const problemOrdinal = String(problem.ordinal); - const unitCode = unit.code; - firestoreMetadata.investigation = investigationOrdinal; - firestoreMetadata.problem = problemOrdinal; - firestoreMetadata.unit = unitCode; - } const validateCommentableDocument = getFirebaseFunction("validateCommentableDocument_v1"); // FIXME-HISTORY: rename this function to validateFirestoreDocumentMetadata_v1 @@ -452,6 +449,15 @@ export class DB { } } + private get currentProblemInfo() { + const { investigation, problem, unit } = this.stores; + return { + investigation: String(investigation.ordinal), + problem: String(problem.ordinal), + unit: unit.code + }; + } + public async createDocument(params: { type: DBDocumentType, content?: string, title?: string }) { const { type, content, title } = params; const { user } = this.stores; @@ -795,12 +801,14 @@ export class DB { metadata: DBOfferingUserProblemDocument) { const {documentKey} = metadata; const group = this.stores.groups.groupForUser(userId); + const problemInfo = this.currentProblemInfo; return this.openDocument({ type, userId, groupId: group?.id, documentKey, - visibility: metadata.visibility + visibility: metadata.visibility, + ...problemInfo }); } @@ -836,6 +844,7 @@ export class DB { return allUsers; }, {} as DBGroupUserConnections); + const problemInfo = this.currentProblemInfo; return this.openDocument({ documentKey, type: "publication", @@ -843,7 +852,8 @@ export class DB { groupId, visibility: "public", groupUserConnections: groupUserConnectionsMap, - pubVersion + pubVersion, + ...problemInfo }); } diff --git a/src/models/document/document-types.ts b/src/models/document/document-types.ts index 7ea9a33ae..7a90da467 100644 --- a/src/models/document/document-types.ts +++ b/src/models/document/document-types.ts @@ -25,6 +25,9 @@ export function isPersonalType(type: string) { export function isLearningLogType(type: string) { return [LearningLogDocument, LearningLogPublication].indexOf(type) >= 0; } +export function isSupportType(type: string) { + return type === SupportPublication; +} export function isExemplarType(type: string) { return type === ExemplarDocument; } diff --git a/src/models/document/document-utils.test.ts b/src/models/document/document-utils.test.ts new file mode 100644 index 000000000..9b6887043 --- /dev/null +++ b/src/models/document/document-utils.test.ts @@ -0,0 +1,179 @@ +import { UnitModel } from "../curriculum/unit"; +import { AppConfigModel } from "../stores/app-config-model"; +import { DocumentMetadataModel } from "../stores/sorted-documents"; +import { PersonalDocument, ProblemDocument, SupportPublication } from "./document-types"; +import { getDocumentDisplayTitle } from "./document-utils"; +import { unitConfigDefaults } from "../../test-fixtures/sample-unit-configurations"; + +describe("document utils", () => { + describe("getDocumentDisplayTitle", () => { + describe("support documents", () => { + test("without caption", () => { + const metadata = DocumentMetadataModel.create({ + type: SupportPublication, + uid: "123", + key: "123", + }); + const unit = UnitModel.create({ + code: "test", + title: "test" + }); + const appConfig = AppConfigModel.create(); + const title = getDocumentDisplayTitle(unit, metadata, appConfig); + expect(title).toBe("Support"); + }); + test("with caption", () => { + const metadata = DocumentMetadataModel.create({ + type: SupportPublication, + uid: "123", + key: "123", + properties: { + caption: "Test Title" + } + }); + const unit = UnitModel.create({ + code: "test", + title: "test" + }); + const appConfig = AppConfigModel.create(); + const title = getDocumentDisplayTitle(unit, metadata, appConfig); + expect(title).toBe("Test Title"); + }); + }); + + describe("personal documents", () => { + test("without title", () => { + const metadata = DocumentMetadataModel.create({ + type: PersonalDocument, + uid: "123", + key: "123", + }); + const unit = UnitModel.create({ + code: "test", + title: "test" + }); + const appConfig = AppConfigModel.create({config: unitConfigDefaults}); + const title = getDocumentDisplayTitle(unit, metadata, appConfig); + expect(title).toBe(null); + }); + + test("with a title", () => { + const metadata = DocumentMetadataModel.create({ + type: PersonalDocument, + uid: "123", + key: "123", + title: "Test Title" + }); + const unit = UnitModel.create({ + code: "test", + title: "test" + }); + const appConfig = AppConfigModel.create({config: unitConfigDefaults}); + const title = getDocumentDisplayTitle(unit, metadata, appConfig); + expect(title).toBe("Test Title"); + }); + + // NOTE: the default appConfig does not configure a timestamp property + // and none of the production units set this property. + // So really this timestamp feature is dead code in production + test("with a title and configured timestamp property", () => { + const metadata = DocumentMetadataModel.create({ + type: PersonalDocument, + uid: "123", + key: "123", + title: "Test Title", + properties: { + timeStamp: "193899600000" + } + }); + const unit = UnitModel.create({ + code: "test", + title: "test" + }); + const appConfig = AppConfigModel.create({config: unitConfigDefaults}); + const title = getDocumentDisplayTitle(unit, metadata, appConfig); + expect(title).toMatch(/Test Title \(23FEB76-..:..:..\)/); + }); + }); + + describe("problem documents", () => { + test("without a unit", () => { + const metadata = DocumentMetadataModel.create({ + type: ProblemDocument, + uid: "123", + key: "123", + }); + const unit = UnitModel.create({ + code: "test", + title: "test" + }); + const appConfig = AppConfigModel.create({config: unitConfigDefaults}); + const title = getDocumentDisplayTitle(unit, metadata, appConfig); + expect(title).toBe("Problem doc without unit"); + }); + test("from another unit", () => { + const metadata = DocumentMetadataModel.create({ + type: ProblemDocument, + uid: "123", + key: "123", + unit: "other", + investigation: "1", + problem: "1" + }); + const unit = UnitModel.create({ + code: "test", + title: "test" + }); + const appConfig = AppConfigModel.create({config: unitConfigDefaults}); + const title = getDocumentDisplayTitle(unit, metadata, appConfig); + expect(title).toBe("Problem doc from other-1.1"); + }); + test("from the same unit but for a problem that doesn't exist", () => { + const metadata = DocumentMetadataModel.create({ + type: ProblemDocument, + uid: "123", + key: "123", + unit: "test", + investigation: "1", + problem: "1" + }); + const unit = UnitModel.create({ + code: "test", + title: "test" + }); + const appConfig = AppConfigModel.create({config: unitConfigDefaults}); + const title = getDocumentDisplayTitle(unit, metadata, appConfig); + expect(title).toBe("Problem doc from test-1.1"); + }); + test("from the same unit", () => { + const metadata = DocumentMetadataModel.create({ + type: ProblemDocument, + uid: "123", + key: "123", + unit: "test", + investigation: "1", + problem: "1" + }); + const unit = UnitModel.create({ + code: "test", + title: "Test Unit", + investigations: [ + { + ordinal: 1, + title: "Test Investigation", + problems: [ + { + ordinal: 1, + title: "Test Problem" + } + ] + } + ] + }); + const appConfig = AppConfigModel.create({config: unitConfigDefaults}); + const title = getDocumentDisplayTitle(unit, metadata, appConfig); + expect(title).toBe("Test Problem"); + }); + }); + }); +}); diff --git a/src/models/document/document-utils.ts b/src/models/document/document-utils.ts index 0712c82db..064db89db 100644 --- a/src/models/document/document-utils.ts +++ b/src/models/document/document-utils.ts @@ -1,31 +1,70 @@ import { getParent } from "mobx-state-tree"; -import { IDocumentMetadata } from "../../../shared/shared"; -import { ProblemModelType } from "../curriculum/problem"; +import { upperFirst } from "lodash"; +import { IDocumentMetadataBase } from "../../../shared/shared"; import { SectionModelType } from "../curriculum/section"; -import { getSectionPath } from "../curriculum/unit"; +import { getSectionPath, UnitModelType } from "../curriculum/unit"; import { AppConfigModelType } from "../stores/app-config-model"; import { UserModelType } from "../stores/user"; import { DocumentModelType, IExemplarVisibilityProvider } from "./document"; import { DocumentContentModelType } from "./document-content"; -import { isExemplarType, isPlanningType, isProblemType, isPublishedType } from "./document-types"; +import { isExemplarType, isPlanningType, isProblemType, isPublishedType, isSupportType } from "./document-types"; +import { IDocumentMetadataModel } from "../stores/sorted-documents"; +import { getLocalTimeStamp } from "../../utilities/time"; + +function getProblemFromDoc(unit: UnitModelType, document: DocumentModelType | IDocumentMetadataModel) { + if (unit.code !== document.unit) { + return undefined; + } + const investigation = unit.getInvestigation(Number(document.investigation)); + const problem = investigation?.getProblem(Number(document.problem)); + return problem; +} + +function getDocumentTitleFromProblem(currentUnit: UnitModelType, document: DocumentModelType | IDocumentMetadataModel) { + const {type, unit, investigation, problem} = document; + const problemModel = getProblemFromDoc(currentUnit, document); + if (problemModel) { + if (isPlanningType(type)) { + return `${problemModel.title}: Planning`; + } + return problemModel.title; + } + + const upperType = upperFirst(document.type); + if (!unit) { + return `${upperType} doc without unit`; + } + return `${upperType} doc from ${unit}-${investigation}.${problem}`; +} + +export function getDocumentTitleWithTimestamp( + document: DocumentModelType | IDocumentMetadataModel, + appConfig: AppConfigModelType +) { + const timeStampPropName = appConfig.docTimeStampPropertyName || undefined; + const timeStampProp = timeStampPropName && document.getProperty(timeStampPropName); + const timeStamp = timeStampProp + ? parseFloat(timeStampProp) + : undefined; + const timeStampStr = timeStamp ? getLocalTimeStamp(timeStamp) : undefined; + return timeStampStr + ? `${document.title} (${timeStampStr})` + : document.title; +} export function getDocumentDisplayTitle( - document: DocumentModelType, appConfig: AppConfigModelType, problem?: ProblemModelType, - unit?: string + unit: UnitModelType, + document: DocumentModelType | IDocumentMetadataModel, + appConfig: AppConfigModelType ) { const { type } = document; - const documentProblemOrdinal = `${document.investigation}.${document.problem}`; - const problemTitle = !(document.problem || document.investigation || document.unit) || - (documentProblemOrdinal === String(problem?.ordinal) && unit === document?.unit) - ? problem?.title || "Unknown Problem" - : "Unknown Problem"; - return document.isSupport - ? document.getProperty("caption") || "Support" - : isProblemType(type) - ? problemTitle - : isPlanningType(type) - ? `${problem?.title || "Unkown"}: Planning` - : document.getDisplayTitle(appConfig); + if (isSupportType(type)) { + return document.getProperty("caption") || "Support"; + } else if (isProblemType(type) || isPlanningType(type)) { + return getDocumentTitleFromProblem(unit, document); + } else { + return getDocumentTitleWithTimestamp(document, appConfig); + } } /** @@ -48,7 +87,7 @@ export function getDocumentIdentifier(document?: DocumentContentModelType) { } export const isDocumentAccessibleToUser = ( - doc: IDocumentMetadata, user: UserModelType, documentStore: IExemplarVisibilityProvider + doc: IDocumentMetadataBase, user: UserModelType, documentStore: IExemplarVisibilityProvider ) => { const ownDocument = doc.uid === user.id; const isShared = doc.visibility === "public"; diff --git a/src/models/document/document.ts b/src/models/document/document.ts index 28b39b3ab..c5f5e367f 100644 --- a/src/models/document/document.ts +++ b/src/models/document/document.ts @@ -15,7 +15,6 @@ import { } from "../../../shared/shared"; import { getFirebaseFunction } from "../../hooks/use-firebase-function"; import { IDocumentProperties } from "../../lib/db-types"; -import { getLocalTimeStamp } from "../../utilities/time"; import { safeJsonParse } from "../../utilities/js-utils"; import { Tree } from "../history/tree"; import { TreeMonitor } from "../history/tree-monitor"; @@ -131,17 +130,6 @@ export const DocumentModel = Tree.named("Document") }); return appConfig.getDocumentLabel(docStr, count, lowerCase); }, - getDisplayTitle(appConfig: AppConfigModelType) { - const timeStampPropName = appConfig.docTimeStampPropertyName || undefined; - const timeStampProp = timeStampPropName && self.getProperty(timeStampPropName); - const timeStamp = timeStampProp - ? parseFloat(timeStampProp) - : undefined; - const timeStampStr = timeStamp ? getLocalTimeStamp(timeStamp) : undefined; - return timeStampStr - ? `${self.title} (${timeStampStr})` - : self.title; - }, getDisplayId(appConfig: AppConfigModelType) { const { docDisplayIdPropertyName } = appConfig; if (!docDisplayIdPropertyName) return undefined; diff --git a/src/models/document/log-document-event.ts b/src/models/document/log-document-event.ts index c0a1404e5..ecf30aad4 100644 --- a/src/models/document/log-document-event.ts +++ b/src/models/document/log-document-event.ts @@ -1,6 +1,7 @@ import { IDocumentMetadata } from "../../../shared/shared"; import { Logger } from "../../lib/logger"; import { LogEventMethod, LogEventName } from "../../lib/logger-types"; +import { IDocumentMetadataModel } from "../stores/sorted-documents"; import { UserModelType } from "../stores/user"; import { DocumentModelType } from "./document"; import { ExemplarDocument } from "./document-types"; @@ -11,7 +12,7 @@ interface ITeacherNetworkInfo { } export interface IDocumentLogEvent extends Record { - document: DocumentModelType | IDocumentMetadata; + document: DocumentModelType | IDocumentMetadata | IDocumentMetadataModel; } export function isDocumentLogEvent(params: Record): params is IDocumentLogEvent { @@ -59,7 +60,7 @@ export function logDocumentEvent(event: LogEventName, _params: IDocumentLogEvent * Convenience function to log the appropriate type of VIEW_SHOW_*_DOCUMENT event for the document. * @param document */ -export function logDocumentViewEvent(document: DocumentModelType | IDocumentMetadata) { +export function logDocumentViewEvent(document: DocumentModelType | IDocumentMetadata | IDocumentMetadataModel) { const isRemote = "isRemote" in document ? document.isRemote : undefined; const event = document.type === ExemplarDocument diff --git a/src/models/history/tree-manager.ts b/src/models/history/tree-manager.ts index ea0c8ee9f..3fed92c55 100644 --- a/src/models/history/tree-manager.ts +++ b/src/models/history/tree-manager.ts @@ -8,8 +8,7 @@ import { IUndoManager, UndoStore } from "./undo-store"; import { TreePatchRecord, HistoryEntry, TreePatchRecordSnapshot, HistoryOperation } from "./history"; import { DEBUG_HISTORY } from "../../lib/debug"; -import { getFirebaseFunction } from "../../hooks/use-firebase-function"; -import { getDocumentPath, ICommentableDocumentParams, IDocumentMetadata } from "../../../shared/shared"; +import { getDocumentPath, IDocumentMetadata } from "../../../shared/shared"; import { Firestore } from "../../lib/firestore"; import { UserModelType } from "../stores/user"; import { UserContextProvider } from "../stores/user-context-provider"; @@ -625,16 +624,36 @@ async function prepareFirestoreHistoryInfo( const documentPath = getDocumentPath(userContext.uid, mainDocument.key, userContext.network); const documentRef = firestore.doc(documentPath); - const docSnapshot = await documentRef.get(); - // create a document if necessary - if (!docSnapshot.exists) { - const validateCommentableDocument = - getFirebaseFunction("validateCommentableDocument_v1"); - - // FIXME-HISTORY: rename this function to validateFirestoreDocumentMetadata_v1 - await validateCommentableDocument({context: userContext, document: mainDocument.metadata}); - } + // The metadata documents are created by DB#createDocument however it does not wait for the metadata + // document to be created. So we might end up here before the metadata document has been created. + await new Promise((resolve, reject) => { + let timeoutId: NodeJS.Timeout | undefined = undefined; + const disposer = documentRef.onSnapshot(doc => { + if (doc.exists) { + resolve(); + if (timeoutId) { + disposer(); + clearTimeout(timeoutId); + } + } + }); + timeoutId = setTimeout(() => { + // If there isn't a firestore metadata document in 5 seconds then give up + disposer(); + console.warn("Could not find metadata document to attach history to", documentPath); + // If there is an error here the history will not be saved for the duration + // of this CLUE session. + // This happens because the rejection will bubble up to completeHistoryEntry. + // That does not handle errors from this promise. The "then" function will + // not be called. The error should be printed as an unhandled promise error. + // The next time a history entry is "completed" this rejected promise + // will be "then'd" again which will again not run its function. + // TODO: consider updating this to create the metadata document itself by + // calling createFirestoreMetadataDocument. + reject(`Could not find metadata document to attach history to ${documentPath}`); + }, 5000); + }); const lastHistoryEntry = await getLastHistoryEntry(firestore, documentPath); diff --git a/src/models/stores/sorted-documents.ts b/src/models/stores/sorted-documents.ts index f373e151c..750f420f0 100644 --- a/src/models/stores/sorted-documents.ts +++ b/src/models/stores/sorted-documents.ts @@ -64,7 +64,13 @@ export const DocumentMetadataModel = types.model("DocumentMetadata", { problem: types.maybeNull(types.string), unit: types.maybeNull(types.string), visibility: types.maybe(types.string) -}); +}) +.views((self) => ({ + getProperty(key: string) { + return self.properties.get(key); + }, +})); + export interface IDocumentMetadataModel extends Instance {} export const MetadataDocMapModel = types.map(DocumentMetadataModel);