Skip to content

Conversation

hotzenklotz
Copy link
Member

@hotzenklotz hotzenklotz commented Mar 12, 2025

In view mode, the navbar menu entry for "Create Animation" is shown to every user, regardless of there permissions/role/admin status.

The corresponding modal React component, however, is only mounted for super users. In other words, the modal could not be open/displayed for the majority of our user base.

Steps to test:

  • Make sure to open a dataset in 1) view mode and 2) as non-superadmin user.
  • Select "Create Animation" from the "Menu" in the navbar
  • Modal should appear

Issues:


(Please delete unneeded items, merge only when none are left open)

@hotzenklotz hotzenklotz self-assigned this Mar 12, 2025
Copy link
Contributor

coderabbitai bot commented Mar 12, 2025

📝 Walkthrough

Walkthrough

The change involves the removal of the conditional rendering of the renderAnimationModal based on the activeUser?.isSuperUser check in the ViewDatasetActionsView function, allowing the modal to render unconditionally. Additionally, a new entry has been added to the changelog to document a bug fix related to the "Create Animation" modal not opening from the navbar menu. There are no modifications to any exported or public entities.

Changes

File Summary of Change
frontend/.../view_dataset_actions_view.tsx Removed conditional check using activeUser?.isSuperUser; now, renderAnimationModal always renders.
CHANGELOG.unreleased.md Added entry for bug fix regarding "Create Animation" modal not opening from navbar menu.
frontend/.../create_animation_modal.tsx Modified modal footer; replaced onOk prop with custom footer including "Cancel" and conditional "Start Animation" button.

Assessment against linked issues

Objective Addressed Explanation
The "Create Animation" menu entry should function correctly (8189)
Users should receive feedback when the feature is disabled (8189)

Suggested labels

enhancement

Suggested reviewers

  • philippotto

Poem

I'm a rabbit with a skip and a hop,
Code changes fresh, I just can't stop.
Conditional checks vanished into the night,
Now modals shine with unbridled light.
Hop along, dear coder, and enjoy the sight! 🐇✨

Tip

⚡🧪 Multi-step agentic review comment chat (experimental)
  • We're introducing multi-step agentic chat in review comments. This experimental feature enhances review discussions with the CodeRabbit agentic chat by enabling advanced interactions, including the ability to create pull requests directly from comments.
    - To enable this feature, set early_access to true under in the settings.

📜 Recent review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 3c446f7 and e93d0a6.

📒 Files selected for processing (1)
  • frontend/javascripts/oxalis/view/action-bar/create_animation_modal.tsx (3 hunks)
🧰 Additional context used
🧬 Code Definitions (1)
frontend/javascripts/oxalis/view/action-bar/create_animation_modal.tsx (1)
frontend/javascripts/oxalis/store.ts (1) (1)
  • OxalisState (599:632)
🔇 Additional comments (4)
frontend/javascripts/oxalis/view/action-bar/create_animation_modal.tsx (4)

1-1: Good addition of the Tooltip component.

Adding the Tooltip component is necessary for providing user feedback when the animation feature is disabled.


97-97: Good addition of activeUser selector.

Adding this selector enables proper management of button state based on user authentication status, which aligns with the PR objective of fixing the "animation modal not showing" issue.


291-309: Well-designed modal footer implementation.

The custom footer implementation effectively addresses the issue by:

  1. Always showing the modal while controlling access to functionality
  2. Providing clear feedback with a tooltip when the feature is unavailable
  3. Properly disabling the "Start Animation" button for unauthenticated users (activeUser == null)

This approach improves user experience by making the UI behavior consistent regardless of user permissions, while still maintaining proper access control.


304-308:

❓ Verification inconclusive

Verify behavior for non-super users who are authenticated.

The button is disabled only when activeUser == null, meaning authenticated non-super users could potentially attempt to start animations. The PR mentions that backend would deny unauthorized API calls, but we should ensure this is properly validated.


🏁 Script executed:

#!/bin/bash
# Check if there are server-side authorization checks for animation creation
rg -A 5 "startRenderAnimationJob|render_animation" --type=ts --type=tsx --type=js --type=jsx

Length of output: 122


Action Required: Manual Verification of Backend Authorization

