Better setup wizard errors #11655

rtibbles · 2023-12-15T23:58:03Z

Summary

Does some cleanup of vue prop warnings
Cleans up some form warnings
Reverts my previous suggestion to @akolson to use UIAlert, as it causes a big old mess without additional strings, and reverts to the AppError component
Always displays a retry and a restart button if there is an error on the provisioning page to ensure that people can start over if needed.

References

Reviewer guidance

I tested this by adding raise Exception here: https://github.com/learningequality/kolibri/blob/release-v0.16.x/kolibri/core/device/api.py#L101

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

Always show retry and restart buttons when there is an error.

github-actions · 2023-12-16T00:34:53Z

Build Artifacts

Asset type	Download link
PEX file	kolibri-0.16.0b10.dev0_git.78.gcbfb35a3.pex
Windows Installer (EXE)	kolibri-0.16.0b10.dev0+git.78.gcbfb35a3-unsigned.exe
Debian Package	kolibri_0.16.0b10.dev0+git.78.gcbfb35a3-0ubuntu1_all.deb
Mac Installer (DMG)	kolibri-0.16.0b10.dev0+git.78.gcbfb35a3-0.3.0.dmg
Android Package (APK)	kolibri-0.16.0b10.dev0+git.78.gcbfb35a3-0.1.0-debug.apk
TAR file	kolibri-0.16.0b10.dev0+git.78.gcbfb35a3.tar.gz
WHL file	kolibri-0.16.0b10.dev0+git.78.gcbfb35a3-py2.py3-none-any.whl

nucleogenesis · 2023-12-18T22:34:09Z

kolibri/plugins/setup_wizard/assets/src/views/onboarding-forms/SettingUpKolibri.vue

@@ -166,7 +163,7 @@
        return this.wizardContext('onMyOwnOrGroup') == UsePresets.ON_MY_OWN;
      },
    },
-    mounted() {
+    created() {


Curious why the change from mounted -> created?

"Do you need to access the DOM? If no, don't use mounted." - past Richard (to me)

Well from the look of it, we are not manipulating the DOM(at least not directly) so it justifies the change.

akolson

Changes look correct to me. With additional manual QA, we should be good to go! Thanks @rtibbles!

nucleogenesis

Overall this LGTM!

nucleogenesis · 2023-12-21T21:28:25Z

packages/kolibri-common/components/AppError/index.vue

+      <KButtonGroup>
+        <slot name="buttons">


marcellamaki · 2024-01-02T15:33:13Z

@radinamatic - could we get some manual QA here? One use case to test Radina was the issue you reported some time ago with running into issues with Android "On My Own" setup with earlier android devices. Testing on this would be helpful because it would be two for the price of one - seeing if the updates here are working, and it would give us more useful error messages for that problem to see if we can continue to make some progress on it, since I've been a bit blocked in being able to figure out what's wrong. Thank you!

marcellamaki

Requesting changes just to block for merge until manual QA happens. Noting that I have not done code review here and have deferred to @akolson and @nucleogenesis on this one, but I would be happy to review the code if needed.

pcenov

Hi @rtibbles, unfortunately now the error page gets briefly displayed in all setup flows just after having successfully completed the setup process. It won't let me click on Help us by reporting this error. Here's an example for On my own:

2024-01-04_16-24-21.mp4

The same is also happening in the Android app:

2024-01-04_16-01-02.mp4

Otherwise I can confirm that in a scenario where there is an actual error it's now being handled gracefully:

2024-01-04_16-29-41.mp4

Logs:
UbuntuLogs.zip

AndroidOnMyOwnLogs.zip

rtibbles · 2024-01-04T17:39:16Z

Thanks @pcenov a simple oversight on my part to not conditionalize the display of the error - should be fixed now.

pcenov

Implemented as specified above, no regression issues observed - good to go!

dismissing as only blocker was manual QA approval

rtibbles added 4 commits December 15, 2023 15:52

Add autocomplete attribute to username component.

c0c721b

Prevent vue prop warnings and form warnings for user credentials form.

a1abca0

Simplify AppError component.

1a95d79

Revert previous suggestion to use UIAlert directly.

1b8ae36

Always show retry and restart buttons when there is an error.

rtibbles added the TODO: needs review Waiting for review label Dec 15, 2023

rtibbles added this to the Kolibri 0.16 Release: Bugs and Regressions milestone Dec 15, 2023

rtibbles requested review from akolson and marcellamaki December 15, 2023 23:58

github-actions bot added APP: Setup Wizard Re: Setup Wizard (facility import, superuser creation, settings, etc.) DEV: frontend SIZE: small labels Dec 15, 2023

nucleogenesis reviewed Dec 18, 2023

View reviewed changes

marcellamaki self-assigned this Dec 19, 2023

akolson reviewed Dec 20, 2023

View reviewed changes

nucleogenesis approved these changes Dec 21, 2023

View reviewed changes

packages/kolibri-common/components/AppError/index.vue

Comment on lines +16 to +17

<KButtonGroup>

<slot name="buttons">

Copy link

Member

nucleogenesis Dec 21, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice <3

marcellamaki previously requested changes Jan 2, 2024

View reviewed changes

pcenov self-requested a review January 3, 2024 15:55

pcenov requested changes Jan 4, 2024

View reviewed changes

pcenov mentioned this pull request Jan 4, 2024

Improve error management and allow the user to retry and report errors during "Setting Up Kolibri" #11574

Closed

6 tasks

Restore conditional display of errors.

8e7aa55

pcenov self-requested a review January 5, 2024 12:30

pcenov approved these changes Jan 5, 2024

View reviewed changes

marcellamaki merged commit dfe1a9d into learningequality:release-v0.16.x Jan 5, 2024
34 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Better setup wizard errors #11655

Better setup wizard errors #11655

rtibbles commented Dec 15, 2023

github-actions bot commented Dec 16, 2023 •

edited

Loading

nucleogenesis Dec 18, 2023

marcellamaki Dec 18, 2023

akolson Dec 20, 2023

akolson left a comment

nucleogenesis left a comment

nucleogenesis Dec 21, 2023

marcellamaki commented Jan 2, 2024

marcellamaki left a comment

pcenov left a comment

rtibbles commented Jan 4, 2024

pcenov left a comment

Better setup wizard errors #11655

Better setup wizard errors #11655

Conversation

rtibbles commented Dec 15, 2023

Summary

References

Reviewer guidance

Testing checklist

PR process

Reviewer checklist

github-actions bot commented Dec 16, 2023 • edited Loading

Build Artifacts

nucleogenesis Dec 18, 2023

Choose a reason for hiding this comment

marcellamaki Dec 18, 2023

Choose a reason for hiding this comment

akolson Dec 20, 2023

Choose a reason for hiding this comment

akolson left a comment

Choose a reason for hiding this comment

nucleogenesis left a comment

Choose a reason for hiding this comment

nucleogenesis Dec 21, 2023

Choose a reason for hiding this comment

marcellamaki commented Jan 2, 2024

marcellamaki left a comment

Choose a reason for hiding this comment

pcenov left a comment

Choose a reason for hiding this comment

rtibbles commented Jan 4, 2024

pcenov left a comment

Choose a reason for hiding this comment

github-actions bot commented Dec 16, 2023 •

edited

Loading