Skip to content

Long running branch: Group documents#2720

Merged
scytacki merged 62 commits intomasterfrom
group-documents
Mar 6, 2026
Merged

Long running branch: Group documents#2720
scytacki merged 62 commits intomasterfrom
group-documents

Conversation

@scytacki
Copy link
Copy Markdown
Member

@scytacki scytacki commented Jan 19, 2026

This work is ready now, it has many changes which have been reviewed in individual PRs.

scytacki and others added 17 commits January 6, 2026 11:21
- group documents just use the firestore metadata instead of having firebase metadata too. This is a pattern that all documents should change to eventually.
- group document content is stored in a different place firebase because they don't have a user. We could use the fake uid that is created for them so they could be stored in the same place in firebase that the other documents are stored, but this seems like it would lead to more problems.
This also enables reloading the page when a group document is open.
replace "group" with GroupDocument
Also:
- save the firebase metadata for the document so it is consistent with other CLUE documents. This removes some complexity around the createdAt timestamp.
- remove some console logs.
- revert the useSyncMst*ToFirebase to requiring a path as a string which makes more sense. However keep them checking for a non-empty path to prevent weird behavior
- have createDocument return the firestore metadata that it creates, so it can be used by other code
- add an official VisibilityType to unify "public" | "private" typing.
- add a type guard for DocumentType, this makes it safer when loading in documents from Firestore and other places.
- unify document title component used in sort-work-document-area and document-view
- note that use-document-caption has overlapping features with the document-title
- update db.ts so the document.self is not required to get the documentKey, some documents do not have this set and it isn't really needed
- add GroupDocument as a sortable type
- update getDocumentDisplayTitle to handle group documents.
- move some createDocMapBy* into the DocumentGroup to simplify the code
- update the byGroup and byName groupings so they handle group documents
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@codecov
Copy link
Copy Markdown

codecov Bot commented Jan 19, 2026

Codecov Report

❌ Patch coverage is 60.84337% with 195 lines in your changes missing coverage. Please review.
✅ Project coverage is 85.91%. Comparing base (ea92ae1) to head (1f6fe32).
⚠️ Report is 238 commits behind head on master.

Files with missing lines Patch % Lines
...ls/history/firestore-history-manager-concurrent.ts 7.27% 102 Missing ⚠️
src/lib/db.ts 53.08% 38 Missing ⚠️
src/models/history/firestore-history-manager.ts 86.29% 17 Missing ⚠️
src/components/document/document.tsx 61.11% 7 Missing ⚠️
src/components/document/document-workspace.tsx 45.45% 6 Missing ⚠️
src/components/document/history-view-panel.tsx 70.00% 6 Missing ⚠️
src/models/document/document-metadata-model.ts 14.28% 6 Missing ⚠️
src/models/stores/documents.ts 58.33% 5 Missing ⚠️
src/components/document/document-title.tsx 90.47% 2 Missing ⚠️
src/models/history/tree-manager.ts 75.00% 2 Missing ⚠️
... and 3 more
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #2720      +/-   ##
==========================================
- Coverage   86.20%   85.91%   -0.29%     
==========================================
  Files         849      851       +2     
  Lines       46449    46727     +278     
  Branches    12072    12133      +61     
==========================================
+ Hits        40040    40145     +105     
- Misses       6002     6569     +567     
+ Partials      407       13     -394     
Flag Coverage Δ
cypress ?
cypress-regression 75.72% <57.39%> (+0.77%) ⬆️
cypress-smoke 42.99% <33.33%> (-0.35%) ⬇️
jest 50.43% <33.93%> (-0.06%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@cypress
Copy link
Copy Markdown

cypress Bot commented Jan 19, 2026

collaborative-learning    Run #18013

Run Properties:  status check passed Passed #18013  •  git commit 1f6fe322aa: Merge pull request #2788 from concord-consortium/CLUE-452-group-doc-pre-merge
Project collaborative-learning
Branch Review group-documents
Run status status check passed Passed #18013
Run duration 08m 48s
Commit git commit 1f6fe322aa: Merge pull request #2788 from concord-consortium/CLUE-452-group-doc-pre-merge
Committer Scott Cytacki
View all properties for this run ↗︎

Test results
Tests that failed  Failures 0
Tests that were flaky  Flaky 2
Tests that did not run due to a developer annotating a test with .skip  Pending 5
Tests that did not run due to a failure in a mocha hook  Skipped 0
Tests that passed  Passing 176
View all changes introduced in this branch ↗︎

scytacki and others added 8 commits January 19, 2026 08:29
Use a Firestore transaction and the metadata to safely track the
last history entry when multiple users are adding history entries
at the same time.
This also add a log statement to make it a little easier to track down
where in the test it is failing.
scytacki and others added 7 commits February 6, 2026 14:49
most complex change is better error handling in uploadQueuedHistoryEntries. The approach doesn't fully fix the problem, but at least it prevents a messed up uploadInProgress state.
resolved conflicts in:
- sort-document-utils.ts
- document-group.ts
- app-config-model.test.ts
- history-view-panel.tsx
- Canvas.js
- single_student_canvas_test.js
Resolved conflicts in:
- document-file-menu.tsx (combined useMemo from master with group doc option)
- document-file-menu.test.tsx (combined both test suites - basic menu ops and group doc visibility)
- document-utils.ts (kept GroupDocument import, removed duplicate imports)
- document-group.ts (kept inline byGroup with GroupDocument handling, added upperWords)

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@scytacki scytacki marked this pull request as ready for review March 6, 2026 03:25
scytacki and others added 3 commits March 6, 2026 08:45
Update document-types.md to reflect that group documents are implemented
(not "in the process of adding"), add metadata details, feature flag
reference, and link to group-documents.md. Remove fragile line numbers
from group-documents.md implementation TODOs.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…bled

The guaranteeInitialDocuments method was querying Firestore on every
returning-user load to check if their primary document is a group
document, even when group documents are disabled. This added an
unnecessary query and, if it failed, could prevent learning log and
planning document initialization.

Now the query only runs when groupDocumentsEnabled is true, and is
wrapped in try/catch so failures don't block other initialization.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…re-merge

CLUE-452 Pre-merge cleanup for group documents
@scytacki scytacki merged commit 3d736bf into master Mar 6, 2026
24 of 27 checks passed
@scytacki scytacki deleted the group-documents branch March 6, 2026 18:04
@scytacki scytacki added the long lived branch A PR for a long lived branch that other PRs are being merged into instead of merging into master label Apr 14, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

long lived branch A PR for a long lived branch that other PRs are being merged into instead of merging into master run regression

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants