Add Recommended Configuration to Task Types #3466

daniel-wer · 2018-11-19T15:07:48Z

This PR allows to add a recommended configuration to task types. The recommended config is shown to users when they start to trace a task and their configuration differs from the recommended one. Users can then accept or decline this config which automatically applies it (or not).

The message will not trigger twice if the "Finish and get next task" functionality is used and the user changes settings in the mean time. It will also not trigger if a tracing is continued. However, if the user returns to the dashboard and starts the next task from there, the message will trigger again, although the task type did not change.
The requirement (from discuss) was:

These settings would then be set whenever a hiwi get a new task type.
again, only once when the task type changes.

but I'm not sure how to achieve that. We would need to persist the last task type for that.
With the current implementation, I'm afraid users might get annoyed with the configuration recommendations. What do you think?

URL of deployed dev instance (used for testing):

https://recommendedsettings.webknossos.xyz

Steps to test:

Create a new task type. Select to "Add Recommended User Settings" and play around with the values, json in the textarea. You should not be able to create the task type with an invalid configuration.
Once you created a task type with a recommended config, click "Edit" in the task type list. The recommended config field should be checked and expanded.
During task type creation, if the recommended config contains errors and you uncheck that you want to add a recommended config, you should still be able to create the task type.
Create a task with a task type with/without a recommended config and get this task as a user.
Open the task to trace. You should see a message informing you about the recommended settings that can be applied or not.
Once you started tracing and reload the page, the message should no longer appear.

Issues:

fixes Allow to specify recommended user settings via task type #3363

Updated changelog
Updated migration guide if applicable
Ready for review

…ommended-settings

daniel-wer · 2018-11-21T12:17:08Z

app/assets/javascripts/oxalis/api/api_latest.js

   *
   * @example
-   * api.user.setConfiguration("segmentationOpacity", 20);
+   * api.data.setConfiguration("segmentationOpacity", 20);


Some of these were messed up, probably because of copy paste errors :)

philippotto

Nice implementation and refactoring 👍 Code per se looks good. I'll test it in a bit.

app/assets/javascripts/admin/api_flow_types.js

app/assets/javascripts/admin/tasktype/task_type_create_view.js

philippotto · 2018-11-21T17:38:27Z

app/assets/javascripts/libs/render_independently.js

