Fix/cdodge/exporting video modules

[chrisndodge]

No description provided.

[chrisndodge]

@wedaly this fixes broken test. The problem was that clone_item() was deprecated on master in between the time the hotfix was made and when it was merged in.

@cpennington this also does the improvement that you asked for by not parsing an XML formatted string

Can you get to this ASAP since master is broken now?

[wedaly]

👍 Thanks for fixing this!

[dmitchell]

You could use:
ItemFactory.create(
parent_location=parent.location,
category="video",
display_name="untitled")
Same effect but doesn't go thru url handler (views)

[cpennington]

I think you should use module_store.create_and_save_xmodule here, rather than using the django test client. Actually, an even better fix would be to use a CourseFactory and a ItemFactory, so that this test doesn't have to know about future changes to how to construct xmodules.

[chrisndodge]

@dmitchell @cpennington can you sign off? I took Don's solution as I have to head into a 1.5 hour meeting. Unfortunately, when I rebase -i, I took the wrong commit message. But can we move forward?

[dmitchell]

There's a bug partly due to the test:

  File "/usr/lib/python2.7/unittest/case.py", line 327, in run testMethod()
  File "/vol/ebs-01/jenkins/jobs/edx-feature-branch-tests/workspace/common/lib/xmodule/xmodule/tests/test_export.py", line 114, in test_toy_roundtrip
 self.check_export_roundtrip(DATA_DIR, "toy")
 File "/vol/ebs-01/jenkins/jobs/edx-feature-branch tests/workspace/common/lib/xmodule/xmodule/tests/test_export.py", line 106, in check_export_roundtrip
second_import.modules[course_id][location])
AssertionError: first_import != second_import

Equivalence is tripping on the import systems != which should be irrelevant AND on display_names not being == which may be more important.

[chrisndodge]

ok, looking into it. This is more complicated than I hoped....

[chrisndodge]

@cpennington @wedaly @dmitchell

I think the stop gap here is to remove that new (which I added)

OK with you all?

Waiting to see test results - this passes on my localdev.

[cpennington]

You mean etree.fromstring('<video />') != etree.Element('video')?

[wedaly]

@chrisndodge That makes sense

[cpennington]

Yeah, 👍 to removing the <video ...> tag from the test course for now.