Useless code path in import_from_xml causing errors removed

[mat-peterson]

@dianakhuang @sarina @nasthagiri

It appears as if a useless code path was causing @carsongee errors while importing course from command line.

@carsongee Can you verify that this fixes your issue?

[mat-peterson]

https://edx-wiki.atlassian.net/browse/LMS-2849

[carsongee]

I just tested that this is allowing imports to occur. I am a little concerned about allowing imports of courses with a different run causing mongo modulestore key issues. Is that all resolved higher up with the opaque keys changes? e.g. Will importing edx/1/Fall and then importing edx/1/Spring create two courses that will then have duplicate shared entries in the Mongo database?

[mat-peterson]

My understanding is that opaque keys have resolved that higher up. @sarina @dianakhuang Confirm?

[sarina]

Eesh that's a better question for @dmitchell or @nasthagiri

[carsongee]

https://github.com/edx/edx-platform/pull/3145 is the original PR to which I am concerned about a regression occurring if that helps.

[dmitchell]

We moved that defensive code down into the MongoModuleStore which cannot handle the id clash since split and xml don't care. As long as there are no other users of create_new_course which expect it to not overwrite the existing course: 👍

[nasthagiri]

Since the semantics of the create_new_course parameter has now changed, you need to update its description in the docstring above. Consider even changing its name to something like create_course_if_not_present.

[mat-peterson]

I changed create_new_course to create_new_course_if_not_present

@nasthagiri is right about it never reaching the exception so I got rid of it.

[mat-peterson]

A test failed testing exactly what the exception was intended to do. I'm going to add it back in.

[mat-peterson]

This latest commit includes the exception.

[nasthagiri]

@mmprandom Can you update the docstring for this method since the parameter name and semantics have changed? Thanks.

[mat-peterson]

Updated.

[mat-peterson]

@dmitchell @nasthagiri All tests have passed! Any concerns?

[nasthagiri]

Please update the docstring to the following:

If True, then a new course is created if it doesn't already exist. The check for existing courses is case-insensitive.

[nasthagiri]

👍 once the docstring is updated once more. thanks!

[dmitchell]

👍 admitting lack of diligence and hoping not to get bitten