It appears that our initial automated search did not reveal any concrete evidence of server-side checks for restricting animation creation to super users. In the current frontend, the "Start Animation" button is enabled for all authenticated users (i.e. when activeUser != null), so it’s essential to confirm that the backend properly enforces authorization (e.g., by checking the user's privileges before executing startRenderAnimationJob).

Next Steps:

  • Verify Server-Side Checks: Please manually review the backend code to confirm that there are proper authorization checks (e.g., validating if the user is a super user) for the endpoint handling animation creation.
  • Test for Unauthorized Access: Ensure that if a non-super user attempts to start an animation, the backend explicitly denies the request.
  • Update Documentation (if needed): If the backend does not enforce these checks, consider adding additional validation before invoking the API from the frontend or enhancing the server-side logic.
✨ Finishing Touches
  • 📝 Generate Docstrings

🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

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)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@hotzenklotz hotzenklotz requested a review from philippotto March 12, 2025 16:02
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)
CHANGELOG.unreleased.md (1)

29-29: Clear Documentation for the 'Create Animation' Modal Bug Fix

The new changelog entry clearly documents that the "Create Animation" modal bug (where the modal did not open when selected from the navbar menu) has been fixed. The entry appropriately references [#8444] for traceability.

Note: Ensure that any related documentation (e.g., migration guides or client library updates) mentioned in the PR checklist is updated accordingly to reflect this change.

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 27852af and f9be22a.

📒 Files selected for processing (1)
  • CHANGELOG.unreleased.md (1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
  • GitHub Check: circleci_build

Copy link
Member

@philippotto philippotto left a comment

Choose a reason for hiding this comment

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

so, only super users have been able to render animations? git blame shows that this isSuperUser part was added in the original pr #7348, but I thought I saw external users creating animations 🤔 then, I must have dreamt that?


regarding the fix:

  • I tested this on the dev instance and there I can open the modal, but the "submit" button is disabled (see screenshot). the button does not explain why it's disabled. I'd change the button text to "Start Animation" and put a tooltip on it à la "This feature is not enabled on this webknossos instance. please visit webknossos.org".
  • I can open this modal without being logged in now. if the job was enabled on the dev instance, I would probably be able to start the job (not sure whether the backend denies this cc @MichaelBuessemeyer). this should not be possible imo.

image

@hotzenklotz
Copy link
Member Author

hotzenklotz commented Mar 13, 2025

so, only super users have been able to render animations? git blame shows that this isSuperUser part was added in the original pr #7348, but I thought I saw external users creating animations 🤔 then, I must have dreamt that?

In "annotaotion" mode, everyone was able to start animations. The bug occurs only in "view" mode.

@philippotto
Copy link
Member

so, only super users have been able to render animations? git blame shows that this isSuperUser part was added in the original pr #7348, but I thought I saw external users creating animations 🤔 then, I must have dreamt that?

In "annotaotion" mode, everyone was able to start animations. The bug occurs only in "viwe" mode.

ah, thanks for clearing this up 👍

@hotzenklotz
Copy link
Member Author

put a tooltip on it

Done.

I can open this modal without being logged in now. if the job was enabled on the dev instance, I would probably be able to start the job (not sure whether the backend denies this cc @MichaelBuessemeyer). this should not be possible imo.

Yes, one can click on the "Start Animation" button but the backend refuses the API call as unauthorized. It is not the prettiest thing in the world, but at least it prevents any mischief.

Screenshot 2025-03-17 at 13 36 40

@philippotto
Copy link
Member

Yes, one can click on the "Start Animation" button but the backend refuses the API call as unauthorized. It is not the prettiest thing in the world, but at least it prevents any mischief.

Could you add a simple activeUser == null check to the disabled prop of the button?

Copy link
Member

@philippotto philippotto left a comment

Choose a reason for hiding this comment

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

🎉

@philippotto philippotto changed the title Fixed "animation modal" not showing Fix "animation modal" not showing Mar 18, 2025
@philippotto philippotto merged commit d36d67e into master Mar 18, 2025
3 checks passed
@philippotto philippotto deleted the fix-render-animation-modal branch March 18, 2025 14:38
@coderabbitai coderabbitai bot mentioned this pull request Aug 22, 2025
7 tasks
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.

"Create animation" menu entry available but does not do anything
2 participants