Ensure the user will see the welcome modal after login #8367

jredrejo · 2021-08-31T16:38:09Z

Summary

According to the proposed design, when finishing the setup wizard on a Learning Only Device, the modal window to recommend import channels will appear after the user logins

References

Closes: #8308

Reviewer guidance

Do a clean installation importing some user and check after the user logins, he's redirected to the device tab and sees the modal window

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

rtibbles · 2021-08-31T19:45:15Z

kolibri/plugins/learn/assets/src/views/ContentUnavailablePage.vue

+    beforeMount() {
+      if (plugin_data.isSubsetOfUsersDevice) {
+        const deviceContentUrl = urls['kolibri:kolibri.plugins.device:device_management'];
+        redirectBrowser(deviceContentUrl());


We should add two conditionals here:

if (this.canManageContent && deviceContentUrl) {

No point redirecting them if the user does not have content management permissions, and if the plugin is disabled, then the deviceContentUrl will be undefined.

Also, is this required? We redirect a user to the device page after initial setup, and we already have a URL for them to be navigate there.

Honestly, I'm not satisfied with this solution either, but it was the only way I found to do it for the case where the setup wizard uses an admin account to sync the users.
In that case, after the wizard finishes, it shows the login screen (the wizard can't sign in with the first synced user because it does not know the user password), and after login the user is redirected to learn, not to device.

As you said, the user is redirected to device after initial setup... if setup syncing is done by the users not but the admin.

Confirmed that this PR shows the modal @jredrejo describes for learners that were imported using an admin account.

Seems reasonable to me to keep this with the introduction of the extra conditions, and preferably also adding a comment explaining why this is necessary

Yeah, I think I can live with this with the extra conditions and the comment - the explanation for why this is necessary helps!

There's another option, @jtamiace I'd like to know your opinion: the main problem comes from the fact that using the admin account to import users we don't know the users' passwords. When importing finishes the wizard logouts the admin user so any user can login and will be redirected to learn.

If the wizard does not logs out the admin, after the wizard finishes kolibri will be open under the admin account in the device section and all the code from ContentUnavailablePage.vue can be removed, it's not needed as first view of kolibri will be automatically in the device page . If being logged as an admin user after the wizard finishes it is a good option I can remove several lines of code because the wizard will finish always with the same behaviour, no matter who did the sync.

I am guessing that the automated call to logout might be at the root of this issue too: #8379 - so simplifying that code might be a good idea overall!

If the wizard does not logs out the admin, after the wizard finishes kolibri will be open under the admin account in the device section and all the code from ContentUnavailablePage.vue can be removed,

@jredrejo streamlining the experience so the admin goes straight to Device is helpful, but would we still need to keep it in, in case they decide to explore other places of the app before they start importing content? Unless content import is the only thing a user can do post-setup, might be good to keep ContentUnavailablePage.vue in.

rtibbles

I think we don't need the automatic redirect from the content unavailable page. I think the proposal in the issue was to update the text there to make it more straight forward, I think that would be the better change here.

jredrejo · 2021-09-01T07:56:02Z

I think we don't need the automatic redirect from the content unavailable page. I think the proposal in the issue was to update the text there to make it more straight forward, I think that would be the better change here.

I'd be very happy to remove it, please, read my above comment and let's confirm it

jredrejo · 2021-09-01T19:19:15Z

I think we don't need the automatic redirect from the content unavailable page. I think the proposal in the issue was to update the text there to make it more straight forward, I think that would be the better change here.

Ok, I've removed the redirection in the content unavailable page and set the wizard to do the same, no matter who's done it: an admin user or a normal user. So, if an admin does the sync he will stay logged and will be redirected to the device/channels page and see the model after finishing the import and will see exactly the same a normal user, i.e. the animated gif in this PR description.

Ensure the user will see the welcome modal after login

7d44f76

jredrejo added the TODO: needs review Waiting for review label Aug 31, 2021

jredrejo added this to the 0.15.0 milestone Aug 31, 2021

jredrejo requested review from bjester and jtamiace August 31, 2021 16:38

fix tests broken due to promise in beforeCreate

db3e662

jtamiace approved these changes Aug 31, 2021

View reviewed changes

rtibbles reviewed Aug 31, 2021

View reviewed changes

rtibbles requested changes Aug 31, 2021

View reviewed changes

jredrejo mentioned this pull request Aug 31, 2021

Restart zeroconf after setup #8370

Merged

9 tasks

indirectlylit mentioned this pull request Sep 1, 2021

logged out of single-user device within minutes #8379

Closed

Treat admin as any other user after the wizard ends

fbb7e09

Redirect to channels from the bottom bar button

e9c2651

rtibbles approved these changes Sep 1, 2021

View reviewed changes

rtibbles merged commit 9e829d1 into learningequality:release-v0.15.x Sep 1, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Ensure the user will see the welcome modal after login #8367

Ensure the user will see the welcome modal after login #8367

jredrejo commented Aug 31, 2021

rtibbles Aug 31, 2021

rtibbles Aug 31, 2021

jredrejo Sep 1, 2021

indirectlylit Sep 1, 2021

rtibbles Sep 1, 2021

jredrejo Sep 1, 2021

rtibbles Sep 1, 2021

jtamiace Sep 1, 2021

rtibbles left a comment

jredrejo commented Sep 1, 2021

jredrejo commented Sep 1, 2021

Ensure the user will see the welcome modal after login #8367

Ensure the user will see the welcome modal after login #8367

Conversation

jredrejo commented Aug 31, 2021

Summary

References

Reviewer guidance

Testing checklist

PR process

Reviewer checklist

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rtibbles left a comment

Choose a reason for hiding this comment

jredrejo commented Sep 1, 2021

jredrejo commented Sep 1, 2021