Add test in import code to make sure two overlapping courses aren't imported - #3145
Conversation
There was a problem hiding this comment.
This docstring doesn't really make sense
There was a problem hiding this comment.
The call that breaks if the course overlap happens is 'findone', but I can try and make that clearer
There was a problem hiding this comment.
the quotes would really help! maybe "doesn't import courses that would create a duplicate of the 'findone' course" - that is more explicit.
|
For the coverage, I really don't understand what is happening there. I know I am covering all that code (or at least most of it) with this test, but it doesn't show up in the report. I noticed this for the video caption code as well, it just doesn't seem to be picking up coverage in the common folder. |
|
Hmm yeah I don't know - maybe @nedbat has an idea? |
|
This sounds like a decision @wedaly made about how to measure code run by LMS but outside LMS. |
|
That decision pre-dates me, but there's been an open testeng ticket to fix it for a while: https://edx-wiki.atlassian.net/browse/TE-223 |
|
I made the suggested changes, and I confirmed that adding common/lib to the source list in cms/.coveragerc shows full line coverage.... |
|
Awesome, thanks for that additional investigation Carson. 👍 from me. |
|
Looks good to me, as well! Thanks! 👍 |
Add test in import code to make sure two overlapping courses aren't imported
5 participants
We have been having an issue where the course term changes and gets imported via the command line. This completes successfully and creates two courses that have overlapping items in the modulestore. This results in major problems when one course is deleted as it fires errors with findone finding more than one location as well as deleting portions of both courses when one is partially deleted.
This code corrects this by logging an error when the bad import occurs and skipping the import that would cause the bad modulestore state to occur. This is already a condition that is checked for in studio, but this enforces it at the import level so that it can't occur via the management command or other possible xml import entry points either.
FYI to @pdpinch and @ichuang
@sarina not sure on reviewers