Remove Discussion and Advanced Components Interface From Libraries (SOL-120) (WIP)

[Kelketek]

• Background: This PR removes the ability to add Advanced or Discussion modules to a content library, restricting the number of supported components to start with.
• Jira tickets: Implements SOL-120
• Discussions: Architecture discussed extensively on the wiki and in meetings, then the revised proposal was presented to the Arch Council on Oct. 21 and given thumbs up.
• Dependencies: https://github.com/edx/edx-platform/pull/6046 (merged, in feature branch https://github.com/edx/edx-platform/commits/content-libraries)
• Sandbox URL: Studio: http://sandbox3.opencraft.com:18010/ - LMS: http://sandbox3.opencraft.com/
• Internal code reviews PRs: Remove Discussion and Advanced Modules from Content Libraries open-craft/openedx-platform#20
• Partner information: 3rd party-hosted open edX instance, for an edX solutions client.

[openedx-webhooks]

Thanks for the pull request, @Kelketek! I've created OSPR-273 to keep track of it in JIRA. JIRA is a place for product owners to prioritize feature reviews by the engineering development teams.

Feel free to add as much of the following information to the ticket:

• supporting documentation
• edx-code email threads
• timeline information ('this must be merged by XX date', and why that is)
• partner information ('this is a course on edx.org')
• any other information that can help Product understand the context for the PR

All technical communication about the code itself will still be done via the Github pull request interface. As a reminder, our process documentation is here.

[e-kolpakov]

No newline at the end of file. :) I'm going to become pylint myself soon :)

[Kelketek]

That does it. I need to find the setting in my IDE to automate this.

[antoviaque]

@Kelketek Created a sandbox based on the current contents of the branch:

• Studio: http://sandbox3.opencraft.com:18010/
• LMS: http://sandbox3.opencraft.com/

[singingwolfboy]

This comment is misleading -- this test is actually testing that the UI to add a discussion block to a library doesn't appear. If you want to write a test to check that we cannot add a discussion block to a library, the test should make an HTTP request attempting to add the discussion block, and should verify that the HTTP response had a status code in the 4XX range and that the database hasn't changed.

[Kelketek]

@singingwolfboy @antoviaque Fixes have been pushed. I've decided that, even though, from my understanding, this limitation is intended to be temporary, I've gone ahead and made it more robust in the latest push, with better tests. You may want to make a new sandbox, though, visually, no changes have been made in the latest revision.

[Kelketek]

@cahrens @explorerleslie @antoviaque This should be ready for review. Jenkins is failing the quality test, but its failures are unreproducible locally, and are dealing with an unrelated file. Is this a known issue?

[antoviaque]

Updated the sandbox with the latest code: http://sandbox3.opencraft.com:18010/

[cahrens]

It would be helpful to include the requested category in the error message.

And if this message could be shown to the user, translate it.

[cahrens]

👍 Just a couple small suggestions (I don't need to re-review).

[Kelketek]

Commenting, just because it seems Jenkins doesn't run again if I don't after a rebase, but it should be good to go. Implemented your suggestions.

[antoviaque]

@Kelketek Jenkins can take some time to be triggered - it's now running FYI.

[antoviaque]

@Kelketek Besides the quality failure from earlier, there seem to be two unit tests failing on the build - before merging this PR it would be good to get those passing. @bradenmacdonald I saw you have a similar issue on https://github.com/edx/edx-platform/pull/6155 with https://jenkins.testeng.edx.org/job/edx-all-tests-auto-pr/19742/SHARD=1,TEST_SUITE=unit/console -- any idea what is causing it? The similarity could point to an issue with the content-library branch, or a flappy test?

[Kelketek]

@antoviaque I saw this some time ago back when the branches hadn't been approved and rebased yet. It's got something to do with the way that settings are being passed along for the tests-- that GITHUB_URL constant or whatever isn't being handled correctly. If it's affecting both of us, it may be something that slipped through the last rebase. It's just a bit odd that it wasn't caught.

[bradenmacdonald]

@antoviaque @Kelketek I believe that issue is either a flaky test or a problem with Jenkins. If you look here you can see that most of the builds on 6155 have been passing. Also, all the tests are passing just fine on my devstack.

Edit: Just looked, and I can see other unrelated PRs with the same test failure (example).

[antoviaque]

@Kelketek @bradenmacdonald Thanks for looking into it - sounds good then, merging.