Skip to content

WIP - Content libraries settings overrides - #6

Closed
bradenmacdonald wants to merge 2 commits into
content-libraries-prototypefrom
content-libraries-settings-overrides
Closed

WIP - Content libraries settings overrides#6
bradenmacdonald wants to merge 2 commits into
content-libraries-prototypefrom
content-libraries-settings-overrides

Conversation

@bradenmacdonald

Copy link
Copy Markdown
Member

Changes split mongo behaviour so that XBlock fields with Scope.settings can have their default overriden in the definition. This adds a new defaults object to definitions in split mongo.

Notes:

  • This approach works well with the studio UI. When overriding a library content child within a course context, you can tell if an override is set or not, and pressing the "reset" button resets back to the library value, not the normal XBlock value.
  • Not quite done - currently for libraries it saves Scope.settings fields to defaults but it loads them back as defaults and not as normal values when editing within the library. This could cause minor bugs in some circumstances and will be fixed. See xmodule/modulestore/split_mongo/split.py:1737
  • What's the purpose of split's persist_xblock_dag and _persist_subdag methods? They are only used in a couple tests as far as I can tell, yet they are not in a tests module... I didn't update them to be compatible with this feature.

One potential problem is that the definitions are often lazy-loaded, so in order to check if the default is set or not, we sometimes now need to load the definition when in the past it wasn't loaded. This was causing some platform tests to fail because more mongo calls are used than before. Possible solutions:

  • Set a has_defaults flag on the structure - this is what I have done for now
  • Avoid/reduce lazy loading
  • Change code to only check for defaults if block is a descendant of a LibraryContentBlock
  • Don't allow XBlock fields to be overridden in this way - use some other approach

@bradenmacdonald bradenmacdonald changed the title Content libraries settings overrides WIP - Content libraries settings overrides Oct 31, 2014

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

It's not quite identical to the replacement. I believe you've tried refactoring to pieces of code into a lambda (which is good), but missed the fact that they are slightly different (which is a problem) :) Namely class_ != self.load_block_type(block_key.type)

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Good eye! It was actually a deliberate change, as it seems as far as I can tell that class_ == self.load_block_type(block_key.type)

@bradenmacdonald
bradenmacdonald force-pushed the content-libraries-prototype branch 4 times, most recently from 773624e to 8cb9bb4 Compare November 3, 2014 21:39
@bradenmacdonald
bradenmacdonald force-pushed the content-libraries-settings-overrides branch from 3d9fc0d to 7cb2c45 Compare November 4, 2014 08:54
@bradenmacdonald
bradenmacdonald force-pushed the content-libraries-prototype branch from cbe44a0 to d9c583b Compare November 4, 2014 22:52
@bradenmacdonald
bradenmacdonald force-pushed the content-libraries-settings-overrides branch from 7cb2c45 to 5c988b2 Compare November 5, 2014 03:52
@bradenmacdonald
bradenmacdonald force-pushed the content-libraries-settings-overrides branch from 5c988b2 to 2879d20 Compare November 6, 2014 20:44
@bradenmacdonald
bradenmacdonald force-pushed the content-libraries-prototype branch from 3e44683 to 1c5c007 Compare November 14, 2014 05:36
@bradenmacdonald
bradenmacdonald force-pushed the content-libraries-settings-overrides branch from 2879d20 to e1ff935 Compare November 14, 2014 06:00
@bradenmacdonald

Copy link
Copy Markdown
Member Author

Replaced by #25.

@bradenmacdonald
bradenmacdonald deleted the content-libraries-settings-overrides branch December 19, 2014 03:48
mtyaka pushed a commit that referenced this pull request Feb 10, 2016
mtyaka pushed a commit that referenced this pull request Feb 10, 2016
…dback-update-7

Hash Update #6: Diagnostic-Feedback [Bug Fixes]
pomegranited added a commit that referenced this pull request Aug 13, 2018
…eference (#6)

Convert mongo read_preference setting at a lower level.

(cherry picked from commit 622ee7a)
pomegranited pushed a commit that referenced this pull request Jun 18, 2019
Sign up for free to 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.

2 participants