Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enable instructors to grab a learner's session #60

Merged
merged 1 commit into from
Sep 26, 2024

Conversation

fghaas
Copy link
Contributor

@fghaas fghaas commented Mar 7, 2018

For a while now, we've wanted to provide instructors with a way to take over, or share, a learner's session. We might just be able to do so by simply adding the show_in_read_only_mode attribute to the XBlock.

show_in_read_only_mode is an attribute that is False by default, and means that when an instructor is masquerading (that is, using the LMS with "View this course as: Specific learner" option), the XBlock is replaced by the text "This type of component cannot be shown while viewing the course as a specific student" (in lms/djangoapps/courseware/masquerade.py):

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.
    """
    if getattr(block, 'show_in_read_only_mode', False):
        return frag
    return Fragment(
        _(u'This type of component cannot be shown while viewing the course as a specific student.')
    )

Now the first thing we'd need to thus do is to set show_in_read_only_mode to True, so that the XBlock is rendered even when masquerading.

I think (or I hope) that by pointing the instructor's browser at the same Guacamole session as the learner, they'll both be able to interact with the learner's terminal. The devil may very much be in the details. For example, the learner's session might be cut off the moment the instructor connects to it.

Even if so, that may be something that is worth accepting, because the instructor could temporarily take over the learner's session, and when they're done, the learner could just refresh the page. Later on, we could figure out how to actually make the session shareable (if that's even possible).

In addition, making the attribute a read-only property on the XBlock is just a first stab — arguably, it would be smart to make this configurable for course authors.

@codecov
Copy link

codecov bot commented Mar 7, 2018

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 94.82%. Comparing base (ed39fff) to head (b204745).
Report is 1 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master      #60   +/-   ##
=======================================
  Coverage   94.82%   94.82%           
=======================================
  Files          24       24           
  Lines        1990     1991    +1     
=======================================
+ Hits         1887     1888    +1     
  Misses        103      103           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@arbrandes
Copy link
Contributor

I've just tested it, and it's way cool! The instructor, when viewing the course as a student, gets to access the stack as one would expect. Staff debug info even gives the correct user info, including stack name.

However, as you feared, it is not possible to access the same session simultaneously. Or at least, not with xrdp, which is what I tested. The last to get in gets access transparently, the first gets a "whoops aborted" message.

Still, not bad. I still have to test suspend/resume behavior, which can't be done reliably on a devstack. guac-dev to the rescue. :)

@fghaas
Copy link
Contributor Author

fghaas commented Mar 7, 2018

Just to mention it, Staff Debug has always given the correct username and stack name; that part has nothing to do what show_in_read_only_mode is set to.

I think what we really need here is exposing connection errors in a more user-friendly fashion. "Whoops! Aborted. See Logs" is pretty terrible UX, if we're being honest. So if we can make this something like "connection to stack lost, this may be due to network connectivity issues", that would be a step in the right direction. Better still if we can word it in a way that tells people to refresh the page except if they have just asked an instructor for help (for in that latter case, they'd probably get a connection but kick the instructor out).

By the way: screen sharing is something that is supported in Guac, if I read https://glyptodon.org/jira/browse/GUAC-844 correctly. So to me this looks like a "doable but perhaps complicated" kind of thing.

@fghaas
Copy link
Contributor Author

fghaas commented Aug 19, 2019

Rebased on current master.

@fghaas
Copy link
Contributor Author

fghaas commented Sep 27, 2023

Note, https://glyptodon.org/jira/browse/GUAC-844 is no longer available but https://issues.apache.org/jira/browse/GUACAMOLE-13 is, and it indicates that the modifications GUAC-844 was meant to track have been merged, and https://guacamole.apache.org/doc/gug/using-guacamole.html#sharing-the-connection now covers connection sharing for the Guacamole Tomcat application.

Copy link
Contributor Author

@fghaas fghaas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have just one tiny note.

README.md Show resolved Hide resolved
@fghaas fghaas changed the title WIP: Enable instructors to grab a learner's session Enable instructors to grab a learner's session Sep 25, 2024
@fghaas fghaas requested a review from mrtmm September 25, 2024 14:24
@fghaas
Copy link
Contributor Author

fghaas commented Sep 25, 2024

@mrtmm, I have now squashed the changes into one commit, and added a Co-authored-by tag for you. Does that work?

mrtmm
mrtmm previously approved these changes Sep 25, 2024
Copy link
Contributor

@mrtmm mrtmm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me, thanks!

@fghaas
Copy link
Contributor Author

fghaas commented Sep 26, 2024

FYI: I'm trying to unbreak the report stage of the GitHub Actions workflow, before merging this one. See #292.

show_in_read_only_mode is an attribute that is False by default, and
means that when an instructor is masquerading (that is, using the
LMS with "View this course as: Specific learner" option), the
XBlock is replaced by the text "This type of component cannot be shown
while viewing the course as a specific student"
(in lms/djangoapps/courseware/masquerade.py).

In our case, however, we do want an instructor to be able to
conveniently take over another learner's session.

Thus, set show_in_read_only_mode to True, so that the XBlock is
rendered even when masquerading.

Co-authored-by: Maari Tamm <[email protected]>
@fghaas fghaas merged commit b204745 into hastexo:master Sep 26, 2024
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants