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

(WIP) Graded library content - Do not merge, for internal review #3

Closed

Conversation

bradenmacdonald
Copy link
Member

This is a continuation of the prototype found in #2, extended to implement SOL-19 "As a student, when I complete interactive exercises displayed in a LibraryContent block in the LMS, I see a single aggregated grade for the set of exercises in the progress tab."

Testing instructions: Follow same testing steps as #2, except on step 7 when adding content to the library, add problems (of any type). On step 10, be sure to set "Scored" to True. Then, after all the other steps, view the course as a student. Check that the "Progress" tab is correct, before and after submitting answers to the problems. Test the weighting and make sure none of the children appear in the progress view - only the combined score of the LibraryContent block.

Library screenshot:
graded1
LMS screenshot:
graded2

branch = key.course_key.branch
if branch is None or branch == ModuleStoreEnum.BranchName.draft:
return True # This is the default branch, so strip out branch from any locators
return False # This is not the default branch - we must leave branch information in
Copy link
Member

Choose a reason for hiding this comment

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

This feels hackier still than in #1 . Would it make sense to clean this up, ie not having get_item removing the branch ever, and fixing the higher-level code that relies on this instead? I imagine it would likely be a lot of work, but if we can leave the place cleaner after us, it will be appreciated.

Copy link
Member Author

Choose a reason for hiding this comment

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

I had to make this change because my earlier code broke the Acid XBlock in studio. I agree it's less than ideal.

About changing it in the modulestore: we would need to discuss that with upstream, probably Don. It's a small change code-wise that could have a lot of big implications throughout the codebase.

Copy link
Member Author

Choose a reason for hiding this comment

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

Thinking about this some more: If we're using a new "LibraryLocator" to reference libraries, then we won't have a special branch, so this code may not be needed at all :)

I'll investigate more today.

Copy link
Member Author

Choose a reason for hiding this comment

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

@antoviaque Yep, good news: I have developed a prototype of new locators for the Content Libraries, and with that change, this code (and the other remove_branch stuff you were questioning) is no longer needed at all.

Copy link
Member

Choose a reason for hiding this comment

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

Great : ) And thank you for the other answers, it useful to read the explanations.

@bradenmacdonald
Copy link
Member Author

Superseded by #5.

@bradenmacdonald bradenmacdonald deleted the graded-library-content branch October 29, 2014 02:40
smarnach added a commit that referenced this pull request Oct 20, 2015
Add French translations and hide course number in the page header.
itsjeyd pushed a commit that referenced this pull request Dec 23, 2015
itsjeyd pushed a commit that referenced this pull request Dec 23, 2015
…dback-update-3

Hash Update #3: Diagnostic-Feedback
pomegranited added a commit that referenced this pull request Feb 20, 2018
Fix pip dependencies broken since ginkgo.1, and mongo replicaset read_preference
samuelallan72 pushed a commit that referenced this pull request Sep 5, 2019
tartansandal pushed a commit that referenced this pull request Mar 6, 2020
…te_configuration_siteconfiguration_values_2.3

Revert "Revert "Rename values in SiteConfiguration (2/3) attempt #2""  (attempt #3)
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