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 partially imported channels in the device plugin #11231

Merged

Conversation

rtibbles
Copy link
Member

@rtibbles rtibbles commented Sep 12, 2023

Summary

  • Channels can now be partially imported which means that the device plugin might still need to import the full channel metadata even if some channel metadata is present
  • Fixes this by ensuring that a partially imported channel will still be imported by a channel import task - and adds a test to be sure of this
  • Also by default filters requests for a single channels metadata for partial=false to ensure that only fully imported channels are displayed
  • Exempts the channel status display page from this, as it is just showing the currently imported channel state, so the partial import is sufficient - all other usages seem to be for importing more, so need the full metadata.

References

Fixes #11185

Note that this slightly exacerbates the situation for #11225 as it will wipe any previously annotated admin_imported=False from anywhere, but I am assigned to that issue too, so it's my problem to clean up there :)

Reviewer guidance

Ensure that partially imported channels (such as that produced by learner downloads or syncing of lessons/quizzes) do not prevent additional resources being imported in those channels.


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 DEV: backend Python, databases, networking, filesystem... APP: Device Re: Device App (content import/export, facility-syncing, user permissions, etc.) SIZE: small labels Sep 12, 2023
@rtibbles rtibbles force-pushed the partial_to_a_bit_more branch from beee743 to 6ce6b34 Compare September 13, 2023 15:53
Copy link
Member

@jredrejo jredrejo left a comment

Choose a reason for hiding this comment

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

LGTM

@rtibbles rtibbles merged commit 7ded957 into learningequality:release-v0.16.x Sep 13, 2023
@rtibbles rtibbles deleted the partial_to_a_bit_more branch September 13, 2023 16:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
APP: Device Re: Device App (content import/export, facility-syncing, user permissions, etc.) DEV: backend Python, databases, networking, filesystem... SIZE: small
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Device plugin does not check that a channel is fully imported before doing 'import more'
2 participants