feat: add filter design to modify the instructor dashboard render #96

mariajgrimaldi · 2023-06-13T20:53:54Z

Description:
This PR adds a new filter to modify the instructor dashboard rendering process. For example: modify the section tabs of the instructor context --that specifies which tabs to render, adding a brand new one defined in a different plugin. The use case we're currently testing is to add a new tab to the instructor dashboard, which renders management information about an Xblock.

Installation instructions:
In case you want to test the use case shown here, you'll need a couple of stuff:

Install the LimeSurvey Xblock in the LMS/CMS services: git+https:/eduNEXT/xblock-limesurvey.git@MJG/instructor-tab
Install this branch in the LMS/CMS services: git+https:/openedx/openedx-filters.git@MJG/instructor-tab-filters
Change your edx-platform branch to MJG/instructor-dashboard-filter
Restart services

Testing instructions:

Add the Open edX Filters configuration to your environment:

OPEN_EDX_FILTERS_CONFIG = {
    "org.openedx.learning.instructor.dashboard.render.started.v1": {
        "fail_silently": False,
        "pipeline": [
            "limesurvey.extensions.filters.AddInstructorLimesurveyTab",
        ]
    },
}

Add the LimeSurvey xblock as a component of your course, you can use dummy values for the Studio configuration. The student view doesn't really matter for these tests.
Go to the instructor dashboard with the proper permissions (as an instructor of the course). You'll see:

Reviewers:

@felipemontoya

Merge checklist:

Post merge:

Create a tag
Check new version is pushed to PyPI after tag-triggered build is
finished.
Delete working branch (if not needed anymore)

openedx-webhooks · 2023-06-13T20:53:59Z

Thanks for the pull request, @mariajgrimaldi!

As a core committer in this repo, you can merge this once the pull request is approved per the core committer reviewer requirements and according to the agreement with your edX Champion.

mariajgrimaldi · 2023-06-13T22:06:25Z

openedx_filters/learning/filters.py

+ class RedirectToPage(OpenEdxFilterException):
+ """
+ Custom class used to stop the instructor dashboard process.
+ """
+
+ def __init__(self, message, redirect_to=""):
+ """
+ Override init that defines specific arguments used in the instructor dashboard render process.
+
+ Arguments:
+ message: error message for the exception.
+ redirect_to: URL to redirect to.
+ """
+ super().__init__(message, redirect_to=redirect_to)
+
+ class RenderInvalidDashboard(OpenEdxFilterException):
+ """
+ Custom class used to stop the instructor dashboard render process.
+ """
+
+ def __init__(self, message, dashboard_template="", template_context=None):
+ """
+ Override init that defines specific arguments used in the instructor dashboard render process.
+
+ Arguments:
+ message: error message for the exception.
+ dashboard_template: template path rendered instead.
+ template_context: context used to the new dashboard_template.
+ """
+ super().__init__(
+ message,
+ dashboard_template=dashboard_template,
+ template_context=template_context,
+ )
+
+ class RenderCustomResponse(OpenEdxFilterException):
+ """
+ Custom class used to stop the instructor dashboard rendering process.
+ """
+
+ def __init__(self, message, response=None):
+ """
+ Override init that defines specific arguments used in the instructor dashboard render process.
+
+ Arguments:
+ message: error message for the exception.
+ response: custom response which will be returned by the dashboard view.
+ """
+ super().__init__(
+ message,
+ response=response,


[curious about other opinions] I wonder if these exceptions are needed in this case, is this a question we should ask every time we add a filter that interacts with templates? Or should we always add the same exceptions for them?

I like the predictability of always having the same exceptions. As a dev I'd like to have an idea of what I can expect of a filter that is *.render.started.v1.

In this case I personally would rather try to use such filter for extending the instructor dashboard capabilities and not to replace them altogether, but the filter should be agnostic to that. I can imagine all sort of uses for this.

Good! This is a good time to standardize all these types of things. I'll make a mental note to add it to the ongoing docs work :)

mariajgrimaldi · 2023-06-13T22:07:47Z

openedx_filters/learning/filters.py

+ """
+ super().__init__(
+ message,
+ dashboard_template=dashboard_template,


[seeking advice] I used dashboard_template instead of instructor_dashboard_template, is it confusing?

I also think it can be easily confused with the student dashboard. However instructor_dashboard_template seems like unnecessary long.

We could use the same compromise the platform used and go with instructor_template. I think it conveys the most critical part of the info.

Thanks! That's what I needed.

mariajgrimaldi · 2023-06-13T22:08:42Z

openedx_filters/learning/filters.py

+ Custom class used to create instructor dashboard filters and its custom methods.
+ """
+
+ filter_type = "org.openedx.learning.instructor.dashboard.render.started.v1"


[seeking advice] should we use: org.openedx.learning.instructor_dashboard.render.started.v1 instead?

We have used "." for separating "course.enrollment" in the past. I think here it's the same kind of separation and thus "." is correct.

Same here for the docs! Thanks for the help :)

mariajgrimaldi · 2023-06-14T16:28:21Z

cc @felipemontoya when you have a chance ;)

felipemontoya

I reviewed this together with https:/openedx/edx-platform/pull/32448/files and I think this filter works as expected.

There is a nit about naming one of the variables but once that is fixed this looks like its good to be merged. I leave my approval already.

felipemontoya · 2023-06-27T23:15:09Z

openedx_filters/learning/filters.py

+ Custom class used to create instructor dashboard filters and its custom methods.
+ """
+
+ filter_type = "org.openedx.learning.instructor.dashboard.render.started.v1"


We have used "." for separating "course.enrollment" in the past. I think here it's the same kind of separation and thus "." is correct.

felipemontoya · 2023-06-27T23:17:33Z

openedx_filters/learning/filters.py

+ """
+ super().__init__(
+ message,
+ dashboard_template=dashboard_template,


I also think it can be easily confused with the student dashboard. However instructor_dashboard_template seems like unnecessary long.

We could use the same compromise the platform used and go with instructor_template. I think it conveys the most critical part of the info.

felipemontoya · 2023-06-27T23:22:44Z

openedx_filters/learning/filters.py

+ class RedirectToPage(OpenEdxFilterException):
+ """
+ Custom class used to stop the instructor dashboard process.
+ """
+
+ def __init__(self, message, redirect_to=""):
+ """
+ Override init that defines specific arguments used in the instructor dashboard render process.
+
+ Arguments:
+ message: error message for the exception.
+ redirect_to: URL to redirect to.
+ """
+ super().__init__(message, redirect_to=redirect_to)
+
+ class RenderInvalidDashboard(OpenEdxFilterException):
+ """
+ Custom class used to stop the instructor dashboard render process.
+ """
+
+ def __init__(self, message, dashboard_template="", template_context=None):
+ """
+ Override init that defines specific arguments used in the instructor dashboard render process.
+
+ Arguments:
+ message: error message for the exception.
+ dashboard_template: template path rendered instead.
+ template_context: context used to the new dashboard_template.
+ """
+ super().__init__(
+ message,
+ dashboard_template=dashboard_template,
+ template_context=template_context,
+ )
+
+ class RenderCustomResponse(OpenEdxFilterException):
+ """
+ Custom class used to stop the instructor dashboard rendering process.
+ """
+
+ def __init__(self, message, response=None):
+ """
+ Override init that defines specific arguments used in the instructor dashboard render process.
+
+ Arguments:
+ message: error message for the exception.
+ response: custom response which will be returned by the dashboard view.
+ """
+ super().__init__(
+ message,
+ response=response,


I like the predictability of always having the same exceptions. As a dev I'd like to have an idea of what I can expect of a filter that is *.render.started.v1.

In this case I personally would rather try to use such filter for extending the instructor dashboard capabilities and not to replace them altogether, but the filter should be agnostic to that. I can imagine all sort of uses for this.

mariajgrimaldi

Thanks for the review @felipemontoya, I'll merge this now.

openedx-webhooks · 2023-07-18T15:44:49Z

@mariajgrimaldi 🎉 Your pull request was merged! Please take a moment to answer a two question survey so we can improve your experience in the future.

feat: add filter design to modify the instructor dashboard render

5021001

openedx-webhooks added core committer open-source-contribution PR author is not from Axim or 2U labels Jun 13, 2023

mariajgrimaldi marked this pull request as ready for review June 13, 2023 22:05

mariajgrimaldi requested a review from a team as a code owner June 13, 2023 22:05

mariajgrimaldi commented Jun 13, 2023

View reviewed changes

mariajgrimaldi requested review from felipemontoya and removed request for a team June 16, 2023 16:11

felipemontoya approved these changes Jun 27, 2023

View reviewed changes

Ian2012 mentioned this pull request Jul 13, 2023

WIP: get data with superset API to the LMS eduNEXT/aspects-superset-aside#2

Closed

refactor: address PR reviews

f56956d

mariajgrimaldi force-pushed the MJG/instructor-tab-filters branch from 131db66 to f56956d Compare July 18, 2023 15:38

docs: update docs for new version

4f49035

mariajgrimaldi commented Jul 18, 2023

View reviewed changes

mariajgrimaldi merged commit c982791 into main Jul 18, 2023
6 checks passed

mariajgrimaldi deleted the MJG/instructor-tab-filters branch July 18, 2023 15:44

mariajgrimaldi mentioned this pull request Aug 29, 2023

feat: add instructor dashboard filter integration openedx/edx-platform#32448

Merged

mariajgrimaldi mentioned this pull request Sep 7, 2023

feat: add instructor dashboard filter integration openedx/edx-platform#33196

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: add filter design to modify the instructor dashboard render #96

feat: add filter design to modify the instructor dashboard render #96

mariajgrimaldi commented Jun 13, 2023 •

edited

Loading

openedx-webhooks commented Jun 13, 2023 •

edited

Loading

mariajgrimaldi Jun 13, 2023

felipemontoya Jun 27, 2023

mariajgrimaldi Jul 18, 2023

mariajgrimaldi Jun 13, 2023

felipemontoya Jun 27, 2023

mariajgrimaldi Jul 18, 2023

mariajgrimaldi Jun 13, 2023

felipemontoya Jun 27, 2023

mariajgrimaldi Jul 18, 2023

mariajgrimaldi commented Jun 14, 2023

felipemontoya left a comment

felipemontoya Jun 27, 2023

felipemontoya Jun 27, 2023

felipemontoya Jun 27, 2023

mariajgrimaldi left a comment

openedx-webhooks commented Jul 18, 2023

feat: add filter design to modify the instructor dashboard render #96

feat: add filter design to modify the instructor dashboard render #96

Conversation

mariajgrimaldi commented Jun 13, 2023 • edited Loading

openedx-webhooks commented Jun 13, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mariajgrimaldi commented Jun 14, 2023

felipemontoya left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mariajgrimaldi left a comment

Choose a reason for hiding this comment

openedx-webhooks commented Jul 18, 2023

mariajgrimaldi commented Jun 13, 2023 •

edited

Loading

openedx-webhooks commented Jun 13, 2023 •

edited

Loading