-    if (unmountResult && div.parentNode) {
-      div.parentNode.removeChild(div);
+// The returned promise gets resolved once the element is destroyed.
+export default function renderIndependently(


app/assets/javascripts/messages.js

app/assets/javascripts/oxalis/controller.js

app/assets/javascripts/oxalis/model/sagas/task_saga.js

app/assets/javascripts/test/enzyme/snapshot.e2e.js

philippotto · 2018-11-22T09:08:07Z

Ok, I just tested the feature and it worked fine except for a few bits:

when editing an existing task type, checking the checkbox for recommended settings yielded in an empty settings field (I tried it for the default task type and was wondering what I should type in)
when using the "finish and get next task", I noticed two things:
- switching to the next task without changing task type or dataset, the modal still appeared (even though you wrote "The message will not trigger twice if the "Finish and get next task" functionality is used").
- in addition, in the above scenario the modal appeared twice in a row even if I did not change the accepted settings. I stepped through the code a bit and found that the keyboard delay (which should be set to 0) seemed to differ in the store? However, the settings should the correct value 🤔 Very strange. You should be able to reproduce it by creating two equal tasks and using finish-and-get-next-task (click accept and don't change settings).

The message will not trigger twice if the "Finish and get next task" functionality is used and the user changes settings in the mean time.

Where is this modelled in the code? I couldn't find the logic for that. I'm only seeing that on each wk_ready event, you are comparing the recommended settings with the actual ones? Wouldn't it make sense to reuse the "did task type change?" logic? If you are not changing the task type, the modal should probably never appear?

…ommended-settings

daniel-wer · 2018-11-22T15:47:00Z

@philippotto Thanks for the thorough testing!

When editing an existing task type without recommended settings, the default settings are now shown when checking the checkbox. I've also added all setting possibilities to the right of the textarea.
Using "Finish and Get Next Task" should no longer re-trigger the recommended settings modal.
The keyboardDelay bug was triggered because mistakenly the datasetConfiguration contained a keyboardDelay value as well (probably some legacy property), although it belongs into the userConfiguration. We've already fixed this in the DB on master. You can fix it locally by running this script in pgadmin: update webknossos.user_datasetconfigurations set configuration = configuration - 'keyboardDelay'.

It would be really cool if you could do another round of testing :)

…ommended-settings

daniel-wer · 2018-11-22T16:31:23Z

@rschwanhold In order to make this feature even more useful we want to persist the lastTaskTypeId for each user. lastTaskTypeId should be an optional field for each user (so it doesn't need to be included when creating or updating the user). In order to make it foolproof to update the lastTaskTypeId, it would be cool if you could add a PATCH route (or probably better make the /api/users/<userId> route accept PATCH requests), meaning we could send a request that only contains the userId and lastTaskTypeId (or whatever other user properties) and only those properties are updated :)

…ields

…ds/webknossos into recommended-settings

rschwanhold · 2018-11-22T22:11:10Z

@daniel-wer I have added the lastTaskTypeId and changed the update-method of the User to support JSON-files with only a few values.
The route is still labeled as PUT instead of PATCH because I didn't want to update the corresponding frontend code, but feel free to do it yourself.

rschwanhold · 2018-11-22T22:14:29Z

app/models/user/UserService.scala

        isSuperUser = false,
-        isDeactivated = !isActive
+        isDeactivated = !isActive,
+        lastTaskTypeId = None


@fm3 why do I need to specify lastTaskTypeId here but not in InitialDataController ?

Do you get compiler errors if you don’t? I don’t see why it would be a problem to omit it.

taking that back, I got the error also for InitialDataController. I suppose this has something to do with the other named parameters

daniel-wer · 2018-11-23T11:58:10Z

@rschwanhold or @fm3 Seems to work very well, except that when getting the user, the lastTaskTypeId is an object with an id property instead of just the id as a string: lastTaskTypeId: {id: "5be99ac0930100d330dfef94"}. Could you change this, so it is just the string?

rschwanhold · 2018-11-25T16:12:32Z

@daniel-wer you said that lastTaskTypeId should be optional. Is there a use case to set the field to NULL (in the DB) after this field contained an actual taskTypeId? Because I think right now this is not supported. How would you intend to set the value to NULL when you call the update-method?

daniel-wer · 2018-11-26T09:30:34Z

@rschwanhold I don't think we'll ever need to set the lastTaskTypeId back to null after it contained a valid value. With "optional" I meant that it does not need to be included when updating the user. So all good :)

Do you know why the schema comparison fails on CircleCI?

…ommended-settings

…ute PATCH

daniel-wer · 2018-11-26T12:13:00Z

When editing an existing task type without recommended settings, the default settings are now shown when checking the checkbox. I've also added all setting possibilities to the right of the textarea.

Using "Finish and Get Next Task" should no longer re-trigger the recommended settings modal.

The keyboardDelay bug was triggered because mistakenly the datasetConfiguration contained a keyboardDelay value as well (probably some legacy property), although it belongs into the userConfiguration. We've already fixed this in the DB on master. You can fix it locally by running this script in pgadmin: update webknossos.user_datasetconfigurations set configuration = configuration - 'keyboardDelay'.

@philippotto In addition to these changes, the lastTaskTypeId of a user is now persisted as well, which allows to really only show the "Task-Type Description" and "Recommended Configuration" Modals if the user traces a different task type than before :)

It would be great if you could have a final look and retest the things that failed for you last time :)

philippotto

It would be great if you could have a final look and retest the things that failed for you last time :)

Works for me 👍 :)

daniel-wer · 2018-11-27T11:41:19Z

@rschwanhold nevermind, I fixed the schema stuff, now scalafmt complains, though. Would be cool if you could fix that :)

…ossos into recommended-settings

This reverts commit 0f19dbb.

…3502)" This reverts commit 246fbae.

