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

Link Previews #7331

Merged
merged 21 commits into from
Oct 2, 2023
Merged

Link Previews #7331

merged 21 commits into from
Oct 2, 2023

Conversation

fm3
Copy link
Member

@fm3 fm3 commented Sep 13, 2023

URL of deployed dev instance (used for testing):

Steps to test:

  • Copy links to annotations and datasets to slack
  • If public or with sharing token, it should show name and sensible text and thumbnail preview
  • Should also work for shortlinks
  • Should not show relevant information in case of non-public datasets (if no sharing token is supplied)

TODOs:

  • Use tokens for permission checks (this should only work on public datasets/annotations or when a token is supplied)
    • ctx for annotations+datasets
    • default depending on page type
    • include thumbnail with token (not only for public)
    • resolve short links
    • Access Check for workflows?
    • test that
  • Remove react-based attempts (would have been the nicer code, but sadly the remote sites don’t execute any javascript)

Issues:


  • Updated changelog
  • Removed dev-only changes like prints and application.conf edits

@fm3 fm3 changed the title WIP: Link Previews Link Previews Sep 28, 2023
@fm3 fm3 marked this pull request as ready for review September 28, 2023 12:27
@fm3 fm3 requested a review from normanrz September 28, 2023 12:27
token: Option[String]): Option[String] =
layerOpt.map { layer =>
val tokenParam = token.map(t => s"&sharingToken=$t").getOrElse("")
s"${conf.Http.uri}/api/datasets/${organization.name}/${dataset.name}/layers/${layer.name}/thumbnail?w=1000&h=300$tokenParam"
Copy link
Member

Choose a reason for hiding this comment

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

the size could live in a constant somewhere

@@ -0,0 +1,152 @@
package oxalis.opengraph
Copy link
Member

Choose a reason for hiding this comment

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

would it be a lot of effort to rename oxalis --> webknossos?

Copy link
Member Author

Choose a reason for hiding this comment

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

Nope, I’ll do that in a separate PR. Truth be told, the package structure doesn’t make that much sense even with the renaming, but it would certainly be an easy improvement

@fm3 fm3 enabled auto-merge (squash) October 2, 2023 09:25
@fm3 fm3 merged commit fa55e89 into master Oct 2, 2023
2 checks passed
@fm3 fm3 deleted the react-helmet branch October 2, 2023 09:42
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.

Add link previews containing more information (dataset/annotation name/page title)
3 participants