Consolidate list remote users api

[AlexVelezLl]

Summary

Consolidates the logic of listing remote users from the user_profile and setup_wizard plugins, and migrates them to core auth so that they are accessible by multiple plugins.

There were some differences between both endpoints, and the following decisions were made to merge them:

• The setup wizard endpoint returned an object { admin, students }, while the user_profile endpoint returned a list of users. It was decided to keep the response the same as that of user_profile as it was more flexible, and in the setup wizard to make the admin, students filter on the frontend instead of the backend.
• The setup wizard endpoint returns a 403 error if there are authentication errors, but the user_profile endpoint returns a 200 code, but with an error object. The 403 error was maintained, and where it is called to the user_profile endpoint in the frontend the error handling was updated.
• The setup_wizard endpoint gets a "facility_id" param, while the user_profile endpoint gets a "facility" param. The naming of "facility_id" was preferred.
• The user_profile endpoint validated the input data, and the setup_wizard endpoint did not. It was preferred to validate it as the user_profile.

Reviewer guidance

• Check the flow of importing users as administrator of the setup wizard.
• Check the flow of changing facilities for an "on my own" user, and merging users using administrator credentials.

Testing checklist

• Contributor has fully tested the PR manually
• If there are any front-end changes, before/after screenshots are included
• Critical user journeys are covered by Gherkin stories
• Critical and brittle code paths are covered by unit tests

PR process

• PR has the correct target branch and milestone
• PR has 'needs review' or 'work-in-progress' label
• If PR is ready for review, a reviewer has been added. (Don't use 'Assignees')
• If this is an important user-facing change, PR or related issue has a 'changelog' label
• If this includes an internal dependency change, a link to the diff is provided

Reviewer checklist

• Automated test coverage is satisfactory
• PR is fully functional
• PR has been tested for accessibility regressions
• External dependency files were updated if necessary (yarn and pip)
• Documentation is updated
• Contributor is in AUTHORS.md

[github-actions]

Build Artifacts

Asset type	Download link
PEX file	kolibri-0.17.0a0.dev0_git.88.g5cf101b9.pex
Windows Installer (EXE)	kolibri-0.17.0a0.dev0+git.88.g5cf101b9-windows-setup-unsigned.exe
Debian Package	kolibri_0.17.0a0.dev0+git.88.g5cf101b9-0ubuntu1_all.deb
Mac Installer (DMG)	kolibri-0.17.0a0.dev0+git.88.g5cf101b9.dmg
Android Package (APK)	kolibri-0.17.0a0.dev0+git.88.g5cf101b9-0.1.4-debug.apk
TAR file	kolibri-0.17.0a0.dev0+git.88.g5cf101b9.tar.gz
WHL file	kolibri-0.17.0a0.dev0+git.88.g5cf101b9-py2.py3-none-any.whl

[rtibbles]

Changes make sense, and being able to reuse the same API endpoint with just some minor tweaks of how they are handled in Javascript is a neat way to do this with minimal changes.

Having more tests, and also using the API endpoints more consistently would be preferable in the future, but this does well to do the minimal required change.

[pcenov]

Hi @AlexVelezLl - I confirm that there are no issues when importing learners as an administrator in the setup wizard or when changing facilities for an "On my own" learner. However when I attempt to merge a learner that was created on the actual "On my own" device then when I try to migrate the learner either by using his password or by using an admin account I am getting permission denied 403 errors in the console and the learner cannot be migrated:

migrate.user.mp4

LogsDB.zip

[AlexVelezLl]

Hi @pcenov! I have solved the mentioned issues both when mergin the self user and with an admin:

@rtibbles I have updated the RemoteFacilityUserAuthenticatedViewset so if the queried user is not an admin we just return the user: 98f2f55.

Also, since this commit the IsSelf permission was not working ok because local_user_id was not in kwargs. I tried to solve this here, would love to hear if you have any comment about this 👐.

[rtibbles]

I think rather than mess with the signature of the task function, it might be simpler just to update how the IsSelf references the args/kwargs?

class IsSelf(BasePermission):
    def user_can_run_job(self, user, job):
        return user.id == job.args[1] or user.id == job.kwargs.get("remote_user_pk", None)

    def user_can_read_job(self, user, job):
        return user.id == job.args[1] or user.id == job.kwargs.get("remote_user_pk", None)

[pcenov]

Thanks @AlexVelezLl - I confirm that everything is working correctly now!

[AlexVelezLl]

Thank you @pcenov! @rtibbles would you give it one last look, please? 👐.