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

Handle backend provisioning errors. #7024

Conversation

rtibbles
Copy link
Member

@rtibbles rtibbles commented Jun 10, 2020

Summary

  • At some point we added device settings data into the bootstrapped data in the user plugin page, this caused the page to error when unprovisioned
  • This broke our client side redirect behaviour when a device is unprovisioned
  • This PR adds a middleware to handle DeviceNotProvisioned exceptions and redirect to a setup url when available

Reviewer guidance

Does the code make sense?
Visit /user/ on an unprovisioned Kolibri, does it redirect you to setup_wizard? (note there will be two redirects, first for language, and then for setup).

References

Fixes #7020


Contributor Checklist

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

Testing:

  • 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

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 rtibbles added the TODO: needs review Waiting for review label Jun 10, 2020
@rtibbles rtibbles added this to the 0.14.0 milestone Jun 10, 2020
@rtibbles rtibbles requested a review from nucleogenesis June 10, 2020 18:46
@codecov
Copy link

codecov bot commented Jun 10, 2020

Codecov Report

Merging #7024 into develop will increase coverage by 0.01%.
The diff coverage is 81.25%.

Impacted Files Coverage Δ
kolibri/deployment/default/settings/base.py 84.14% <ø> (ø)
kolibri/core/device/hooks.py 83.33% <50.00%> (-6.67%) ⬇️
kolibri/core/device/middleware.py 92.68% <83.33%> (-3.87%) ⬇️
kolibri/core/views.py 83.00% <100.00%> (+0.64%) ⬆️

@nucleogenesis
Copy link
Member

nucleogenesis commented Jun 10, 2020

/user/ redirects to /setup_wizard/ as expected

However, when I go through with device provisioning I get a "Sign In" browser prompt when I finish the provisioning on the request below.

The request stalls until I cancel the prompt then it responds 401

curl 'http://localhost:8000/api/device/deviceprovision/?1591820440348=1591820440348' \
  -H 'Connection: keep-alive' \
  -H 'Accept: application/json, text/plain, */*' \
  -H 'X-CSRFToken: DjZDpwJzek3Z5j02TTlMvKPtNRxykmo2KmQjm9Vu61BTIcHwyJIAqpxj81vwZ40c' \
  -H 'User-Agent: Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/83.0.4103.61 Safari/537.36' \
  -H 'Content-Type: application/json;charset=utf-8' \
  -H 'Sec-Fetch-Site: same-origin' \
  -H 'Sec-Fetch-Mode: cors' \
  -H 'Sec-Fetch-Dest: empty' \
  -H 'Referer: http://localhost:8000/en/setup_wizard/' \
  -H 'Accept-Language: en-US,en;q=0.9' \
  -H 'Cookie: visitor_id=d41472ad73624f56b109b25a421d8c23; app_key_cookie=208ad8a986f14027b81edd014edaf187; kolibri_csrftoken=DjZDpwJzek3Z5j02TTlMvKPtNRxykmo2KmQjm9Vu61BTIcHwyJIAqpxj81vwZ40c' \
  --compressed

@indirectlylit indirectlylit changed the base branch from develop to release-v0.14.x June 10, 2020 21:23
@rtibbles
Copy link
Member Author

The issue you are reporting is logged at #7030 and there is a separate PR #7034 that resolves it. Sounds like this is good to go though?

Copy link
Contributor

@indirectlylit indirectlylit left a comment

Choose a reason for hiding this comment

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

works for me

@indirectlylit indirectlylit merged commit 95a9a0c into learningequality:release-v0.14.x Jun 17, 2020
@jonboiser jonboiser removed the TODO: needs review Waiting for review label Aug 5, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

non-provisioned devices are not redirected to Setup Wizard when going to /user/ routes
4 participants