Add per-project hierarchical collaborative notes#855
Open
BlaiseOfGlory wants to merge 19 commits intoGhostManager:masterfrom
Open
Add per-project hierarchical collaborative notes#855BlaiseOfGlory wants to merge 19 commits intoGhostManager:masterfrom
BlaiseOfGlory wants to merge 19 commits intoGhostManager:masterfrom
Conversation
Reimplements the collab notes feature addressing all review feedback from PR GhostManager#805. Key changes from the original submission: Backend: - ProjectCollabNote and ProjectCollabNoteField models with proper Django conventions (default=timezone.now, no raw SQL migrations) - Image upload uses Django Form with ImageField for validation - Merged upload views into single endpoint with optional field_pk - ZIP export uses StreamingHttpResponse instead of reading to memory - Exception handlers use logger.exception() throughout - Model imports at module level (no circular import workarounds) - CheckEditPermissions uses available_models dict (no special case) Frontend: - NoteEditor derives field state from Yjs doc via observeDeep (single source of truth, no manual React state sync) - Tree structure stored in Yjs Y.Array for real-time sync across clients (replaces stateless message broadcast + GraphQL re-fetch) - DeleteConfirmModal uses top-level const map (not closure) - Components use CSS classes instead of extensive inline styles - ClipboardEvent typed directly (no casting) - MIME checks use startsWith("image/") not indexOf Includes migrations, Hasura metadata, collab server handlers, factories, and 17 test cases.
- Reduce return statements in ajax_upload_note_field_image by extracting _update_existing_image_field and _create_new_image_field - Narrow except clause from Exception to (OSError, IntegrityError) - Use dict .items() iteration in export_collab_notes_zip
Missing from initial commit — required by NoteTreeView and NoteEditor field reordering.
- Enable allow_aggregations in Hasura select permissions for both collab note tables (fixes _aggregate query 404) - Use ReactModal style prop for overlay positioning instead of className (fixes modals rendering at page bottom) - Calculate drop position from cursor Y relative to item rect (shows before/after/inside indicators during drag)
- Add RunSQL migration for DB-level NOW() defaults on timestamp columns (Django 4.2 default= is ORM-only, Hasura bypasses it) - Add Hasura column presets (created_at/updated_at = now()) for insert permissions on both collab note tables - Include auto-generated BigAutoField migration from Django - Fix drag indicator position calculation to use active element's translated rect center Y for accurate before/after/inside zones
- Add RunSQL migration for NOW() defaults on timestamp columns (Django 4.2 default= is ORM-only; Hasura/collab server need DB defaults) - Add Hasura column presets for created_at/updated_at on insert - Include Django auto-generated BigAutoField migration - Simplify DnD drop position to use delta.y direction - Make drop indicator lines 3px with glow, folders get outline - Add console.debug for drag-over to diagnose indicator visibility
Switch from closestCenter to pointerWithin collision detection — closestCenter fails with nested DOM structures because sortable items are inside each other. Also bind handleDragOver to onDragMove which fires on every pointer movement, not just when the over target changes.
The Vite build extracts CSS into separate files but the template only loaded the JS bundle. Add stylesheet links for both the component CSS and the shared collab CSS.
- Update Yjs tree with both parentId AND position on drop (was only updating parentId, causing items to land in the right folder but at their old position) - Add inline border styles for drop indicators as primary approach (CSS pseudo-elements weren't visible)
buildTree was pushing children in Y.Array insertion order, ignoring the position field. After updateNode changed a node's position value, the array order didn't change so items appeared in the wrong place. Now sorts roots and children by position then title at every level.
Ghostwriter doesn't expose /media/ directly — all file serving goes through Django views with permission checks. Add serve_note_field_image view that checks user_can_view before returning the file. Update upload responses and collab server handler to use the new URL pattern instead of raw /media/ paths.
Items couldn't be dragged to the very first or last position because there was no sortable target above the first item or below the last. Add sentinel droppable zones that highlight on hover and handle placement at position -1000/+1000 relative to the first/last root node.
Move sentinel droppables outside SortableContext so they don't conflict with sortable collision detection. Increase sentinel height to 24px for easier targeting. Remove console.warn debug messages from drag handler.
Folders now split into three drop zones based on pointer Y relative to the item rect: top 25% = before, middle 50% = inside, bottom 25% = after. Notes split at 50% for before/after. This allows dragging items between adjacent folders instead of always dropping inside.
- Lock @dnd-kit dependency versions from npm install - Add scripts/seed_test_data.py for populating dev environment with test users, clients, projects, and collab notes
Author
|
Addressing the CodeFactor issues now. |
Python: - Use 'raise Http404 from exc' for explicit exception chaining - Move markdownify import to module level - Rewrite seed script: remove unused imports, use dict literals, extract functions to fix scope and redefinition warnings TypeScript: - Extract computeDropPosition() and sentinelPosition() from handleDragOver/handleDragEnd to reduce cyclomatic complexity - Extract TreeItemIcon, TreeItemChevron, TreeItemActions from TreeItem to reduce component complexity
Resolve conflicts in test_models.py and test_views.py (added imports from both branches). Renumber collab notes migrations 0061-0064 to 0062-0065 to follow upstream's new 0061_projectrole_position_ordering.
Author
|
Fixed merge conflicts. The remaining CodeFactor issues are related to a test script that could be stripped out, it's just for adding seed data to speed up testing. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds a full collaborative notes system to projects — hierarchical folders and notes with real-time multi-user editing, drag-and-drop reorganization, multi-field notes (rich text + images), and ZIP export.
This is a reimplementation of #805 addressing all review feedback from @ColonelThirtyTwo.
What's included
ProjectCollabNote(hierarchical tree nodes) andProjectCollabNoteField(per-note rich text and image fields)Project.collab_note, and DB-level timestamp defaults for Hasura compatibilityChanges from PR #805
All 20 review comments addressed:
CheckEditPermissionsavailable_modelsdictdefault=timezone.now+ separate RunSQL for DB-level defaults (required for Hasura on Django 4.2)logger.errorwith manual formattinglogger.exception()throughoutImageFieldfield_pkparameterStreamingHttpResponsewithBytesIONamedTemporaryFilecleanupdebounce: 2000onConnectnot throwing on missing docdocker-rebuild-diagnostics.tsgetTypeLabel()closureTYPE_LABELSconst mapstylepropClipboardEventcastindexOf("image")for MIMEstartsWith("image/")observeDeepas single source of truthY.Arraywith real-time syncmarkdownify(HTML→MD for export, opposite direction frommarkdown)local.ymllocal.ymlchangesCodeFactor issues from #805 also addressed (return count, broad exceptions, dict iteration, unused imports, complexity).
Additional improvements
/media/directly)Test plan
pytest ghostwriter/rolodex/tests/for model and view tests