ECHO-263 Make public images or videos visible#172
Conversation
…ures - Added a new "Public" folder in folders.json for better organization. - Introduced permissions for the "directus_files" collection, allowing read access based on folder hierarchy. - Updated GraphQL specifications to include new queries and mutations for aspects and project report notification participants. - Modified OpenAPI specifications to support new endpoints for managing aspect items. - Improved system subscriptions to include aspect and project report notification participants events.
WalkthroughThis change introduces a "Public" folder entry in the folders collection and adds a granular read permission for files in folders named "Public" (including up to two ancestor levels). The new permission specifies accessible fields and applies a default public policy, with no changes to exported or public entities. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Directus
participant Permissions Engine
participant Folders
User->>Directus: Request to read file metadata
Directus->>Permissions Engine: Check read permission for file
Permissions Engine->>Folders: Retrieve folder and ancestor names
Folders-->>Permissions Engine: Return folder hierarchy
Permissions Engine-->>Directus: Approve if "Public" in any folder name (up to 2 ancestors)
Directus-->>User: Return allowed file fields if permitted
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (3)
echo/directus/sync/specs/item.graphqlis excluded by!echo/directus/sync/specs/**echo/directus/sync/specs/openapi.jsonis excluded by!echo/directus/sync/specs/**echo/directus/sync/specs/system.graphqlis excluded by!echo/directus/sync/specs/**
📒 Files selected for processing (2)
echo/directus/sync/collections/folders.json(1 hunks)echo/directus/sync/collections/permissions.json(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: ci-check-server
🔇 Additional comments (2)
echo/directus/sync/collections/folders.json (1)
1-7:Details
✅ Verification successful
LGTM – “Public” folder sync entry looks good!
Root‐level folder added withparent: nulland a unique_syncId. Verify that thenamematches exactly what your permissions filter expects (case-sensitivity could matter).To double-check for duplicates or name mismatches, run:
🏁 Script executed:
#!/bin/bash # Search for folder named "Public" rg -n '"name":\s*"Public"' echo/directus/sync/collections/folders.jsonLength of output: 96
✅ Thrusters green: “Public” folder is unique
• echo/directus/sync/collections/folders.json:3 shows the sole “Public” entry
• Name matches exactly—case-sensitive filter satisfiedAll systems go—LGTM!
echo/directus/sync/collections/permissions.json (1)
78-110: LGTM – explicit field whitelist is clear.
You’ve whitelisted many attributes, includinguploaded_byand storage metadata. Confirm these fields align with your public policy and won’t leak PII or internal details.Please audit the field list against your privacy/GDPR requirements.
| { | ||
| "collection": "directus_files", | ||
| "action": "read", | ||
| "permissions": { | ||
| "_and": [ | ||
| { | ||
| "_or": [ | ||
| { | ||
| "folder": { | ||
| "name": { | ||
| "_contains": "Public" | ||
| } | ||
| } | ||
| }, | ||
| { | ||
| "folder": { | ||
| "parent": { | ||
| "name": { | ||
| "_contains": "Public" | ||
| } | ||
| } | ||
| } | ||
| }, | ||
| { | ||
| "folder": { | ||
| "parent": { | ||
| "parent": { | ||
| "name": { | ||
| "_contains": "Public" | ||
| } | ||
| } | ||
| } | ||
| } | ||
| } | ||
| ] | ||
| } | ||
| ] | ||
| }, |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
LGTM – permission filter nesting is consistent!
The new directus_files.read rule correctly checks the folder name at three levels of ancestry. You could simplify by removing the redundant outer _and, but consistency with other rules is also valuable. Ensure _contains: "Public" behaves case-insensitively in your Directus version so all “Public” folders (and their ancestors) are captured.
Example flattening:
- "permissions": { "_and": [ { "_or": [ … ] } ] }
+ "permissions": { "_or": [ … ] }Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In echo/directus/sync/collections/permissions.json between lines 38 and 75, the
permission filter for directus_files.read uses an outer _and with a single _or
condition inside, which is redundant. To simplify, remove the outer _and and
keep only the _or array with the three folder name checks. Also, verify that the
_contains operator is case-insensitive in your Directus version to ensure all
relevant "Public" folders are matched.
Enhance Directus sync configuration with new aspect and folder structures - Added a new "Public" folder in folders.json for better organization. - Introduced permissions for the "directus_files" collection, allowing read access based on folder hierarchy. - Updated GraphQL specifications to include new queries and mutations for aspects and project report notification participants. - Modified OpenAPI specifications to support new endpoints for managing aspect items. - Improved system subscriptions to include aspect and project report notification participants events.
Summary by CodeRabbit