Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix rare crash when viewing shared annotation #6892

Merged
merged 8 commits into from
Mar 9, 2023

Conversation

philippotto
Copy link
Member

@philippotto philippotto commented Mar 2, 2023

If a user (who is not the owner) views a shared annotation, editing the layer settings will cause failed edit-annotation requests. After several retries (will take 5 minutes and new edits will reset the timer), the sagas will crash since the user is not allowed to edit annotation properties (only the tracings itself may be changed).

The PR fixes this by:

  • ensuring that the failed saga won't crash the entire app
  • avoiding trying to save if it's not allowed in the first place

URL of deployed dev instance (used for testing):

  • https://___.webknossos.xyz

Steps to test:

  • create a shared annotation
  • ensure that the dataset is accessible to the sample2 user
  • log in as sample2
  • ensure that the UI doesn't expose the edit annotation name and description buttons
  • ensure that changing the layer visibility doesn't cause failed network requests

Issues:


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

Copy link
Contributor

@MichaelBuessemeyer MichaelBuessemeyer left a comment

Choose a reason for hiding this comment

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

Thanks for fixing this bug 🚫 🐛 .

Everything works and the code looks fine besides a small thing you might have missed (see my comment). After you resolved this, it should be ready to be merged 👯

Comment on lines +1 to +13
import type { OxalisState } from "oxalis/store";

export function mayEditAnnotationProperties(state: OxalisState) {
const { owner, restrictions } = state.tracing;
const activeUser = state.activeUser;

return !!(
restrictions.allowUpdate &&
restrictions.allowSave &&
activeUser &&
owner?.id === activeUser.id
);
}
Copy link
Contributor

Choose a reason for hiding this comment

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

🎉

@philippotto philippotto enabled auto-merge (squash) March 9, 2023 10:38
@philippotto philippotto merged commit c5673b9 into master Mar 9, 2023
@philippotto philippotto deleted the fix-crash-in-shared-annotation branch March 9, 2023 10:57
hotzenklotz added a commit that referenced this pull request Mar 9, 2023
…alized_format

* 'master' of github.com:scalableminds/webknossos:
  Fix rare crash when viewing shared annotation (#6892)
hotzenklotz added a commit that referenced this pull request Mar 13, 2023
…come_header_UI

* 'master' of github.com:scalableminds/webknossos:
  Fix rare crash when viewing shared annotation (#6892)
hotzenklotz added a commit that referenced this pull request Mar 13, 2023
…pdown-menu

* 'master' of github.com:scalableminds/webknossos:
  Avoid SQL error when fetching view config for zero-layer dataset (#6912)
  Fix date formatting for VX reports (#6908)
  Fix rare crash when viewing shared annotation (#6892)
  Slim down view mode dropdown by using icons (#6900)
  Logging on password reset/change (#6901)
  When merging volume tracings, also merge segment lists (#6882)
  avoid spinner when switching tabs in dashboard (#6894)
@philippotto philippotto mentioned this pull request Mar 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Client tries to store layer visibility in shared annotation
2 participants