Write wkurl into annotation nml file #6964

frcroth · 2023-04-04T13:04:43Z

URL of deployed dev instance (used for testing):

https://wkurlnml.webknossos.xyz/

Steps to test:

Create an annotation
Upload the annotation from the dashboard -> no problem
In your downloaded nml file, change the org (or dataset name) and wkUrl
See a descriptive error

When uploading annotations from the dataset view it seems like org and dataset are not checked? Therefore wkUrl is not used there.

Issues:

fixes Save & use wk-URL in NML #6610

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

fm3

Backend mostly LGTM :)

I left one comment on checking the url on upload. My other two refactorign-related comments I’m not certain about myself, maybe you could consider your view on them.

The front-end also has an NML export in serializeToNml in nml_helpers.ts. The wkurl should be added there too (serializeParameters). @philippotto maybe you could give a pointer on how to get the current host there? Simply from window.location? Or is there a better way?

app/models/annotation/nml/NmlParser.scala

app/models/annotation/nml/NmlWriter.scala

app/controllers/AnnotationIOController.scala

philippotto · 2023-04-06T13:25:58Z

@philippotto maybe you could give a pointer on how to get the current host there? Simply from window.location? Or is there a better way?

Yes, I think ${location.protocol}//${location.host} should be fine.

fm3

Backend LGTM and works as expected :) front-end export works as expected as well.
Looks like location is not available during the front-end tests, though, making the CI red. @philippotto any pointers? 😅

…wkurl-nml

conf/messages

philippotto · 2023-04-12T13:06:09Z

Frontend looks good to me, too :) I hope that I fixed the CI.

frcroth · 2023-04-12T13:26:09Z

snapshot needs to be recreated. Also the resulting wkUrl="//" does not look very useful. Is there just no host at time of test?

…wkurl-nml

philippotto · 2023-04-12T14:22:59Z

snapshot needs to be recreated. Also the resulting wkUrl="//" does not look very useful. Is there just no host at time of test?

I adapted the mocked location to have a protocol and a host. I also tried running yarn refresh-snapshots but get the following errrors:

