Skip to content

Support ContentNode.options when publishing. - #1896

Merged
rtibbles merged 4 commits into
learningequality:hotfixesfrom
kollivier:options_master
Jun 26, 2020
Merged

Support ContentNode.options when publishing.#1896
rtibbles merged 4 commits into
learningequality:hotfixesfrom
kollivier:options_master

Conversation

@kollivier

Copy link
Copy Markdown
Contributor

Description

One thing we have needed in Kolibri is a way to specify rendering and other display options for particular ContentNodes. A common example of this is when HTML5 content needs to render at a particular height and/or width.

There is a PR to add this on the Kolibri side (learningequality/kolibri#6873) but we also need to be able to export those options to the SQLite db as part of the publish operation. That is what this PR does. :)

Issue Addressed (if applicable)

Addresses #PR# HERE

Before/After Screenshots (if applicable)

Insert images here

Steps to Test

  • Set extra_fields to {"options": {"height": "1000px"}} on some ricecooker HTML5 kind ContentNodes
  • Run ricecooker and publish the channel
  • View the channel on a Kolibri with the above Kolibri PR applied and make sure the content appears at the new height!

Implementation Notes (optional)

At a high level, how did you implement this?

Briefly describe how this works

Does this introduce any tech-debt items?

List anything that will need to be addressed later

Checklist

Delete any items that don't apply

  • Is the code clean and well-commented?
  • Has the docs label been added if this introduces a change that needs to be updated in the user docs?
  • 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?
  • Are all user-facing strings translated properly (if applicable)?
  • Has the notranslate class been added to elements that shouldn't be translated by Google Chrome's automatic translation feature (e.g. icons, user-generated text)?
  • Are all UI components LTR and RTL compliant (if applicable)?
  • Are there any new ways this uses user data that needs to be factored into our Privacy Policy?
  • Are there any new interactions that need to be added to the QA Sheet?
  • Are there opportunities for using Google Analytics here (if applicable)?
  • If the Pipfile has been changed, is the updated Pipfile.lock file also included in this PR?
  • Are the migrations safe for a large db (if applicable)?

Comments

Any additional notes you'd like to add

Reviewers

If you are looking to assign a reviewer, here are some options:

  • Jordan jayoshih (full stack)
  • Aron aronasorman (back end, devops)
  • Micah micahscopes (full stack)
  • Kevin kollivier (back end)
  • Ivan ivanistheone (Ricecooker)
  • Richard rtibbles (full stack, Kolibri)
  • Radina @radinamatic (documentation)

jayoshih
jayoshih previously requested changes May 20, 2020

@jayoshihjayoshih left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Compatibility check

license = models.ForeignKey('License', null=True, blank=True)

# A JSON Dictionary of properties to configure loading, rendering, etc. the file
options = JSONField(default={})

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Noting that Kolibri uses a sqlite database, so not sure if this is compatible

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

It's using the jsonfield library JSONField which is what Kolibri uses too, so should be ok.

@jayoshih
jayoshih dismissed their stale reviewMay 20, 2020 22:42

Nvm, just need to do a json.dumps

@kollivier
kollivier changed the base branch from master to hotfixesJune 17, 2020 22:21
@codecov

codecovBot commented Jun 17, 2020

Copy link
Copy Markdown

Codecov Report

Merging #1896 into hotfixes will increase coverage by 0.00%.
The diff coverage is 90.00%.

Impacted file tree graph

@@ Coverage Diff @@## hotfixes #1896 +/- ##
=========================================
Coverage 80.84% 80.84% =========================================
Files 285 286 +1 Lines 12683 12693 +10 =========================================
+ Hits 10253 10262 +9 - Misses 2430 2431 +1 
Impacted FilesCoverage Δ
contentcuration/contentcuration/utils/publish.py77.46% <66.66%> (-0.09%)⬇️
...bri_content/migrations/0009_contentnode_options.py100.00% <100.00%> (ø)
contentcuration/kolibri_content/models.py80.44% <100.00%> (+0.10%)⬆️

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 203e173...a4730cb. Read the comment docs.

@rtibbles
rtibbles merged commit 3deba56 into learningequality:hotfixesJun 26, 2020
Sign up for freeto 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.

3 participants

@kollivier@rtibbles@jayoshih