fix(sharing): address code review: fix default role, auto-share logic, aria labels
Co-authored-by: christianlouis <361235+christianlouis@users.noreply.github.com> Agent-Logs-Url: https://github.com/christianlouis/DocuElevate/sessions/8091889d-4810-4794-b9d3-6b8f7f5257c4
This commit is contained in:
copilot-swe-agent[bot]
+3
-2
@@ -264,8 +264,9 @@ def create_comment(
|
||||
|
||||
if _settings.multi_user_enabled:
|
||||
for mentioned_user in mentions:
|
||||
# Skip the commenter themselves and the file owner
|
||||
if mentioned_user in (owner_id, file_record.owner_id):
|
||||
# Skip the file owner (already has full access) and the commenter
|
||||
# themselves (they already have access to be posting a comment).
|
||||
if mentioned_user in {file_record.owner_id, owner_id}:
|
||||
continue
|
||||
existing_share = (
|
||||
db.query(FileShare)
|
||||
|
||||
+5
-1
@@ -503,7 +503,11 @@ def file_annotations_page(request: Request, file_id: int, db: Session = Depends(
|
||||
current_owner_id = get_current_owner_id(request)
|
||||
user_session = request.session.get("user")
|
||||
is_admin = isinstance(user_session, dict) and bool(user_session.get("is_admin"))
|
||||
current_user_role = "owner" if is_admin else (get_file_role(file_record, current_owner_id, db) or "viewer")
|
||||
if is_admin:
|
||||
current_user_role: str | None = "owner"
|
||||
else:
|
||||
current_user_role = get_file_role(file_record, current_owner_id, db)
|
||||
# None means no access — the template will not show owner-only UI
|
||||
|
||||
return templates.TemplateResponse(
|
||||
"file_annotations.html",
|
||||
|
||||
+138
@@ -3108,3 +3108,141 @@ Returns all non-blocked user profiles for the @mention autocomplete.
|
||||
{ "user_id": "bob", "display_name": "Bob Baker" }
|
||||
]
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## File Sharing & Permissions
|
||||
|
||||
DocuElevate supports per-user document sharing with role-based access control.
|
||||
|
||||
### Roles
|
||||
|
||||
| Role | View | Comment / Annotate | Edit metadata | Delete | Share |
|
||||
|----------|------|--------------------|---------------|--------|-------|
|
||||
| `owner` | ✓ | ✓ | ✓ | ✓ | ✓ |
|
||||
| `editor` | ✓ | ✓ | ✓ | ✗ | ✗ |
|
||||
| `viewer` | ✓ | ✓ | ✗ | ✗ | ✗ |
|
||||
|
||||
- Only the **file owner** can share a document, change roles, or delete the document.
|
||||
- When a user is **@mentioned** in a comment they are automatically granted `viewer` access to the document (multi-user mode only).
|
||||
|
||||
---
|
||||
|
||||
### List Shares
|
||||
|
||||
**GET** `/api/files/{file_id}/shares`
|
||||
|
||||
Returns all active shares for a document. Only the file owner (or an admin) may call this endpoint.
|
||||
|
||||
**Response (200):**
|
||||
```json
|
||||
[
|
||||
{
|
||||
"id": 1,
|
||||
"file_id": 42,
|
||||
"owner_id": "alice",
|
||||
"shared_with_user_id": "bob",
|
||||
"role": "viewer",
|
||||
"created_at": "2026-03-22T10:00:00+00:00",
|
||||
"updated_at": "2026-03-22T10:00:00+00:00"
|
||||
}
|
||||
]
|
||||
```
|
||||
|
||||
**Error Responses:**
|
||||
- `403`: Not the file owner
|
||||
- `404`: File not found
|
||||
|
||||
---
|
||||
|
||||
### Create Share
|
||||
|
||||
**POST** `/api/files/{file_id}/shares`
|
||||
|
||||
Share a document with another user. Only the file owner may call this endpoint. If the user already has a share, their role is updated.
|
||||
|
||||
**Request:**
|
||||
```json
|
||||
{
|
||||
"shared_with_user_id": "bob",
|
||||
"role": "viewer"
|
||||
}
|
||||
```
|
||||
|
||||
`role` must be `"viewer"` (default) or `"editor"`.
|
||||
|
||||
**Response (201):**
|
||||
```json
|
||||
{
|
||||
"id": 1,
|
||||
"file_id": 42,
|
||||
"owner_id": "alice",
|
||||
"shared_with_user_id": "bob",
|
||||
"role": "viewer",
|
||||
"created_at": "2026-03-22T10:00:00+00:00",
|
||||
"updated_at": "2026-03-22T10:00:00+00:00"
|
||||
}
|
||||
```
|
||||
|
||||
**Error Responses:**
|
||||
- `403`: Not the file owner
|
||||
- `422`: Invalid role, empty user ID, or sharing with self
|
||||
|
||||
---
|
||||
|
||||
### Update Share Role
|
||||
|
||||
**PUT** `/api/files/{file_id}/shares/{share_id}`
|
||||
|
||||
Change the role of an existing share. Only the file owner may call this endpoint.
|
||||
|
||||
**Request:**
|
||||
```json
|
||||
{
|
||||
"role": "editor"
|
||||
}
|
||||
```
|
||||
|
||||
**Response (200):** Updated share object.
|
||||
|
||||
**Error Responses:**
|
||||
- `403`: Not the file owner
|
||||
- `404`: Share not found
|
||||
- `422`: Invalid role
|
||||
|
||||
---
|
||||
|
||||
### Revoke Share
|
||||
|
||||
**DELETE** `/api/files/{file_id}/shares/{share_id}`
|
||||
|
||||
Remove a user's access to a document. Only the file owner may revoke shares.
|
||||
|
||||
**Response (200):**
|
||||
```json
|
||||
{ "status": "success", "message": "Share revoked successfully" }
|
||||
```
|
||||
|
||||
**Error Responses:**
|
||||
- `403`: Not the file owner
|
||||
- `404`: Share not found
|
||||
|
||||
---
|
||||
|
||||
### List Shared With
|
||||
|
||||
**GET** `/api/files/{file_id}/shared-with`
|
||||
|
||||
Returns who a document is shared with. Accessible to any user with at least `viewer` access (owner, editors, and viewers can all call this).
|
||||
|
||||
**Response (200):**
|
||||
```json
|
||||
[
|
||||
{
|
||||
"share_id": 1,
|
||||
"user_id": "bob",
|
||||
"display_name": "Bob Baker",
|
||||
"role": "viewer"
|
||||
}
|
||||
]
|
||||
```
|
||||
|
||||
@@ -103,7 +103,9 @@
|
||||
var roleLabel = s.role === 'editor' ? _t('role_editor') : _t('role_viewer');
|
||||
return (
|
||||
'<div style="display:flex;align-items:center;justify-content:space-between;gap:0.5rem;padding:0.5rem 0;border-bottom:1px solid #f1f5f9;">' +
|
||||
'<span style="font-size:0.875rem;color:#334155;overflow:hidden;text-overflow:ellipsis;white-space:nowrap;flex:1;" title="' + _esc(s.user_id) + '">' +
|
||||
'<span style="font-size:0.875rem;color:#334155;overflow:hidden;text-overflow:ellipsis;white-space:nowrap;flex:1;" ' +
|
||||
'aria-label="' + _esc(_t('user_id_label') + ': ' + (s.display_name || s.user_id)) + '" ' +
|
||||
'title="' + _esc(s.user_id) + '">' +
|
||||
_esc(s.display_name || s.user_id) +
|
||||
'</span>' +
|
||||
'<select' +
|
||||
|
||||
@@ -686,7 +686,6 @@
|
||||
id="share-user-input"
|
||||
placeholder="{{ _('sharing.user_id_placeholder') }}"
|
||||
style="width:100%;padding:0.5rem 0.75rem;border:1px solid #cbd5e1;border-radius:0.375rem;font-size:0.875rem;"
|
||||
aria-label="{{ _('sharing.user_id_label') }}"
|
||||
/>
|
||||
</div>
|
||||
<div style="min-width:120px;">
|
||||
@@ -696,7 +695,6 @@
|
||||
<select
|
||||
id="share-role-input"
|
||||
style="width:100%;padding:0.5rem 0.75rem;border:1px solid #cbd5e1;border-radius:0.375rem;font-size:0.875rem;background:#fff;"
|
||||
aria-label="{{ _('sharing.role_label') }}"
|
||||
>
|
||||
<option value="viewer">{{ _("sharing.role_viewer") }}</option>
|
||||
<option value="editor">{{ _("sharing.role_editor") }}</option>
|
||||
|
||||
Reference in New Issue
Block a user