e2e-tests_1                | [error] /home/sbt-user/webknossos/test/backend/NMLUnitTestSuite.scala:80:53: type mismatch;
e2e-tests_1                | [error]  found   : net.liftweb.common.Box[(Option[com.scalableminds.webknossos.datastore.SkeletonTracing.SkeletonTracing], List[models.annotation.UploadedVolumeLayer], String, Option[String])]
e2e-tests_1                | [error]  required: net.liftweb.common.Box[(Option[com.scalableminds.webknossos.datastore.SkeletonTracing.SkeletonTracing], List[models.annotation.UploadedVolumeLayer], String, String)]
e2e-tests_1                | [error]       assert(!isParseSuccessful(writeAndParseTracing(newTracing)))
e2e-tests_1                | [error]                                                     ^
e2e-tests_1                | [error] /home/sbt-user/webknossos/test/backend/NMLUnitTestSuite.scala:86:53: type mismatch;
e2e-tests_1                | [error]  found   : net.liftweb.common.Box[(Option[com.scalableminds.webknossos.datastore.SkeletonTracing.SkeletonTracing], List[models.annotation.UploadedVolumeLayer], String, Option[String])]
e2e-tests_1                | [error]  required: net.liftweb.common.Box[(Option[com.scalableminds.webknossos.datastore.SkeletonTracing.SkeletonTracing], List[models.annotation.UploadedVolumeLayer], String, String)]
e2e-tests_1                | [error]       assert(!isParseSuccessful(writeAndParseTracing(newTracing)))
e2e-tests_1                | [error]                                                     ^
e2e-tests_1                | [error] /home/sbt-user/webknossos/test/backend/NMLUnitTestSuite.scala:93:53: type mismatch;
e2e-tests_1                | [error]  found   : net.liftweb.common.Box[(Option[com.scalableminds.webknossos.datastore.SkeletonTracing.SkeletonTracing], List[models.annotation.UploadedVolumeLayer], String, Option[String])]
e2e-tests_1                | [error]  required: net.liftweb.common.Box[(Option[com.scalableminds.webknossos.datastore.SkeletonTracing.SkeletonTracing], List[models.annotation.UploadedVolumeLayer], String, String)]
e2e-tests_1                | [error]       assert(!isParseSuccessful(writeAndParseTracing(newTracing)))
e2e-tests_1                | [error]                                                     ^
e2e-tests_1                | [error] /home/sbt-user/webknossos/test/backend/NMLUnitTestSuite.scala:100:53: type mismatch;
e2e-tests_1                | [error]  found   : net.liftweb.common.Box[(Option[com.scalableminds.webknossos.datastore.SkeletonTracing.SkeletonTracing], List[models.annotation.UploadedVolumeLayer], String, Option[String])]
e2e-tests_1                | [error]  required: net.liftweb.common.Box[(Option[com.scalableminds.webknossos.datastore.SkeletonTracing.SkeletonTracing], List[models.annotation.UploadedVolumeLayer], String, String)]
e2e-tests_1                | [error]       assert(!isParseSuccessful(writeAndParseTracing(newTracing)))
e2e-tests_1                | [error]                                                     ^
e2e-tests_1                | [error] /home/sbt-user/webknossos/test/backend/NMLUnitTestSuite.scala:107:53: type mismatch;
e2e-tests_1                | [error]  found   : net.liftweb.common.Box[(Option[com.scalableminds.webknossos.datastore.SkeletonTracing.SkeletonTracing], List[models.annotation.UploadedVolumeLayer], String, Option[String])]
e2e-tests_1                | [error]  required: net.liftweb.common.Box[(Option[com.scalableminds.webknossos.datastore.SkeletonTracing.SkeletonTracing], List[models.annotation.UploadedVolumeLayer], String, String)]
e2e-tests_1                | [error]       assert(!isParseSuccessful(writeAndParseTracing(newTracing)))
e2e-tests_1                | [error]                                                     ^
e2e-tests_1                | [error] /home/sbt-user/webknossos/test/backend/NMLUnitTestSuite.scala:113:53: type mismatch;
e2e-tests_1                | [error]  found   : net.liftweb.common.Box[(Option[com.scalableminds.webknossos.datastore.SkeletonTracing.SkeletonTracing], List[models.annotation.UploadedVolumeLayer], String, Option[String])]
e2e-tests_1                | [error]  required: net.liftweb.common.Box[(Option[com.scalableminds.webknossos.datastore.SkeletonTracing.SkeletonTracing], List[models.annotation.UploadedVolumeLayer], String, String)]
e2e-tests_1                | [error]       assert(!isParseSuccessful(writeAndParseTracing(newTracing)))
e2e-tests_1                | [error]                                                     ^
e2e-tests_1                | [error] /home/sbt-user/webknossos/test/backend/NMLUnitTestSuite.scala:121:53: type mismatch;
e2e-tests_1                | [error]  found   : net.liftweb.common.Box[(Option[com.scalableminds.webknossos.datastore.SkeletonTracing.SkeletonTracing], List[models.annotation.UploadedVolumeLayer], String, Option[String])]
e2e-tests_1                | [error]  required: net.liftweb.common.Box[(Option[com.scalableminds.webknossos.datastore.SkeletonTracing.SkeletonTracing], List[models.annotation.UploadedVolumeLayer], String, String)]
e2e-tests_1                | [error]       assert(!isParseSuccessful(writeAndParseTracing(newTracing)))
e2e-tests_1                | [error]                                                     ^
e2e-tests_1                | [error] /home/sbt-user/webknossos/test/backend/NMLUnitTestSuite.scala:128:53: type mismatch;
e2e-tests_1                | [error]  found   : net.liftweb.common.Box[(Option[com.scalableminds.webknossos.datastore.SkeletonTracing.SkeletonTracing], List[models.annotation.UploadedVolumeLayer], String, Option[String])]
e2e-tests_1                | [error]  required: net.liftweb.common.Box[(Option[com.scalableminds.webknossos.datastore.SkeletonTracing.SkeletonTracing], List[models.annotation.UploadedVolumeLayer], String, String)]
e2e-tests_1                | [error]       assert(!isParseSuccessful(writeAndParseTracing(newTracing)))
e2e-tests_1                | [error]                                                     ^
e2e-tests_1                | [error] /home/sbt-user/webknossos/test/backend/NMLUnitTestSuite.scala:134:53: type mismatch;
e2e-tests_1                | [error]  found   : net.liftweb.common.Box[(Option[com.scalableminds.webknossos.datastore.SkeletonTracing.SkeletonTracing], List[models.annotation.UploadedVolumeLayer], String, Option[String])]
e2e-tests_1                | [error]  required: net.liftweb.common.Box[(Option[com.scalableminds.webknossos.datastore.SkeletonTracing.SkeletonTracing], List[models.annotation.UploadedVolumeLayer], String, String)]
e2e-tests_1                | [error]       assert(!isParseSuccessful(writeAndParseTracing(newTracing)))
e2e-tests_1                | [error]                                                     ^
e2e-tests_1                | [error] 9 errors found
e2e-tests_1                | [error] (Test / compileIncremental) Compilation failed
e2e-tests_1                | [error] Total time: 27 s, completed Apr 12, 2023 4:20:43 PM

I ran a ./clean beforehand, but this doesn't change anything. Maybe you know how to fix this?

frcroth · 2023-04-16T11:08:35Z

Don't know why, but the wkUrl is not added when recreating the snapshots.

fm3 · 2023-04-17T11:07:05Z

@philippotto Do you have insights as to why that may be so? Also the frontend tests still fail 🙈 maybe you could have a look there too. No great hurry :)

philippotto · 2023-04-18T06:58:17Z

Should be fixed now. I also took the liberty to rename the yarn commands a bit to make them clearer. There are two different snapshot kinds (from e2e tests and from unit tests) which explains why the necessary snapshots didn't update (I also fell for this).
The distinction between e2e and all (instead of unit) is not ideal, but a bit harder to change due to the folder hierarchy. I hope this increment is an improvement, nevertheless.

fm3 · 2023-04-18T07:04:39Z

Sounds good, thanks! And refreshing the unit test snapshots is just yarn start after previously removing them? 😮

philippotto · 2023-04-18T07:06:52Z

Sounds good, thanks! And refreshing the unit test snapshots is just yarn start after previously removing them? open_mouth

Oops. Still early apparently 🙈

fm3 · 2023-04-18T07:07:49Z

No worries :) Thanks for taking care of this!

frcroth and others added 5 commits April 4, 2023 13:38

Write wkurl into nml file

9e4736d

Check wk url for better errors on annotation uploading

9538eb8

Merge branch 'master' into wkurl-nml

8000be6

Fix typo

2e5830b

Update changelog

95bd296

frcroth requested a review from fm3 April 5, 2023 09:31

frcroth marked this pull request as ready for review April 5, 2023 09:31

fm3 reviewed Apr 6, 2023

View reviewed changes

app/models/annotation/nml/NmlParser.scala Outdated Show resolved Hide resolved

app/models/annotation/nml/NmlWriter.scala Show resolved Hide resolved

app/controllers/AnnotationIOController.scala Outdated Show resolved Hide resolved

philippotto reviewed Apr 6, 2023

View reviewed changes

app/controllers/AnnotationIOController.scala Show resolved Hide resolved

frcroth changed the title ~~Draft: Write wkurl into nml file~~ Write wkurl into nml file Apr 12, 2023

frcroth and others added 3 commits April 12, 2023 09:35

Merge branch 'master' into wkurl-nml

b4b234c

Apply suggestions from code review

7e5bebc

Write wkurl in front end nml serializer

ea50b52

frcroth requested a review from fm3 April 12, 2023 08:09

philippotto assigned frcroth Apr 12, 2023

fm3 approved these changes Apr 12, 2023

View reviewed changes

philippotto added 2 commits April 12, 2023 14:55

fix that test tries to use global location

63ebb85

Merge branch 'wkurl-nml' of github.com:scalableminds/webknossos into …

e875bd4

…wkurl-nml

philippotto reviewed Apr 12, 2023

View reviewed changes

conf/messages Outdated Show resolved Hide resolved

frcroth and others added 3 commits April 12, 2023 15:49

Fix typos in messages

48e89e0

adapt mocked location to have protocol and host

286e738

Merge branch 'wkurl-nml' of github.com:scalableminds/webknossos into …

a46d964

…wkurl-nml

frcroth and others added 2 commits April 16, 2023 12:44

Fix test

8752755

Merge branch 'master' into wkurl-nml

84b3d5f

philippotto added 2 commits April 17, 2023 17:33

update nml snapshot

d0299c4

make snapshot-related yarn commands clearer

6a7fd9e

fix yarn start to yarn test

c6465f0

Merge branch 'master' into wkurl-nml

71a262e

frcroth changed the title ~~Write wkurl into nml file~~ Write wkurl into annotation nml file Apr 18, 2023

frcroth merged commit 0e6e6e7 into master Apr 18, 2023
1 check passed

frcroth deleted the wkurl-nml branch April 18, 2023 07:45

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Write wkurl into annotation nml file #6964

Write wkurl into annotation nml file #6964

frcroth commented Apr 4, 2023 •

edited by fm3

fm3 left a comment

philippotto commented Apr 6, 2023

fm3 left a comment

philippotto commented Apr 12, 2023 •

edited

frcroth commented Apr 12, 2023

philippotto commented Apr 12, 2023

frcroth commented Apr 16, 2023

fm3 commented Apr 17, 2023

philippotto commented Apr 18, 2023

fm3 commented Apr 18, 2023

philippotto commented Apr 18, 2023

fm3 commented Apr 18, 2023

Write wkurl into annotation nml file #6964

Write wkurl into annotation nml file #6964

Conversation

frcroth commented Apr 4, 2023 • edited by fm3

URL of deployed dev instance (used for testing):

Steps to test:

Issues:

fm3 left a comment

Choose a reason for hiding this comment

philippotto commented Apr 6, 2023

fm3 left a comment

Choose a reason for hiding this comment

philippotto commented Apr 12, 2023 • edited

frcroth commented Apr 12, 2023

philippotto commented Apr 12, 2023

frcroth commented Apr 16, 2023

fm3 commented Apr 17, 2023

philippotto commented Apr 18, 2023

fm3 commented Apr 18, 2023

philippotto commented Apr 18, 2023

fm3 commented Apr 18, 2023

frcroth commented Apr 4, 2023 •

edited by fm3

philippotto commented Apr 12, 2023 •

edited