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

Fixed icon encodings not getting saved #1576

Merged
merged 1 commit into from
Aug 8, 2019

Conversation

jayoshih
Copy link
Contributor

@jayoshih jayoshih commented Aug 8, 2019

Description

Persists save on channel thumbnail to end of the command

It turns out that Django's save function acts more like a POST request in that all fields get submitted, rather than those that have changed like a PATCH request. This was causing an issue as the publish_channel function had one version of the channel and the create_content_database had another version. The create_content_database method would save the icon_encoding. However, any saving after the create_content_database function would then use the outdated model from publish_channel, which caused icon_encoding to get set back to null

This just passes the model itself to create_content_database from publish_channel so they are working from the same object in memory

Issue Addressed (if applicable)

Addresses #1299

Steps to Test

  • Remove the thumbnail encoding from a channel
  • Publish and check the public channels endpoint

Checklist

  • Is the code clean and well-commented?
  • Has the CHANGELOG label been added to this pull request? Items with this label will be added to the CHANGELOG at a later time
  • Are there tests for this change?

@jayoshih jayoshih requested a review from kollivier August 8, 2019 00:25
@jayoshih jayoshih added the changelog Needs to be added to the changelog (remove once added) label Aug 8, 2019
@codecov
Copy link

codecov bot commented Aug 8, 2019

Codecov Report

Merging #1576 into master will decrease coverage by 0.03%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1576      +/-   ##
==========================================
- Coverage   79.49%   79.45%   -0.04%     
==========================================
  Files         252      252              
  Lines       11757    11766       +9     
==========================================
+ Hits         9346     9349       +3     
- Misses       2411     2417       +6
Impacted Files Coverage Δ
contentcuration/contentcuration/utils/publish.py 77.97% <100%> (-0.21%) ⬇️
...ration/contentcuration/tests/test_exportchannel.py 90.84% <100%> (-2.95%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b1054e8...3f7a0c9. Read the comment docs.

@jayoshih jayoshih merged commit 8a4160e into learningequality:master Aug 8, 2019
@jayoshih jayoshih deleted the publish-fix-fixed branch March 9, 2020 18:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
changelog Needs to be added to the changelog (remove once added)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants