Check enable_ccx value before binding module to student

[pdpinch]

Per @cpennington

make sure that _lineage isn't called if there are no override providers that are enabled for a course...

The main thing would be to make https://github.com/edx/edx-platform/blob/master/lms/djangoapps/courseware/module_render.py#L518 conditional on whether any of the field override providers are enabled

@amir-qayyum-khan this would build on your work on https://github.com/edx/edx-platform/pull/8207

[pdpinch]

@amir-qayyum-khan here is testing suggestion from @cpennington

I think the appropriate test for per-course setting for overrides would be to take the general structure of the test in #8260, and then have a test that is parameterized on whether ccx is enabled for the course or not (and toggle that independently of whether it's enabled globally)

If you have any questions you can ask them here or ping me or @bdero

FYI @cewing @cguardia

[cewing]

Looking at this, I'm not sure what good it would do. The implementation of`OverrideFieldData.wrap() returns the unwrapped field data if there are no provider classes anyway.

I guess what would be better would be to update the .wrap() method there to check if any of the globally-enabled field override providers (which are declared via settings.py) are available for a course. If the field data passed into .wrap() is aware of the course_id, then I suppose that could be used to check the settings for that course and only use the field override providers for overrides that are enabled for the course.

Would looking up the course setting for enabling a provider be something that ends up costing more performance than it saves?

[cpennington]

Yes, checking course features inside .wrap() could work, or passing a filtered list of providers in when wrap is called.

[cpennington]

In module_render, we already have the course instantiated, so checking whether a feature is enabled on it or not is relatively cheap, compared to walking the parent tree when looking for any field in the block.

[cewing]

@cpennington: good, so we might update 'wrap' to take the course and then filter the providers in the context of that call. Or I suppose we could do the filtering outside of the call and then not bother with wrap, but that seems like leaking an implementation detail to me.

[pdpinch]

Looking at https://github.com/edx/edx-platform/pull/8207 you should be able to just test course.enable_ccx

[cguardia]

An initial attempt has been committed to the check-ccx-enabled branch:

https://github.com/jazkarta/edx-platform/commit/92a5909a99c1f2aec8bb56e05d777e3e1a2bbc51

Note that there is a second wrap() call inside module_render.py:

https://github.com/edx/edx-platform/blob/master/lms/djangoapps/courseware/module_render.py#L717

@cpennington said we already have the course instantiated, but I can only see the course_id, since I think we can't assume that the descriptor is a CourseDescriptor. That means an additional call to get_course_by_id.

[cguardia]

Made a WIP PR for easier discussion:

https://github.com/jazkarta/edx-platform/pull/106

[pdpinch]

This was addressed in edx#8519