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

feat: add author different author view for studio #30

Merged
merged 7 commits into from
Oct 4, 2023

Conversation

mariajgrimaldi
Copy link
Contributor

@mariajgrimaldi mariajgrimaldi commented Oct 2, 2023

Description

This PR adds an author view that differs from the student view so we better manage what to show Studio authors. Before these changes, what was shown was manage by the same student view which shows the grading interface unless these conditions meet:

def show_staff_grading_interface(self) -> bool:
        """
        Return if current user is staff and not in studio.

        Returns:
            bool: True if current user is instructor and not in studio.
        """
        in_studio_preview = self.scope_ids.user_id is None
        return not in_studio_preview and self.is_course_team

In_studio_preview returns the user ID when we're in the studio preview, so the check returned True when it wasn't supposed to since self.scope_ids.user_id is different than None in Studio preview using edx-platforms' master branch (although it's supposed to return None according to these docs, I'm not sure what's wrong). It was probably working because the user wasn't course staff but an instructor, so the overall check returned false before the changes in this PR.

Also, in_student_view which allows loading the scripts for mindmaps, should indicate whether we're in the student view not if the user has a role like it was before.

How to test

Install the Xblock as usual, then go to the components' Studio preview. It should show the default mindmap. Also, when using a static mindmap it should show the default static mindmap. The Studio edit should work as well.

@mariajgrimaldi mariajgrimaldi requested a review from a team October 2, 2023 21:49
@mariajgrimaldi
Copy link
Contributor Author

Do not review for now. I'll let you know!

@mariajgrimaldi mariajgrimaldi marked this pull request as draft October 2, 2023 22:08
@mariajgrimaldi mariajgrimaldi marked this pull request as ready for review October 4, 2023 14:40
@mariajgrimaldi
Copy link
Contributor Author

@eduNEXT/athena-team: folks! This is ready for your review.

Returns:
bool: True if current user is instructor and not in studio.
"""
in_studio_preview = self.scope_ids.user_id is None
Copy link
Contributor Author

@mariajgrimaldi mariajgrimaldi Oct 4, 2023

Choose a reason for hiding this comment

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

Check yourself: self.scope_ids.user_id when in Studio preview returns the actual user ID, in Studio edit returns None as it should. So this check only works for the studio edit view

@@ -287,6 +287,27 @@ def student_view(self, _context=None) -> Fragment:

return frag

def author_view(self, _context=None) -> Fragment:
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Having an author view allows us to manage what to show in the Studio preview

@johnvente
Copy link
Contributor

johnvente commented Oct 4, 2023

@mariajgrimaldi It's working good for me!

@BryanttV
Copy link
Contributor

BryanttV commented Oct 4, 2023

@mariajgrimaldi, I installed the xblock but currently, I am having the following error in the LMS:

tutor_nightly_dev-lms-1  |   File "/openedx/venv/lib/python3.8/site-packages/storages/backends/s3.py", line 36, in <module>
tutor_nightly_dev-lms-1  |     import s3transfer.constants
tutor_nightly_dev-lms-1  | ModuleNotFoundError: No module named 's3transfer.constants'
tutor_nightly_dev-lms-1  | 
tutor_nightly_dev-lms-1  | During handling of the above exception, another exception occurred:
tutor_nightly_dev-lms-1  | 
tutor_nightly_dev-lms-1  | Traceback (most recent call last):
tutor_nightly_dev-lms-1  |   File "./manage.py", line 103, in <module>
tutor_nightly_dev-lms-1  |     startup.run()
tutor_nightly_dev-lms-1  |   File "/openedx/edx-platform/lms/startup.py", line 20, in run
tutor_nightly_dev-lms-1  |     django.setup()
tutor_nightly_dev-lms-1  |   File "/openedx/venv/lib/python3.8/site-packages/django/__init__.py", line 24, in setup
tutor_nightly_dev-lms-1  |     apps.populate(settings.INSTALLED_APPS)
tutor_nightly_dev-lms-1  |   File "/openedx/venv/lib/python3.8/site-packages/django/apps/registry.py", line 114, in populate
tutor_nightly_dev-lms-1  |     app_config.import_models()
tutor_nightly_dev-lms-1  |   File "/openedx/venv/lib/python3.8/site-packages/django/apps/config.py", line 301, in import_models
tutor_nightly_dev-lms-1  |     self.models_module = import_module(models_module_name)
tutor_nightly_dev-lms-1  |   File "/opt/pyenv/versions/3.8.15/lib/python3.8/importlib/__init__.py", line 127, in import_module
tutor_nightly_dev-lms-1  |     return _bootstrap._gcd_import(name[level:], package, level)
tutor_nightly_dev-lms-1  |   File "<frozen importlib._bootstrap>", line 1014, in _gcd_import
tutor_nightly_dev-lms-1  |   File "<frozen importlib._bootstrap>", line 991, in _find_and_load
tutor_nightly_dev-lms-1  |   File "<frozen importlib._bootstrap>", line 975, in _find_and_load_unlocked
tutor_nightly_dev-lms-1  |   File "<frozen importlib._bootstrap>", line 671, in _load_unlocked
tutor_nightly_dev-lms-1  |   File "<frozen importlib._bootstrap_external>", line 843, in exec_module
tutor_nightly_dev-lms-1  |   File "<frozen importlib._bootstrap>", line 219, in _call_with_frames_removed
tutor_nightly_dev-lms-1  |   File "/openedx/edx-platform/common/djangoapps/student/models/__init__.py", line 4, in <module>
tutor_nightly_dev-lms-1  |     from .course_enrollment import *
tutor_nightly_dev-lms-1  |   File "/openedx/edx-platform/common/djangoapps/student/models/course_enrollment.py", line 46, in <module>
tutor_nightly_dev-lms-1  |     from lms.djangoapps.verify_student.models import SoftwareSecurePhotoVerification
tutor_nightly_dev-lms-1  |   File "/openedx/edx-platform/lms/djangoapps/verify_student/models.py", line 45, in <module>
tutor_nightly_dev-lms-1  |     from openedx.core.storage import get_storage
tutor_nightly_dev-lms-1  |   File "/openedx/edx-platform/openedx/core/storage.py", line 13, in <module>
tutor_nightly_dev-lms-1  |     from storages.backends.s3boto3 import S3Boto3Storage
tutor_nightly_dev-lms-1  |   File "/openedx/venv/lib/python3.8/site-packages/storages/backends/s3boto3.py", line 3, in <module>
tutor_nightly_dev-lms-1  |     from storages.backends.s3 import S3File as S3Boto3StorageFile  # noqa
tutor_nightly_dev-lms-1  |   File "/openedx/venv/lib/python3.8/site-packages/storages/backends/s3.py", line 42, in <module>
tutor_nightly_dev-lms-1  |     raise ImproperlyConfigured("Could not load Boto3's S3 bindings. %s" % e)
tutor_nightly_dev-lms-1  | django.core.exceptions.ImproperlyConfigured: Could not load Boto3's S3 bindings. No module named 's3transfer.constants'

I think we should remove the boto3 package from the requirements, we don't need it for the current version.

@BryanttV
Copy link
Contributor

BryanttV commented Oct 4, 2023

At the moment to solve the previous error I removed from the constraints.txt the boto3 and botocore packages and it works.

@@ -287,6 +287,27 @@ def student_view(self, _context=None) -> Fragment:

return frag

def author_view(self, _context=None) -> Fragment:
Copy link
Contributor

@BryanttV BryanttV Oct 4, 2023

Choose a reason for hiding this comment

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

Adding the author_view seems great, I only have one suggestion: From the student preview in Studio we should disable the ability to edit the mind map, you could only edit it from the edit modal (studio_view).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You're right! Thanks

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done! Thanks again

@mariajgrimaldi
Copy link
Contributor Author

Merging since we all agree it works!

@mariajgrimaldi mariajgrimaldi merged commit 8eb145d into main Oct 4, 2023
@mariajgrimaldi mariajgrimaldi deleted the MJG/add-author-view branch October 4, 2023 19:21
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