Bug #15407

[WB2] Property keys on collections are getting translated from snake_case to camelCase

Added by Lucas Di Pentima 6 months ago. Updated about 2 months ago.

Status:
Resolved
Priority:
Normal
Assigned To:
Category:
Workbench2
Target version:
Start date:
08/19/2019
Due date:
% Done:

100%

Estimated time:
(Total: 0.00 h)
Story points:
-
Release relationship:
Auto

Description

While working on https://dev.arvados.org/issues/14874 I came across an issue with how workbench2 handles JSON data from the API server: the keys are being translated from snake_case format to camelCase.

In the context of properties handling, the problem is evident when looking at a collection that has a property with a key like responsible_person_uuid, because it's displayed as responsiblePersonUuid.

Also, when updating properties, the translated keys are being submitted to the API server, corrupting the original set of properties.

Looking at the code, it seems that the snake case formatting is done due to internal representation of data, but this is applied to all the API server provided data recursively. I've tried modifying the recursive call on https://github.com/curoverse/arvados-workbench2/blob/06fc8f6a97afa24144cb435f5af228fec6162a6b/src/services/common-service/common-service.ts#L57 and the properties start show and behave correctly. The problem is that a couple of tests start to fail because this recursive behavior is needed on other places.


Subtasks

Task #15509: Review 15407-camel-casing-fixResolvedLucas Di Pentima


Related issues

Related to Arvados - Feature #14874: [API] Protected collection property -- preserves original project owner through copy/move/modify operationsResolved06/18/2019

Related to Arvados - Story #15067: [Workbench 2] Update property editing to use IDsResolved11/12/2019

Has duplicate Arvados Workbench 2 - Bug #15645: [Workbench2] Property Keys are converted into Camel Case Format in Collection ViewDuplicate

Blocks Arvados Workbench 2 - Bug #14984: Display command line, inputs with links to collectionsNew

History

#1 Updated by Lucas Di Pentima 6 months ago

  • Related to Feature #14874: [API] Protected collection property -- preserves original project owner through copy/move/modify operations added

#2 Updated by Tom Morris 5 months ago

  • Target version set to Workbench2 Q3, Q4

#3 Updated by Tom Morris 5 months ago

  • Blocks Bug #14984: Display command line, inputs with links to collections added

#4 Updated by Lucas Di Pentima 4 months ago

  • Related to Story #15067: [Workbench 2] Update property editing to use IDs added

#5 Updated by Tom Morris 4 months ago

  • Target version changed from Workbench2 Q3, Q4 to 2019-08-14 Sprint
  • Project changed from Arvados Workbench 2 to Arvados

#6 Updated by Tom Morris 4 months ago

  • Assigned To set to Lucas Di Pentima

#7 Updated by Lucas Di Pentima 4 months ago

  • Status changed from New to In Progress

#8 Updated by Lucas Di Pentima 4 months ago

  • Category set to Workbench2
  • Subject changed from Property keys on collections are getting translated from snake_case to camelCase to [WB2] Property keys on collections are getting translated from snake_case to camelCase

#9 Updated by Lucas Di Pentima 4 months ago

  • Target version changed from 2019-08-14 Sprint to 2019-08-28 Sprint

#10 Updated by Lucas Di Pentima 4 months ago

Updates at commit d88effa - branch 14507-camel-casing-fix (wb2 repo)

  • Limit the camelCasing to first level keys on objects from the api server, recursing on items to support listings
  • Added tests for the above
  • Fixes handling of keys on JSONB columns
  • Fixes handling of CWL mounts' keys
  • Fixes "re-run process" feature also failing on the current wb2 version

#11 Updated by Eric Biagiotti 4 months ago

  • Looks like your commits are labeled with the wrong ticket number. Not sure there is anything you can do though...
  • Are we sure $graph in workflowDefinition.$graph!.find can't be null? If not we should check explicitly.
  • If "/var/lib/cwl/workflow.json" is a constant, it should be defined somewhere.
  • In process-input-actions, ln 18. Can this be split up so the "find" doesn't have to be called twice and we don't have a 380 column line.

#12 Updated by Lucas Di Pentima 4 months ago

Eric Biagiotti wrote:

  • Looks like your commits are labeled with the wrong ticket number. Not sure there is anything you can do though...

I've renamed the branch and amended the commit comments, thanks for the notice!

  • Are we sure $graph in workflowDefinition.$graph!.find can't be null? If not we should check explicitly.
  • If "/var/lib/cwl/workflow.json" is a constant, it should be defined somewhere.
  • In process-input-actions, ln 18. Can this be split up so the "find" doesn't have to be called twice and we don't have a 380 column line.

All fixed at commit c5fa0c4e - branch 15407-camel-casing-fix (wb2 repo)

Also added a check when starting the websocket service, if the cluster config doesn't include an url for that service, it'll notify the user and start the app instead of crashing.

#13 Updated by Eric Biagiotti 4 months ago

Lucas Di Pentima wrote:

Eric Biagiotti wrote:

  • Looks like your commits are labeled with the wrong ticket number. Not sure there is anything you can do though...

I've renamed the branch and amended the commit comments, thanks for the notice!

  • Are we sure $graph in workflowDefinition.$graph!.find can't be null? If not we should check explicitly.
  • If "/var/lib/cwl/workflow.json" is a constant, it should be defined somewhere.
  • In process-input-actions, ln 18. Can this be split up so the "find" doesn't have to be called twice and we don't have a 380 column line.

All fixed at commit c5fa0c4e - branch 15407-camel-casing-fix (wb2 repo)

Also added a check when starting the websocket service, if the cluster config doesn't include an url for that service, it'll notify the user and start the app instead of crashing.

Awesome, thanks! This LGTM, except kind of a nitpick. Can we add a bit more detail to the error/user warning. Like "...Container log streaming will not work. Contact the site administrator to enable this feature..." or something to that effect. Up to you though.

And now that I'm thinking about it, I'm worried that if not using websockets is intentional, all users will get this warning popup every time they log in. This is what the notification menu solves, but it looks like it doesn't actually have any functionality :/ Maybe the better approach is to scrap the snackbar message and make a ticket for getting the notifications menu working and adding this to it.

Also, just double checking that we have a ticket for workflow inputs not working. Thanks!

#14 Updated by Lucas Di Pentima 4 months ago

Updates at commit 79d59f23

  • Removed the snackbar message.
  • Added #15569 to implement the warning as a notification on the bell menu.
  • Updated #15557 to also include input selection on workflow running dialog.

#15 Updated by Lucas Di Pentima 4 months ago

  • Status changed from In Progress to Resolved

#16 Updated by Tom Morris about 2 months ago

  • Related to Bug #15645: [Workbench2] Property Keys are converted into Camel Case Format in Collection View added

#17 Updated by Lucas Di Pentima about 2 months ago

  • Release set to 27

#18 Updated by Tom Morris about 2 months ago

  • Related to deleted (Bug #15645: [Workbench2] Property Keys are converted into Camel Case Format in Collection View)

#19 Updated by Tom Morris about 2 months ago

  • Has duplicate Bug #15645: [Workbench2] Property Keys are converted into Camel Case Format in Collection View added

Also available in: Atom PDF