Skip to content

video_module: Add deserialize logic for metadata fields. - #679

Closed
cahrens wants to merge 2 commits into
masterfrom
christina/quotes
Closed

video_module: Add deserialize logic for metadata fields.#679
cahrens wants to merge 2 commits into
masterfrom
christina/quotes

Conversation

@cahrens

@cahrens cahrens commented Aug 14, 2013

Copy link
Copy Markdown

STUD-640

@cahrens

cahrens commented Aug 14, 2013

Copy link
Copy Markdown
Author

@peter-fogg @vaxXxa Please review.

@peter-fogg

Copy link
Copy Markdown
Contributor

Looks good; 👍 when tests pass.

STUD-640

Handle the case of the double-quotes appearing within the string.

We don't need to worry about the whole concatenated ID being
double-quoted, since the concatenation was done by overriding xml_module's
export method.

Use the method already fined in XmlDescriptor.
@cahrens

cahrens commented Aug 14, 2013

Copy link
Copy Markdown
Author

I modified the solution to make use of the existing deserialize method in XmlDescriptor.

@peter-fogg

Copy link
Copy Markdown
Contributor

👍 New code also looks good.

@vasylnakvasiuk

Copy link
Copy Markdown
Contributor

@cahrens, if I correct understand, in the future we will delete block of code, where we manage quoted strings, right? Cause, VideoModule shouldn't fix this problem in the future.

@cahrens

cahrens commented Aug 15, 2013

Copy link
Copy Markdown
Author

@vaxXxa No, I don't expect we will remove this code in the future. We could possibly remove the code on L299 once we are confident that all our courses do not have the double-quoting within the concatenation (that could only have been created on export during a small time frame, since we stopped double-quoting strings), but I don't think we should remove line 357. We have continued using json.dumps for exporting all non-String types, so it is possible to hit issues with other fields as well.

Peter wrote a custom xml_exporter for videoalpha (currently in master as video). So export right now goes through video_alpha's custom implementation. But with @chrisndodge ' PR (https://github.com/edx/edx-platform/pull/688), videoalpha will go back to using XmlDescriptor's serialization mechanism, which means we should deserialize in a consistent way.

@cahrens

cahrens commented Aug 15, 2013

Copy link
Copy Markdown
Author

@vaxXxa You are correct that I can stick with the classmethod instead of staticmethod. I have made that change and pushed up.

I am now going to take these changes and put them on Chris' branch so we can release this stuff together.

@vasylnakvasiuk

Copy link
Copy Markdown
Contributor

@cahrens I'm looking for #668 now.

@cahrens

cahrens commented Aug 15, 2013

Copy link
Copy Markdown
Author

This change got pulled into Chris' PR.

@cahrens cahrens closed this Aug 15, 2013
chrisrossi pushed a commit to jazkarta/edx-platform that referenced this pull request Mar 31, 2014
jenkins-ks pushed a commit to nttks/edx-platform that referenced this pull request Mar 30, 2016
e-kolpakov referenced this pull request in open-craft/openedx-platform Jun 22, 2016
jfavellar90 pushed a commit to eduNEXT/edx-platform that referenced this pull request Apr 11, 2018
…anslation-fix

UPD: removing translating of studio options, not needed, and breaks e…
rediris pushed a commit to gymnasium/edx-platform that referenced this pull request Feb 25, 2021
Sujeet1379 pushed a commit to chandrudev/edx-platform that referenced this pull request Nov 17, 2022
DanielVZ96 referenced this pull request in open-craft/openedx-platform Jan 31, 2024
* Update mongo3.6 to 4 on devstack

* Added script to upgrade mongo to 4
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.

3 participants