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

NewBadge missing when downloading channel #11484

Merged
merged 2 commits into from
Nov 10, 2023

Conversation

thesujai
Copy link
Contributor

@thesujai thesujai commented Nov 1, 2023

Summary

task.channel_id was undefined here

References

Related to #6730

Reviewer guidance

It is clear that this addresses the bug due to which #6730 was not being able to reproduced


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 APP: Device Re: Device App (content import/export, facility-syncing, user permissions, etc.) SIZE: very small labels Nov 1, 2023
@thesujai thesujai changed the title Update icon NewBadge missing when downloading channel Nov 1, 2023
@rtibbles
Copy link
Member

rtibbles commented Nov 2, 2023

Nice detective work! This feels like a good fix to go into 0.16, so I'll retarget it. Let me know if you're comfortable rebasing onto the release-v0.16.x branch, or I am happy to do it for you.

@rtibbles rtibbles changed the base branch from develop to release-v0.16.x November 2, 2023 15:09
@rtibbles rtibbles self-assigned this Nov 2, 2023
@rtibbles rtibbles self-requested a review November 2, 2023 15:32
@thesujai
Copy link
Contributor Author

thesujai commented Nov 2, 2023

@rtibbles okay you did it anyways! Happy to contribute:)

@rtibbles
Copy link
Member

rtibbles commented Nov 2, 2023

I retargeted the PR, but did not rebase - this would need to be rebased onto release-v0.16.x (rather than currently where it is branched from develop, so has about 231 commits in it that we don't want in the release branch!).

@thesujai
Copy link
Contributor Author

thesujai commented Nov 3, 2023

@rtibbles should be GTG now

@bjester bjester merged commit dd407cc into learningequality:release-v0.16.x Nov 10, 2023
34 checks passed
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.) SIZE: very small
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants