Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Setup Wizard - importing user from facility that doesn't require learner passwords #11605

Closed
thanksameeelian opened this issue Dec 8, 2023 · 5 comments · Fixed by #11704
Closed
Assignees
Labels
APP: Setup Wizard Re: Setup Wizard (facility import, superuser creation, settings, etc.) bug Behavior is wrong or broken TODO: needs decisions Design or specifications are necessary

Comments

@thanksameeelian
Copy link
Contributor

thanksameeelian commented Dec 8, 2023

Observed behavior

if progressing through the setup wizard: Group-learning > LOD > Import one or more existing user accounts... & the user selects a facility where learners are not required to have a password, the following Import individual user accounts page does not display a password field due to its conditional rendering.

if user enters a username that does not exist within the selected facility, currently an error occurs that is not visible to users and keeps them from progressing further in the wizard. the error appears only in the console and doesn't seem to accurately reflect to the scenario that occurred, as the error details are Password is required, despite the password field not being available to the user.

Errors and logs

image
image
image

code for errors arising in this case was updated recently in #11541

Expected behavior

  • surfacing error to user: currently if the user attempts to enter a non-existent username, a console error occurs but no error surfaced to the user and they remain on the page, inexplicably unable to progress. the user should be made aware of what problems are keeping them from progressing through setup.

  • raising accurate error: currently the error that is raised if user enters a non-existent username, when investigated, has the detail Password is required - this message does not seem to accurately represent the scenario that has occurred, which is expected to be the detail that accompanies error_constants.AUTHENTICATION_FAILED ("401 Client Error: Unauthorized for url:...")

  • handling legitimate non-learner cases: if an extant coach or admin username is entered into the username field on the Import individual user accounts page, user should be prompted for that username's password as well - even if a facility does not require learner passwords, it still does require passwords for coach or admin users. in this case, raising the Password is required error could be appropriate but users also need guidance/a next step nudging them towards the Use an admin account link if the password text field is still not made available on the page.

User-facing consequences

user gets "stuck" on Import individual user accounts page without any indication of what is wrong

Steps to reproduce

  • on/based off of release-v0.16.x branch
  • fresh kolibri instance to trigger setup wizard
  • Group-learning > LOD > Import one or more existing user accounts...
  • connect to a facility in which learner passwords are not required
  • enter a non-existent username into the provided field & attempt to progress through the wizard
  • see console for error details
@thanksameeelian thanksameeelian added bug Behavior is wrong or broken APP: Setup Wizard Re: Setup Wizard (facility import, superuser creation, settings, etc.) labels Dec 8, 2023
@thanksameeelian thanksameeelian added the TODO: needs decisions Design or specifications are necessary label Dec 8, 2023
@aarishshahmohsin
Copy link

Can I be assigned this issue?

@bjester
Copy link
Member

bjester commented Dec 14, 2023

Hi @aarishshahmohsin, thank you for your interest. I don't think this one is suitable for development at the moment. I think we might need new user facing string to better inform the user on what they should do in this situation, which means we need to reprioritize this as well.

We currently have contributing opportunities in three repositories. You can see the contributing guidelines including links to issues suitable for contribution for each repository here:

@bjester
Copy link
Member

bjester commented Dec 14, 2023

For posterity:
In the setup wizard, we already know whether the facility requires a password for learners. We won't be able to provide information through the originating API on whether the learner exists or simply isn't a learner and needs to provide a password. So I think in this scenario we should inform the user to Check the username or provide admin credentials instead.

@nick2432
Copy link
Contributor

can i work on this ?

@akolson
Copy link
Member

akolson commented Dec 29, 2023

Hi @nick2432! Based on @bjester's above comment, I don't think so, but @bjester should be able to respond if otherwise.
Thanks again!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
APP: Setup Wizard Re: Setup Wizard (facility import, superuser creation, settings, etc.) bug Behavior is wrong or broken TODO: needs decisions Design or specifications are necessary
Projects
None yet
6 participants