Skip to content

Use XBlock handlers in the LMS ... - #1531

Merged
cpennington merged 3 commits into
openedx:masterfrom
cpennington:lms-xblock-handlers
Nov 8, 2013
Merged

Use XBlock handlers in the LMS ...#1531
cpennington merged 3 commits into
openedx:masterfrom
cpennington:lms-xblock-handlers

Conversation

@cpennington

Copy link
Copy Markdown
Contributor

... and use them to implement ajax_url for XModule compatibility

@cpennington

Copy link
Copy Markdown
Contributor Author

@nedbat, @sarina: Review?

@sarina

sarina commented Oct 30, 2013

Copy link
Copy Markdown
Contributor

looks like a lot of XModule unit tests are failing... eek

@cpennington

Copy link
Copy Markdown
Contributor Author

Yeah, I'm working on that. The set of changes should be small, though.

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.

seems like using quote_slashes is unnecessary for the string 'invalid Location'

@sarina

sarina commented Oct 30, 2013

Copy link
Copy Markdown
Contributor

Did a first-pass and it looks pretty good. Will do a more thorough pass when tests pass. Also consider having someone from Studio look at this - there are some CMS changes here.

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.

Maybe I'm just not up on how the CMS will use handlers: is it a given that a handler in the CMS will imply "preview"? Or is this a misleading use of the word "preview"?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

No, you're right, this is preview only, and should be named that way.

@nedbat

nedbat commented Oct 30, 2013

Copy link
Copy Markdown
Contributor

Take care of a few things, and then 👍

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.

This multiline split doesn't seem right (closing paren on final line; would be neater to put the first dictionary entry on line 336 not 335) - split the same way it's split in test_peer_grading.py l 33

Why is input 5 intentionally left unset? I might prefer that in a comment above the declaration of valid_get_dict

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.

4 participants