Upgrade Script

[bradenmacdonald]

When testing the upgrade path from mentoring v1 to problem builder (mentoring v2), I found a few bugs in the upgrade script:

• if <tip> elements had a width or height, they wouldn't work
• Warnings from the XML upgrade script were not displayed to the user
• Did not convert the actual block type from mentoring to problem-builder

I also added a check for multiple blocks that have the same url_name, since that should not happen but it is the case on some courses I worked with.

Note:

• This is based on PR Rename mentoring to problem builder to allow simultaneous installation #1 so ignore the many changes coming from that
• This is irrelevant to new courses - it only affects upgrading an old course.

[e-kolpakov]

@bradenmacdonald heads up - parse_xml is going to be removed: open-craft/xblock-mentoring#16

[bradenmacdonald]

@e-kolpakov Yep, that's great. Thanks :)

[e-kolpakov]

@bradenmacdonald does it work fine with mentorings with comments? It should at least convert them right removing comments, but preserving comments would be ideal (but it might be non-trivial)

[e-kolpakov]

@bradenmacdonald nevermind, I've checked it out and it indeed works just fine with comments

[e-kolpakov]

@bradenmacdonald also, there's a dangling statement in xm_changes.py:187 - it was introduced earlier, but now is a good enough moment to remove it.

[e-kolpakov]

@bradenmacdonald 👍 conditional exception is fixed.

[Kelketek]

Same nit @e-kolpakov expressed earlier about other strings being unicode-- this one isn't.

[bradenmacdonald]

Yeah, there are actually a lot of non-unicode strings in this file that contain unicode characters. My bad. Because they're just being treated as bytes and getting piped directly to stdout, it hasn't caused a problem, but you're right that they should be marked as Unicode.

[Kelketek]

Looks like @e-kolpakov found all the good stuff already, save for one other unicode nit. 👍 conditional on you fixing the issue he mentioned with string formatting.

[bradenmacdonald]

Rebased and addressed all your comments. I added in code to automatically fix the url_names if a --force argument is given. Thanks @e-kolpakov and @Kelketek. I'll merge tomorrow based on your tentative thumbs up, but if you can review again, that would be ideal.

Note: extra commits are showing up because I should be merging to develop, not master. I will fix that when merging.

[e-kolpakov]

@bradenmacdonald I've added a PR to include import-export fix. It targets this branch though, as I tried to simplify your work of merging this all, so please, take a look at #5 and merge.

[bradenmacdonald]

Merged this to develop via ba65ce0, merging Eugeny's import fix as #7.