Bug #16735

Require password login check throws error that variable is undefined

Added by Daniel Kutyła about 1 year ago. Updated about 1 year ago.

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

100%

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

Description

requirePasswordLogin function need better null|undefined checks in order to aviod null pointer errors


Subtasks

Task #16737: Review 16735-Require-password-loginResolvedDaniel Kutyła

Associated revisions

Revision a708e219
Added by Daniel Kutyła about 1 year ago

Merge branch '16735-Require-password-login'
Closes #16735

Arvados-DCO-1.1-Signed-off-by: Daniel Kutyła <>

History

#1 Updated by Daniel Kutyła about 1 year ago

  • Status changed from New to In Progress

#3 Updated by Lucas Di Pentima about 1 year ago

Reviewing arvados-workbench2|f0fb757 - branch 16735-Require-password-login

  • Great idea to test the function!
  • Instead of adding more checks, one for every Login type, how about having an array of login type names and iterate over it to check if at least one of those exist and has an "Enabled: true” member instead of requiring that all login type to exist on all the clusters that wb2 has a session? This alternative approach would make easier to add more login types (for example, Tom is adding a “Test” login type atm) and also allow wb2 to talk with older cluster versions at the same time.

#5 Updated by Lucas Di Pentima about 1 year ago

Great, thanks! How about adjusting the tests so that validates the new behaviour?
Having a list of Login types and confirming that the func returns true if any of those is enabled. That way we could easily expand the test cases when new login types are added.
We could also check that having a login type not listed doesn't make the func return true.

#7 Updated by Lucas Di Pentima about 1 year ago

LGTM, thanks!

#8 Updated by Daniel Kutyła about 1 year ago

  • % Done changed from 0 to 100
  • Status changed from In Progress to Resolved

#9 Updated by Peter Amstutz about 1 year ago

  • Release set to 25

Also available in: Atom PDF