Capa problem type filtering via search index (SOL-286)

[e-kolpakov]

Background: This PR is a reimplementation of https://github.com/edx/edx-platform/pull/6346 and contains the following:

• Capa problems search index support
• Machinery to create dedicated index for library contents
• Library capa problem type filtering reimplemented using search

Jira tickets: SOL-286, OC-557
Discussions: discussion on related PR, personal email to @martynjames
Dependencies: Search in multivalue fields
Sandbox URL: LMS, Studio
Partner information: 3rd party-hosted open edX instance, for an edX solutions client.

Testing instructions: no user facing changes should appear. Test instructions similar to https://github.com/edx/edx-platform/pull/6346:

1. Create a library with a couple of Problem XBlocks of different types (e.g. "Checkboxes", "Dropdown", "Multiple Choice")
2. Add Library Content XBlock to a course.
3. Edit Library Content settings. By default "Problem Type" should be set to "Any Type", LMS view should display problems of any type.
4. Set "Problem Type" to one of the problem types in the library (e.g. "Checkboxes", "Dropdown" or "Multiple Choice""). Please note that some problem type templates fall into same problem type (e.g. "Custom Javascript/Python" and "Drag and Drop" fall into "Custom Evaluated Script"). Save, publish, navigate to LMS as a student and ensure only Problems of selected type are shown.
5. Set "Problem Type" to any problem type not in the library (e.g. any except "Checkboxes", "Dropdown" or "Multiple Choice""). Save, publish, note the warning message in Studio, navigate to LMS as a student and ensure no XBlocks are shown.

[openedx-webhooks]

Thanks for the pull request, @e-kolpakov! It looks like you're a member of a company that does contract work for edX. If you're doing this work as part of a paid contract with edX, you should talk to edX about who will review this pull request. If this work is not part of a paid contract with edX, then you should ensure that there is an OSPR issue to track this work in JIRA, so that we don't lose track of your pull request.

To automatically create an OSPR issue for this pull request, just visit this link: http://openedx-webhooks.herokuapp.com/github/process_pr?number=7448&repo=edx%2Fedx-platform

[Kelketek]

@e-kolpakov This PR needs a rebase. This may lead you to where the failures are coming from, if some dependent component was involved.

[Kelketek]

Has a ticket been created for this?

[e-kolpakov]

@Kelketek this TODO is not mine, I just moved it along with the code. Git diff didn't pick the renaming as the file was extensively modified later, but it's not new code. Original author (Marty James) probably created it.

[Kelketek]

Also make sure to look over the quality issues.

[openedx-webhooks]

Thanks for the pull request, @e-kolpakov! It looks like you're a member of a company that does contract work for edX. If you're doing this work as part of a paid contract with edX, you should talk to edX about who will review this pull request. If this work is not part of a paid contract with edX, then you should ensure that there is an OSPR issue to track this work in JIRA, so that we don't lose track of your pull request.

To automatically create an OSPR issue for this pull request, just visit this link: http://openedx-webhooks.herokuapp.com/github/process_pr?repo=edx%2Fedx-platform&number=7448

[e-kolpakov]

test this please

[Kelketek]

@e-kolpakov I got this traceback when trying to load up a library content block's contents:

2015-03-27 11:26:41,549 ERROR 4213 [edx.modulestore] search_index.py:176 - Indexing error encountered, courseware index may be out of date course-v1:Sandbox+Sandbox+whatevz - 'NoneType' object has no attribute 'scope_ids'
Traceback (most recent call last):
  File "/edx/app/edxapp/edx-platform/common/lib/xmodule/xmodule/modulestore/search_index.py", line 167, in add_to_search_index
    remove_index_item_location(location)
  File "/edx/app/edxapp/edx-platform/common/lib/xmodule/xmodule/modulestore/search_index.py", line 163, in remove_index_item_location
    searcher.remove(self.DOCUMENT_TYPE, unicode(self._id_modifier(item.scope_ids.usage_id)))
AttributeError: 'NoneType' object has no attribute 'scope_ids'

Upon refresh, the problem appeared to go away, however. Any idea what may have happened?

[Kelketek]

@e-kolpakov I've tested this manually, and it seems to work, though see my note above. However, for some context on the error, I had also just pressed the delete action on another RCB that was made from a duplicate of that one per my PR. It may have been mid-deletion when that error occurred. If this is something that would only happen on the devstack and not the production stack, it's probably fine. I'm not certain if the devstack uses a different search engine configuration or not, but it would make sense if it could have a conflict like this if it's not running as a constant service.

If all of that is fine, and the tests pass, it has my 👍 , but please confirm that is the case.

[e-kolpakov]

@Kelketek regarding that 'NoneType' object has no attribute 'scope_ids' - I've fixed the issue.

[Kelketek]

@e-kolpakov Excellent. If the tests pass, it's fine with me. :)

[Kelketek]

test this please

[e-kolpakov]

@martynjames this is ready for review - the only remaining problem is failing unit tests (more on that later), so it won't change much.

As of failing unit test - I've tried a couple of setups, but looks like CI build runs cms unit tests with quite special settings: despite being set both in lms/envs/test.py and cms/envs/test.py, neither MOCK_SEARCH_ENGINE nor MOCK_SEARCH_BACKING_FILE can't be imported from settings for some reason, and latest run so far reported settings.DATABASES is improperly configured. Please supply the ENGINE value

However, an entire test suite runs just fine in development; running that specific test suite on CI server succeeds too. So I'm out of ideas on how to fix that. Maybe someone from test engineering could shed some light on this?

@clytwynec do I understand right you are from test engineering team? Could some of you guys take a look at what's the problem with test failures in this branch?

[clytwynec]

common/lib should not be depending on django at all (including any django settings). If you run locally with paver test_lib -l xmodule/modulestore/tests/test_search_index.py:TestCoursewareSearchIndexer this should reproduce.

[e-kolpakov]

@clytwynec maybe, and yet it's widely used both in actual code and tests, to mention only few:

• https://github.com/edx/edx-platform/blob/master/common/lib/xmodule/xmodule/services.py#L4
• https://github.com/edx/edx-platform/blob/master/common/lib/xmodule/xmodule/tests/test_services.py#L9 (these two are actually my code, but it have passed upstream review and didn't received any critics for accessing django settigns. Moreover, it is designed to allow accessing django settings from XBlocks)
• https://github.com/edx/edx-platform/blob/master/common/lib/xmodule/xmodule/tests/test_video.py#L31
• https://github.com/edx/edx-platform/blob/master/common/lib/xmodule/xmodule/capa_base.py#L31
• https://github.com/edx/edx-platform/blob/master/common/lib/xmodule/xmodule/library_tools.py#L4

I could have done without using django.conf.settings and override_settings, but unfortunately this tests excersice feature that uses them under the hood. Thank you for the hint on how to reproduce it locally - at least now I see what's wrong with them.

Could you please confirm one more thing - do I understand right that test_lib command runs without Django settings configured, so it's not possible to test any functionality that depends on using django settings, which in this case is edx-search

[martynjames]

As part of the change over to indexing from a signal - I am moving the indexing code away from common and into cms - there is no longer a need for this to be in place.

Perhaps this is blocking us from continuing properly.

[clytwynec]

@e-kolpakov Correct. test_lib, which runs the tests in common/lib doesn't use any django settings.

It is a known issue that the xmodule lib relies on django in some places, but I don't fully know the background. (See SOL-570 for example)

[e-kolpakov]

@martynjames any time bounds on when to expect that change to happen? Looks like the best I can do here is to wait for that and rebase on top of it. (+ your change is likely larger than mine, so it would be easier to resolve conflicts on my side)

Anyway, I'll try rewriting that test without using settings tomorrow, but I'ma bit pessimistic about that.

@clytwynec thank you very much for the insight.

[martynjames]

I am hoping that this move fixes the tests for me - then I believe the code is ready for review. I will tag you (@e-kolpakov) as a reviewer for this task. So hopefully buy end-of-day here, so you can look in the morning.

[e-kolpakov]

@martynjames ok, looking forward to it. This PR hangs on for quite some time to start to irritate me :)

[openedx-webhooks]

Thanks for the pull request, @e-kolpakov! It looks like you're a member of a company that does contract work for edX. If you're doing this work as part of a paid contract with edX, you should talk to edX about who will review this pull request. If this work is not part of a paid contract with edX, then you should ensure that there is an OSPR issue to track this work in JIRA, so that we don't lose track of your pull request.

To automatically create an OSPR issue for this pull request, just visit this link: http://openedx-webhooks.herokuapp.com/github/process_pr?repo=edx%2Fedx-platform&number=7448