Bug #17154

user profile form bugs

Added by Ward Vandewege 5 days ago. Updated 1 day ago.

Status:
Resolved
Priority:
Normal
Assigned To:
Category:
Workbench
Target version:
Start date:
11/20/2020
Due date:
% Done:

100%

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

Description

The mandatory user profile is now activated on pirca.

Bugs:

  • the first time the data is entered, for a new user, it doesn't seem to save the data, and the user is forced to fill it in again.
  • when visiting the form later, from the 'Manage profile' link, the 'role at organization' is always shown as the first item in the list ('Analyst'). It's either not being saved at all, or the form isn't preselecting the saved value correctly.

Subtasks

Task #17156: Review 17154-wb-profileResolvedPeter Amstutz

Associated revisions

Revision b95c8179
Added by Peter Amstutz 2 days ago

Merge branch '17154-wb-profile' refs #17154

Arvados-DCO-1.1-Signed-off-by: Peter Amstutz <>

History

#1 Updated by Peter Amstutz 5 days ago

17154-wb-profile @ c11f254a98e7caf437422d86eab38c7ba4f04096

The issue is that when you get a user record using "current user of token" it can fetch a stale user record from the local cluster. So even though the profile settings were written to the user record on the login cluster, the profile settings don't appear in the "current user" response because it is returning a cached record from the local cluster.

Explicitly fetching user record forces it to get a fresh record from the login cluster.

https://ci.arvados.org/view/Developer/job/developer-run-tests/2189/

#2 Updated by Peter Amstutz 5 days ago

  • Assigned To set to Peter Amstutz
  • Status changed from New to In Progress

#3 Updated by Peter Amstutz 5 days ago

Another way to approach this would be to tweak controller so that user record updates also ensure that the update is sent to the local cluster.

#4 Updated by Peter Amstutz 5 days ago

#5 Updated by Ward Vandewege 5 days ago

Review comments:

  • In apps/workbench/app/controllers/users_controller.rb, dev profile, please add a comment about why it forces a user object re-read.
  • in lib/controller/federation/conn.go, would the more generic solution be to force a cache refresh for the user object? Do we have infrastructure for that yet? Doing the update again on the local cluster seems like it could end us up in an undesirable situation with 2 diverging objects (e.g. if the local copy is already out of date, then the code would update the login cluster and then apply the update to the local cluster -- which would then have an object that's a mix of a stale object and the updated user profile).

#6 Updated by Tom Clegg 2 days ago

9d755d7 -- should use !strings.HasPrefix() instead of options.UUID[:5]!=... to avoid crashing on short value (just good practice -- even though in this case an invalid UUID shouldn't get this far because UserUpdate would fail).

#7 Updated by Peter Amstutz 2 days ago

Ward Vandewege wrote:

Review comments:

  • In apps/workbench/app/controllers/users_controller.rb, dev profile, please add a comment about why it forces a user object re-read.

Added comment.

  • in lib/controller/federation/conn.go, would the more generic solution be to force a cache refresh for the user object? Do we have infrastructure for that yet? Doing the update again on the local cluster seems like it could end us up in an undesirable situation with 2 diverging objects (e.g. if the local copy is already out of date, then the code would update the login cluster and then apply the update to the local cluster -- which would then have an object that's a mix of a stale object and the updated user profile).

The update on the local cluster uses the update response from the home cluster. So it isn't like it is making the same change in both places, it is making the change, getting back a full copy of the object, and updating locally from that.

17154-wb-profile @ 0fe238d1f4240d22b2c58838b03a7607656652d3

https://ci.arvados.org/view/Developer/job/developer-run-tests/2192/

#8 Updated by Ward Vandewege 2 days ago

Peter Amstutz wrote:

Ward Vandewege wrote:

Review comments:

  • In apps/workbench/app/controllers/users_controller.rb, dev profile, please add a comment about why it forces a user object re-read.

Added comment.

  • in lib/controller/federation/conn.go, would the more generic solution be to force a cache refresh for the user object? Do we have infrastructure for that yet? Doing the update again on the local cluster seems like it could end us up in an undesirable situation with 2 diverging objects (e.g. if the local copy is already out of date, then the code would update the login cluster and then apply the update to the local cluster -- which would then have an object that's a mix of a stale object and the updated user profile).

The update on the local cluster uses the update response from the home cluster. So it isn't like it is making the same change in both places, it is making the change, getting back a full copy of the object, and updating locally from that.

17154-wb-profile @ 0fe238d1f4240d22b2c58838b03a7607656652d3

https://ci.arvados.org/view/Developer/job/developer-run-tests/2192/

Thanks, LGTM.

#9 Updated by Peter Amstutz 2 days ago

  • Release set to 37

#10 Updated by Peter Amstutz 2 days ago

  • Status changed from In Progress to Resolved

Also available in: Atom PDF