Merge pull request #2415 from concord-consortium/188328447-unify-doc-…

…titles unify more the titles displayed for documents
concord-consortium · Sep 30, 2024 · d23f00b · d23f00b
2 parents 3f37fc1 + f4a95b5
commit d23f00b
Show file tree

Hide file tree

Showing 16 changed files with 347 additions and 119 deletions.
diff --git 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<string, string>;
   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
@@ -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)}
       <div ref={docListContainerRef} className="doc-group-list simple" data-testid="doc-group-list">
-        {documentGroup.documents?.map((doc: any) => {
+        {documentGroup.documents?.map((doc) => {
           return (
             <SimpleDocumentItem
               key={doc.key}
               document={doc}
-              investigationOrdinal={doc.investigation}
-              problemOrdinal={doc.problem}
               onSelectDocument={onSelectDocument}
             />
           );

diff --git 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<IProps, IState> {
               { !hideButtons && <EditButton onClick={this.handleDocumentRename} /> }
             </div>
           : <div className="title" data-test="personal-doc-title">
-              <TitleInfo docTitle={`${document.getDisplayTitle(appConfig)}`} onClick={this.handleDocumentRename} />
+              <TitleInfo
+                docTitle={`${getDocumentTitleWithTimestamp(document, appConfig)}`}
+                onClick={this.handleDocumentRename} />
               { !hideButtons && <EditButton onClick={this.handleDocumentRename} /> }
             </div>
         }

diff --git 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<IProps> = 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<IProps> = 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
@@ -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<IProps> = 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
@@ -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
@@ -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(
     <div
       className={isPrivate ? "simple-document-item private" : "simple-document-item"}
       data-test="simple-document-item"
-      title={title}
+      title={titleWithUser}
       onClick={!isPrivate ? handleClick : undefined}
     >
     </div>

diff --git 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
@@ -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,24 +440,24 @@ 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<ICommentableDocumentParams>("validateCommentableDocument_v1");
       // FIXME-HISTORY: rename this function to validateFirestoreDocumentMetadata_v1
       validateCommentableDocument({context: userContext, document: firestoreMetadata});
     }
   }
 
+  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,14 +844,16 @@ export class DB {
         return allUsers;
       }, {} as DBGroupUserConnections);
 
+    const problemInfo = this.currentProblemInfo;
     return this.openDocument({
       documentKey,
       type: "publication",
       userId,
       groupId,
       visibility: "public",
       groupUserConnections: groupUserConnectionsMap,
-      pubVersion
+      pubVersion,
+      ...problemInfo
     });
   }
 

diff --git 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;
 }