[TC_LEARNER_39] Notes failed to load by default

[cmltaWt0]

Notes failed to load by default.

Steps:

• Enable Notes in a Course Advanced settings
• Open Course as a Student
• Check Notes tab
• Open any Unit in a subsection
• Select text to note
• Save the note
• Check the Note tab
• Return to the Course Subsection
• Check that the prviously noted text is selected and can be seen

Status: FAILED

Important notes:

• the issue is reproducible but it works fine for the first time
• need to walk through the units
• to fix the issue for a current Unit - just click Hide Notes and than Show notes again (literally reboot the PC and try it again)

Links

• Problem with notes visibility feature frontend-app-learning#1005

[jfavellar90]

Hi @cmltaWt0, thanks for the report. According to my investigations, this is a problem related to a discrepancy between the management of the notes in the MFE and the Iframe serving an xblock unit content.

How it's decided if the student notes must be rendered when a courseware unit page is loaded?

As I already mentioned, the unit content is loaded through an Iframe, for instance, the content here uses the Iframe loaded from here

There's a decorator in https://github.com/openedx/edx-platform/blob/open-release/olive.master/lms/djangoapps/edxnotes/decorators.py which determines if the edxnotes visibility is enabled or not (check the specific line) and based on that value, notes are rendered on the page or not. This visibility value depends on a student preference stored in the table courseware_xmodulestudentinfofield. The preference name is "edxnotes_visibility". If the preference is not present for the currently logged-in user, the notes visibility is enabled by default.

How the frontend-app-learning MFE determines if the notes are visible or not?

The MFE does not determine if the notes should be highlighted the first time a unit page is loaded, however, it stores the notes visibility value, which is taken from a call to the courseware API (https://github.com/openedx/frontend-app-learning/blob/open-release/olive.master/src/courseware/data/api.js#L128). This API returns the values for notes in the format below:

notes:
  enabled: true
  visible: true

And this API always returns the visible field with a true value, ignoring the "edxnotes_visibility" preference of the user. This generates an incongruence between the values in the MFE and the Iframe rendering the block unit. For instance, there are cases where the notes are hidden by user preference in the iframe, but the button "Hide Notes" is displayed in the MFE, because, for the MFE, the notes visibility is enabled (value taken from the courseware API).

How to resolve the problem

For me, the solution is to fix the MFE so it gets the notes visibility value properly, taking into account the edxnotes_visibility user preference. I'm not sure how to proceed in this case, for now, I'll open an issue directly in the frontend-app-learning MFE repo.

Additional notes

• This issue is also present in Nutmeg.

[regisb]

Thank you for this great analysis Jhony.

And this API always returns the visible field with a true value, ignoring the "edxnotes_visibility" preference of the user.

So if I understand you correctly, the bug comes from the LMS which does not send the right reply to the API call?

[jfavellar90]

@regisb for me it seems that way. When you call the LMS API (for instance https://olive.demo.overhang.io/api/courseware/course/course-v1:edX+DemoX+Demo_Course) you'll always get notes visibility in true, no matter the user notes visibility preference. This is the view handling such API calls. Additional to this, a small change in the frontend-app-learning MFE is required to toggle the notes visibility for a user properly.

[mariajgrimaldi]

label: olive testing

[jfavellar90]

I decided to publish this issue in the frontend-app-learning repo, I consider the problem resides there, even when the solution could involve a change in edx-platform repo.

[jalondonot]

@arbrandes, do we have any news from the Frontend WG regarding this issue?

[arbrandes]

@jalondonot, no news: noboby's picked the issue to work on, yet.

[jalondonot]

I don't know if this is a high-priority task, @arbrandes. What do you think? should we highlight this issue in the FWG to encourage someone to pick it up and start working on it?

[jfavellar90]

@jalondonot I don't think at all it's a high-priority issue. This is a legacy issue coming from Nutmeg and last time I checked, it was also present in edx.org (the impact on the platform functionality is low). That said, we can validate if the issue is still happening, and as you suggest, add it to the FWG backlog to be taken and fixed in the next couple of months, preferably before Palma's cutoff. @arbrandes would you agree with this approach?

[arbrandes]

Totally on board, though I couldn't guarantee this would actually be picked up by Palm. I could slap a "Palm.1" milestone on it, though, which would automatically make it higher priority.