Skip to content

feat: implement File Lock & Edit Access Request System#145

Open
Shivangi1515 wants to merge 7 commits into
suresh1319:masterfrom
Shivangi1515:feat/lock-unlock-file
Open

feat: implement File Lock & Edit Access Request System#145
Shivangi1515 wants to merge 7 commits into
suresh1319:masterfrom
Shivangi1515:feat/lock-unlock-file

Conversation

@Shivangi1515
Copy link
Copy Markdown

@Shivangi1515 Shivangi1515 commented May 28, 2026

Closes #137

Description

What

Implemented a new ** File Lock & Edit Access Request System** for the CollabCE real-time collaborative code editor. This feature enables users to lock individual files they are actively editing, preventing other users from overwriting or making conflicting edits, while providing a real-time workflow to request edit access.

Why

Currently, all collaborators can edit any file concurrently, which often leads to:

  • Accidental overwrites and race conditions.
  • Interrupted debugging/refactoring sessions when multiple users edit the same critical section of code.
  • Lack of coordination on who is working on what file.

This feature ensures editing safety and improves team collaboration by providing a structured, real-time lock-and-request flow.

Demo

Screen.Recording.2026-05-28.221746.mp4

How

  1. Socket Actions (src/Actions.js): Defined event names for lock/unlock transitions, edit access requests, approval, rejection, active editor tracking, and state syncs.
  2. Backend Sync Layer (server.js):
    • Managed fileLocks and activeEditors maps inside the room state.
    • Handled events to process locks, unlock files, route requests to owners, and save approved users on the lock.
    • Implemented cleanup in the disconnect handler to auto-unlock files owned by the disconnecting client and clear active editor flags.
  3. Core Interface State (src/pages/EditorPage.js):
    • Listened for real-time status updates and routed toasts.
    • Implemented real-time interactive notifications for file owners (with Approve/Reject buttons) and access results for requesters.
    • Computed dynamic file-level write permissions (hasFileEditPermission) to disable editing on locked files.
  4. UI Elements:
    • FileExplorer.js & FileTabs.js: Rendered closed locks, owner tooltips, and highlights (fs-node-locked, fs-node-locked-by-me) for locked files.
    • Editor.js: Added "Lock/Unlock" action buttons, lock labels, and a Currently editing: [Username] banner to track active editors.
    • RequestAccessModal.js: Added dynamic props to reuse it for file access requests.
    • App.css: Added styling for glowing rows, colored locks, request buttons, and warning overlays.

Pull Request Checklist

  • npm run dev starts successfully with your changes
  • Code is properly formatted
  • There are no console errors or warnings in the browser
  • PR description explains what, why, and how
  • If applicable, screenshots or a video are included to demonstrate UI changes

@Shivangi1515
Copy link
Copy Markdown
Author

Hi @suresh1319

Kindly review the PR for the task #137

Comment thread src/components/FileExplorer.js Outdated
Comment thread src/pages/EditorPage.js
@entelligence-ai-pr-reviews
Copy link
Copy Markdown
Contributor

File: server.js

MAJOR CORRECTNESS FS_DELETE_NODE doesn't purge fileLocks for deleted files

server.js FS_DELETE_NODE (line 217) cleans fileContents for each deleted node but never removes corresponding fileLocks entries. Stale lock objects for deleted files persist in roomState and are broadcast in every subsequent LOCK_STATUS_UPDATE, growing unboundedly.

toDelete.forEach(id => { delete fs[id]; delete roomState[roomId].fileContents[id]; delete roomState[roomId].fileLocks?.[id]; delete roomState[roomId].activeEditors?.[id]; });
Prompt to fix with AI

Copy this prompt into your AI coding assistant to fix this issue.

In server.js inside the FS_DELETE_NODE handler, extend the toDelete.forEach loop to also delete `roomState[roomId].fileLocks[id]` and `roomState[roomId].activeEditors[id]` for each deleted node id, mirroring the existing fileContents cleanup pattern.

@entelligence-ai-pr-reviews
Copy link
Copy Markdown
Contributor


Confidence Score: 3/5 - Review Recommended

Review recommended — this PR implements a File Lock & Edit Access Request System with reasonable structure, but contains a correctness issue in server.js where FS_DELETE_NODE cleans fileContents for deleted nodes yet never purges corresponding entries from fileLocks, meaning deleted files can leave stale lock state that persists server-side. Additionally, handleRequestEditAccess in EditorPage.js fires the REQUEST_EDIT_ACCESS socket event immediately without attaching the message or reason from RequestAccessModal, rendering the new modal props effectively dead code. The unused Unlock import in FileExplorer.js is minor but the logic gaps in lock lifecycle management and the broken request flow are substantive enough to warrant fixes before merging.

Key Findings:

  • In server.js, the FS_DELETE_NODE handler clears fileContents for deleted nodes but never removes matching entries from fileLocks, creating a ghost-lock bug where a re-created file with the same path could appear locked by a user who locked the now-deleted version.
  • In EditorPage.js, handleRequestEditAccess emits REQUEST_EDIT_ACCESS with no message payload, while the RequestAccessModal was wired with props for capturing a user-provided reason — the modal's input is silently discarded, making the UX feature incomplete and potentially confusing to users and reviewers alike.
  • The unused Unlock import in FileExplorer.js is a low-impact nit but signals that the lock indicator logic may have been simplified mid-implementation without full cleanup, worth tidying for maintainability.
Files requiring special attention
  • server.js
  • src/pages/EditorPage.js
  • src/components/FileExplorer.js

@entelligence-ai-pr-reviews
Copy link
Copy Markdown
Contributor

entelligence-ai-pr-reviews Bot commented May 28, 2026

EntelligenceAI PR Summary

Fixes a structural/formatting issue in the ACTIONS constants object within src/Actions.js by removing a misplaced constant entry.

  • Removed PERMISSION_DENIED: 'permission_denied' that was incorrectly indented and positioned inside the FS_UPLOAD_BATCH comment block
  • Restores proper structural organization of the ACTIONS constants object

Confidence Score: 5/5 - Safe to Merge

Safe to merge — this PR makes a straightforward structural cleanup in src/Actions.js by removing a misplaced PERMISSION_DENIED: 'permission_denied' constant that was incorrectly positioned inside the FS_UPLOAD_BATCH comment block. The change is purely cosmetic/organizational in nature, restoring proper formatting and structure to the ACTIONS constants object without altering any runtime behavior or logic. No review comments were generated and all heuristic checks passed cleanly.

Key Findings:

  • The removal of the misplaced PERMISSION_DENIED: 'permission_denied' entry from within the FS_UPLOAD_BATCH comment block is a pure formatting fix — assuming this constant was never legitimately used elsewhere under this key, the change has zero runtime impact.
  • No logic, validation, security, or performance concerns were identified by any reviewer or heuristic analysis across the single changed file src/Actions.js.
  • The PR's scope is minimal and well-contained — a single-file change that corrects structural organization of a constants object, making it safe for merge without further review.
Files requiring special attention
  • src/Actions.js

@Shivangi1515
Copy link
Copy Markdown
Author

Hi @suresh1319

Kindly review the PR

@entelligence-ai-pr-reviews
Copy link
Copy Markdown
Contributor

File: server.js

MAJOR CORRECTNESS Leave activity broadcast reaches disconnecting socket, causing self-notification on drop

During the 'disconnecting' event the socket is still a member of its rooms. logAndBroadcastActivity calls io.to(roomId).emit, which delivers the leave event back to the disconnecting socket. EditorPage.js:371-373 then increments unreadActivityCount and appends the self-leave to the activity feed on the still-mounted component (connection-drop case).

      // Log the leave activity
      const userName = userSocketMap[socket.id];
      if (userName && roomState[roomId]) {
        socket.to(roomId).emit(ACTIONS.ACTIVITY_RECEIVE, (() => {
          const room = roomState[roomId];
          const activity = {
            id: uuid(),
            userId: socket.id,
            userName,
            type: 'leave',
            message: `${userName} left the room.`,
            timestamp: new Date().toISOString(),
            meta: {}
          };
          if (!room.activities) room.activities = [];
          room.activities.push(activity);
          if (room.activities.length > 50) room.activities.shift();
          return activity;
        })());
      }
Prompt to fix with AI

Copy this prompt into your AI coding assistant to fix this issue.

In server.js inside the 'disconnecting' handler, replace the logAndBroadcastActivity call for the 'leave' event with a socket.to(roomId).emit (which excludes the emitting socket) instead of io.to(roomId).emit. You can either modify logAndBroadcastActivity to accept an optional 'excludeSelf' boolean, or inline the activity construction for the leave case using socket.to(roomId).emit directly, while still pushing the activity into room.activities for history purposes.

@suresh1319
Copy link
Copy Markdown
Owner

suresh1319 commented Jun 2, 2026

@Shivangi1515 please fix this File: server.js

MAJOR CORRECTNESS Leave activity broadcast reaches disconnecting socket, causing self-notification on drop

During the 'disconnecting' event the socket is still a member of its rooms. logAndBroadcastActivity calls io.to(roomId).emit, which delivers the leave event back to the disconnecting socket. EditorPage.js:371-373 then increments unreadActivityCount and appends the self-leave to the activity feed on the still-mounted component (connection-drop case).

  // Log the leave activity
  const userName = userSocketMap[socket.id];
  if (userName && roomState[roomId]) {
    socket.to(roomId).emit(ACTIONS.ACTIVITY_RECEIVE, (() => {
      const room = roomState[roomId];
      const activity = {
        id: uuid(),
        userId: socket.id,
        userName,
        type: 'leave',
        message: `${userName} left the room.`,
        timestamp: new Date().toISOString(),
        meta: {}
      };
      if (!room.activities) room.activities = [];
      room.activities.push(activity);
      if (room.activities.length > 50) room.activities.shift();
      return activity;
    })());
  }

@entelligence-ai-pr-reviews
Copy link
Copy Markdown
Contributor

File: src/Actions.js

NIT CORRECTNESS PERMISSION_DENIED key defined twice in ACTIONS object

src/Actions.js lines 21 and 25 both define PERMISSION_DENIED: 'permission_denied'. In JS the last declaration silently wins; here both values are identical so there is no runtime break today, but the duplicate creates confusion and masks any future divergence if one copy is changed.

    // Server → admin-only: delivers the secure ownership token for reconnect validation
    ADMIN_TOKEN: 'admin_token',
    // Permission denial feedback to client
    PERMISSION_DENIED: 'permission_denied',
Prompt to fix with AI

Copy this prompt into your AI coding assistant to fix this issue.

Remove the first duplicate `PERMISSION_DENIED: 'permission_denied',` at line 21 of src/Actions.js (the one that appears before the ADMIN_TOKEN entry), keeping only the one at line 25 that has the explanatory comment above it.

@Shivangi1515
Copy link
Copy Markdown
Author

Hi @suresh1319
pls review the PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

feat: add file lock and edit access request workflow for collaborative rooms

2 participants