Skip to content
New issue

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

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

Already on GitHub? # to your account

Moves setting current_channel to init #11567

Merged

Conversation

marcellamaki
Copy link
Member

Summary

The error
AttributeError: 'NoLearningActivitiesChannelImport' object has no attribute 'current_channel' was causing channel upgrade to break. The lookup was moved to init() so that it would be defined before it was called in each place it was used

References

Fixes #11557

channel-import-fixes.mp4

Reviewer guidance

Does updating/upgrading a channel to a new version work?
Are there any possible side effects of when this might still not be defined, at this point in the code? (i.e. should some error handling be added)


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 github-actions bot added the DEV: backend Python, databases, networking, filesystem... label Nov 29, 2023
Copy link
Member

@bjester bjester left a comment

Choose a reason for hiding this comment

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

Couple changes needed to ensure defensiveness against more errors, but glad this fixed the issue!

@marcellamaki marcellamaki requested a review from bjester November 29, 2023 19:40
Copy link
Member

@bjester bjester left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Member

@pcenov pcenov left a comment

Choose a reason for hiding this comment

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

I confirm - the channel update is working correctly now!

@marcellamaki
Copy link
Member Author

can somebody (@bjester @rtibbles ) confirm my python test "fixes" are reasonable before merge? if we are being honest I solved this via the error messages not via a deep understanding of why my changes affected these tests in this way

@bjester
Copy link
Member

bjester commented Nov 30, 2023

can somebody (@bjester @rtibbles ) confirm my python test "fixes" are reasonable before merge? if we are being honest I solved this via the error messages not via a deep understanding of why my changes affected these tests in this way

They look appropriate to me!

@bjester bjester merged commit 033b522 into learningequality:release-v0.16.x Nov 30, 2023
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
DEV: backend Python, databases, networking, filesystem... SIZE: medium SIZE: small SIZE: very small
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Device > Channels - Channel update process keeps failing
3 participants