* Revert "Revert "Add Recommended Configuration to Task Types (#3466)" (#3502)" This reverts commit 246fbae. * add route to change the lasttasktypeid * use new taskTypeId route to update user's lastTaskTypeId * add display names and comments to recommended settings * add buttons to remove some of the recommended configuration quickly * uppercase button caption

daniel-wer added 6 commits November 7, 2018 17:52

add recommended settings to task type view

6726b04

Merge branch 'master' of github.com:scalableminds/webknossos into rec…

b2d3cde

…ommended-settings

show modal to apply recommended settings in task

42570d4

Merge branch 'master' of github.com:scalableminds/webknossos into rec…

42bb20e

…ommended-settings

Merge branch 'master' of github.com:scalableminds/webknossos into rec…

1722192

…ommended-settings

ux fixes for task type creation, correctly apply zoom

0beff2c

daniel-wer added the frontend label Nov 19, 2018

daniel-wer self-assigned this Nov 19, 2018

daniel-wer requested a review from philippotto November 19, 2018 15:07

daniel-wer commented Nov 21, 2018

View reviewed changes

philippotto reviewed Nov 22, 2018

View reviewed changes

Merge branch 'master' into recommended-settings

3ee26e9

daniel-wer added 2 commits November 22, 2018 13:50

Merge branch 'master' of github.com:scalableminds/webknossos into rec…

886f6cf

…ommended-settings

apply PR feedback, fix e2e tests

847093a

Merge branch 'master' of github.com:scalableminds/webknossos into rec…

37c2229

…ommended-settings

daniel-wer and others added 4 commits November 22, 2018 18:53

use lastTaskTypeId attribute of user and update it, untested

a1a979f

add lastTaskTypeId to user and allow updates of user for individual f…

abf43b2

…ields

Merge branch 'recommended-settings' of https://github.com/scalablemin…

f3c11bb

…ds/webknossos into recommended-settings

fix number of evolution

5f2f402

rschwanhold reviewed Nov 22, 2018

View reviewed changes

stringify lastTaskTypeId for json

63387d8

daniel-wer added 2 commits November 26, 2018 10:52

Merge branch 'master' of github.com:scalableminds/webknossos into rec…

9fcfa74

…ommended-settings

Merge branch 'master' of github.com:scalableminds/webknossos into rec…

7e54634

…ommended-settings

add changelog entry, persist user after changing, make user update ro…

350774e

…ute PATCH

philippotto approved these changes Nov 26, 2018

View reviewed changes

fix schemaVersion and update MIGRATIONS.md

adc42ed

daniel-wer added 9 commits November 28, 2018 13:56

scalafmt

cd778f2

Merge branch 'master' into recommended-settings

a1d0208

adapt users.csv

7ab52c4

Merge branch 'recommended-settings' of github.com:scalableminds/webkn…

67c8028

…ossos into recommended-settings

remove md5 column header from users.csv

81e75ec

correct users.csv column order

ebbe0fa

refresh snapshots

025f78a

revert clippingDistanceArbitrary to old value

07e33a0

Merge branch 'master' into recommended-settings

1e34e5a

fm3 merged commit 0f19dbb into master Nov 29, 2018

fm3 deleted the recommended-settings branch November 29, 2018 09:16

daniel-wer added a commit that referenced this pull request Nov 29, 2018

Revert "Add Recommended Configuration to Task Types (#3466)"

4937fff

This reverts commit 0f19dbb.

daniel-wer mentioned this pull request Nov 29, 2018

Revert "Add Recommended Configuration to Task Types" #3502

Merged

daniel-wer added a commit that referenced this pull request Nov 29, 2018

Revert "Add Recommended Configuration to Task Types (#3466)" (#3502)

246fbae

This reverts commit 0f19dbb.

daniel-wer added a commit that referenced this pull request Nov 29, 2018

Revert "Revert "Add Recommended Configuration to Task Types (#3466)" (#…

e5aa043

…3502)" This reverts commit 246fbae.

Add Recommended Configuration to Task Types #3466

Add Recommended Configuration to Task Types #3466

Uh oh!

Conversation

daniel-wer commented Nov 19, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

URL of deployed dev instance (used for testing):

Steps to test:

Issues:

Uh oh!

daniel-wer Nov 21, 2018

Choose a reason for hiding this comment

Uh oh!

philippotto left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

philippotto Nov 21, 2018

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

philippotto commented Nov 22, 2018

Uh oh!

daniel-wer commented Nov 22, 2018

Uh oh!

daniel-wer commented Nov 22, 2018

Uh oh!

rschwanhold commented Nov 22, 2018

Uh oh!

rschwanhold Nov 22, 2018

Choose a reason for hiding this comment

Uh oh!

fm3 Nov 22, 2018

Choose a reason for hiding this comment

Uh oh!

fm3 Nov 23, 2018

Choose a reason for hiding this comment

Uh oh!

daniel-wer commented Nov 23, 2018

Uh oh!

rschwanhold commented Nov 25, 2018

Uh oh!

daniel-wer commented Nov 26, 2018

Uh oh!

daniel-wer commented Nov 26, 2018

Uh oh!

philippotto left a comment

Choose a reason for hiding this comment

Uh oh!

daniel-wer commented Nov 27, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

daniel-wer commented Nov 19, 2018 •

edited

Loading

daniel-wer commented Nov 27, 2018 •

edited

Loading