Prevent 500 error when staff tries to see discussion as specific student

[amirtds]

NYIF is testing view as specific student functionality on units that are a mix of different XBlock. We applied this fix to PDF and Scorm but seems they have discussion unit with other XBlock and to make the whole subsection visible as a specific student we need to a turn on this flag on built-in discussion XBlock.

[amirtds]

(4) NYIF: server error message caused by XBlocks when viewing as a specific student

[bryanlandia]

@amirtds It looks to me from lms/djangoapps/courseware/masquerade.py comment:

def filter_displayed_blocks(block, unused_view, frag, unused_context):
    """
    A wrapper to only show XBlocks that set `show_in_read_only_mode` when masquerading as a specific user.

    We don't want to modify the state of the user we are masquerading as, so we can't show XBlocks
    that store information outside of the XBlock fields API.
    """

we have to make sure that data associated with the user and the XBlock instance can't be modified while masquerading as the user. Do you know whether it's possible to change discussion posts as the user while masquerading? It seems possible that some of the user/discussion data is not actually stored using the XBlock fields API in this case.

Also, I wonder if we should look at modifying something in the XBlock package instead so that the default is show_in_read_only_mode = True, instead of changing each type of XBlock? Not sure there.

[amirtds]

@bryanlandia Seems here https://github.com/appsembler/edx-platform/blob/appsembler/ficus/master/common/lib/xmodule/xmodule/x_module.py#L283 we can set the default as True , wdyt ?

[bryanlandia]

@amirtds I am having second thoughts about changing this to True globally. The ideal thing would be to keep this as it is, False, and rely on XBlocks to properly reset this to True unless they are storing data outside of the XBlock/modulestore interface.

A more complex but possibly better approach given that XBlock authors haven't/won't do this, would be to access some kind of registry set in the dB or ENV.json that provides an override. WDYT?

We might even want to run this through a quick tech-design-proposal.

[bryanlandia]

We might be able to make the smallest change by putting logic in the SettingsMixin from xblock-utils package, for those XBlocks that do use it.

[bryanlandia]

We decided to just merge this for now but make an Issue/Jira task to follow-up.

[bryanlandia]

approved with idea to look later at more universal fix to this issue across multiple XBlocks