Skip to content

Conversation

fm3
Copy link
Member

@fm3 fm3 commented Sep 10, 2025

Steps to test:

  • On multi-orga setup, run yarn enable-jobs and set a dataset to public
  • When viewing the public dataset of the other orga, users should be allowed to export tiff and render animation. This was previously rejected.
  • When viewing other orga’s private datasets via sharing token, it is still not possible to run those jobs (separately tracked in Unclear which dataset actions are allowed when viewing via sharing token #7360 )

Issues:


  • Added changelog entry (create a $PR_NUMBER.md file in unreleased_changes or use ./tools/create-changelog-entry.py)
  • Considered common edge cases

@fm3 fm3 self-assigned this Sep 10, 2025
@fm3 fm3 changed the title Allow export_tiff and render_animation jobs cross-orga for public dat… Allow export_tiff and render_animation jobs cross-orga for public datasets Sep 10, 2025
Copy link
Contributor

coderabbitai bot commented Sep 10, 2025

📝 Walkthrough

Walkthrough

Removed organization-ownership checks from specific job endpoints in JobController, adjusted corresponding localization messages, and added an unreleased changelog entry noting that export_tiff and render_animation are now allowed for public datasets of other organizations.

Changes

Cohort / File(s) Change summary
Authorization checks removal
app/controllers/JobController.scala
Removed per-request organization-ownership guards from runConvertToWkwJob, runExportTiffJob, and runRenderAnimationJob; retained dataset/org lookups, validations, and job submission flow.
Localization message updates
conf/messages
Removed org-scoped not-allowed messages for exportTiff and renderAnimation; added/repurposed org-scoped message for segmentIndexFile not-allowed.
Changelog entry
unreleased_changes/8911.md
Added note that export_tiff and render_animation are allowed for public datasets of other organizations.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

Suggested labels

bug

Suggested reviewers

  • frc roth
  • MichaelBuessemeyer

Pre-merge checks (4 passed, 1 warning)

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Out of Scope Changes Check ⚠️ Warning The pull request also removes the organization authorization guard for the convertToWkw job and introduces a segmentIndexFile error message, neither of which are part of issue #8910’s scope of enabling only export_tiff and render_animation for public datasets. Please revert the changes to runConvertToWkwJob authorization and the unrelated segmentIndexFile message updates or split them into a separate PR aligned with a corresponding issue.
✅ Passed checks (4 passed)
Check name Status Explanation
Title Check ✅ Passed The current title succinctly and accurately describes the primary change of allowing export_tiff and render_animation jobs across organizations for public datasets, making it clear to reviewers without extraneous detail.
Linked Issues Check ✅ Passed The changes remove the organization ownership checks for export_tiff and render_animation jobs, update the related error messages, and add a matching changelog entry, fully satisfying the main objective of issue #8910.
Description Check ✅ Passed The description outlines relevant testing steps and links to the issue being fixed, directly relating to the changes in cross-organization job permissions, so it matches the changeset.
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.

Poem

I nibbled the checks from the meadow of code,
Let public paths be happily trod.
TIFFs and reels now hop down the road,
No fences where the datasets nod.
Thump-thump—jobs run, tails applaud! 🐇✨

Tip

👮 Agentic pre-merge checks are now available in preview!

Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.

  • Built-in checks – Quickly apply ready-made checks to enforce title conventions, require pull request descriptions that follow templates, validate linked issues for compliance, and more.
  • Custom agentic checks – Define your own rules using CodeRabbit’s advanced agentic capabilities to enforce organization-specific policies and workflows. For example, you can instruct CodeRabbit’s agent to verify that API documentation is updated whenever API schema files are modified in a PR. Note: Upto 5 custom checks are currently allowed during the preview period. Pricing for this feature will be announced in a few weeks.

Please see the documentation for more information.

Example:

reviews:
  pre_merge_checks:
    custom_checks:
      - name: "Undocumented Breaking Changes"
        mode: "warning"
        instructions: |
          Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).

Please share your feedback with us on this Discord post.

✨ Finishing Touches
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch some-jobs-cross-orga-if-public

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@fm3 fm3 marked this pull request as ready for review September 10, 2025 13:30
@fm3 fm3 requested a review from normanrz September 10, 2025 13:30
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Nitpick comments (1)
unreleased_changes/8911.md (1)

1-2: Clarify scope and tighten phrasing; format job names

The sentence is slightly awkward and misses the “public-only, multi-orga” scope and token-access caveat. Also, format job names with backticks for readability.

 ### Changed
-- The webknossos-worker jobs export_tiff and render_animation are now allowed also for public datasets of other organizations.
+- Allow running webknossos-worker jobs `export_tiff` and `render_animation` on public datasets owned by other organizations (multi-organization setups only). Access via sharing tokens is not included; see #7360. Fixes #8910.
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 170245d and 3cfe6f4.

📒 Files selected for processing (3)
  • app/controllers/JobController.scala (0 hunks)
  • conf/messages (0 hunks)
  • unreleased_changes/8911.md (1 hunks)
💤 Files with no reviewable changes (2)
  • app/controllers/JobController.scala
  • conf/messages
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
  • GitHub Check: backend-tests
  • GitHub Check: frontend-tests
  • GitHub Check: build-smoketest-push

@fm3 fm3 merged commit b525db4 into master Sep 10, 2025
5 checks passed
@fm3 fm3 deleted the some-jobs-cross-orga-if-public branch September 10, 2025 13:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Allow export_tiff, render_animation for public datasets of other organization

2 participants