Allow downloading tasks of teams you are not in #8155
Conversation
|
Warning Rate limit exceeded@fm3 has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 11 minutes and 10 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. WalkthroughThe pull request introduces several user-facing enhancements and bug fixes to the WEBKNOSSOS application. Key updates include new features for metadata in annotations, improved search capabilities, and enhanced loading speeds for precomputed meshes. It also modifies error handling in the Changes
Assessment against linked issues
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Outside diff range and nitpick comments (1)
app/controllers/AnnotationIOController.scala (1)
459-461: Consider separating access control from task retrieval.
The current implementation mixes task retrieval with access control concerns. To better align with the PR objective of allowing downloads from other teams, consider:
- Separating the task retrieval logic from access control
- Adding explicit team access validation
- Handling different error cases separately (task not found vs. no team access)
Consider refactoring the task retrieval and access control:
+ // First, try to find the task
+ task <- Fox.runOptional(annotation._task)(taskDAO.findOne(_)(GlobalAccessContext)) ?~> "task.notFound"
+ // Then, verify team access separately
+ _ <- validateTaskAccess(task, issuingUser) ?~> "team.access.denied"
- taskOpt <- Fox.runOptional(annotation._task)(taskDAO.findOne(_)(GlobalAccessContext)) ?~> "team.notFound"This separation would make the code more maintainable and the error handling more precise.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (3)
- CHANGELOG.unreleased.md (1 hunks)
- app/controllers/AnnotationIOController.scala (1 hunks)
- conf/messages (1 hunks)
🧰 Additional context used
🪛 LanguageTool
CHANGELOG.unreleased.md
[typographical] ~38-~38: Usually, there’s no comma before “when”.
Context: ...task annotations of teams you are not in, when accessing directly via URI. [#8155](htt...
(IF_NO_COMMA)
🔇 Additional comments (1)
conf/messages (1)
26-26: LGTM! Error message update aligns with PR objectives.
The updated error message "Team could not be found or accessed" better reflects the actual scenarios where this error might occur, particularly in the context of allowing users to download tasks from teams they're not members of. This change improves clarity for users by explicitly indicating that the issue could be either due to the team not existing or due to access restrictions.
…s/webknossos into download-task-of-other-team
You can not list tasks of teams you are not in, but you can view individual task annotations if you know the ID/URL. This is consistent with our permission model. (Default task visibility “Internal”)
Now if you wanted to download them, this worked in the skeleton-only case but failed in the volume case. This PR unifies the behavior by allowing this also in the volume case.
URL of deployed dev instance (used for testing):
Steps to test:
Failed to download annotationIssues:
Summary by CodeRabbit
New Features
Bug Fixes
Improvements