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: setup instructor dashboard filter #2

Merged
merged 14 commits into from
Oct 19, 2023

Conversation

Ian2012
Copy link
Contributor

@Ian2012 Ian2012 commented Oct 4, 2023

Description

This PR setup the instructor dashboard filter integration:

image

Testing instructions

name: settings
version: 0.1.0
patches:
      OPEN_EDX_FILTERS_CONFIG = {
        "org.openedx.learning.instructor.dashboard.render.started.v1": {
          "fail_silently": False,
          "pipeline": [
            "platform_plugin_superset.extensions.filters.AddSupersetTab",
          ]
        },
      }

Merge checklist:
Check off if complete or not applicable:

  • Version bumped
  • Changelog record added
  • Documentation updated (not only docstrings)
  • Fixup commits are squashed away
  • Unit tests added/updated
  • Manual testing instructions provided
  • Noted any: Concerns, dependencies, migration issues, deadlines, tickets

@Ian2012 Ian2012 force-pushed the cag/instructor-filter-integration branch from a4968f5 to e89ccbe Compare October 13, 2023 20:55
@mariajgrimaldi
Copy link
Contributor

Could you add the Athena team to the collaborators list? So we can tag them as reviewers

platform_plugin_superset/extensions/filters.py Outdated Show resolved Hide resolved
platform_plugin_superset/extensions/filters.py Outdated Show resolved Hide resolved
platform_plugin_superset/extensions/filters.py Outdated Show resolved Hide resolved
platform_plugin_superset/extensions/filters.py Outdated Show resolved Hide resolved
@Ian2012 Ian2012 requested a review from a team October 17, 2023 13:53
@Ian2012
Copy link
Contributor Author

Ian2012 commented Oct 17, 2023

@mariajgrimaldi @johnvente can you re review?

Copy link

@johnvente johnvente left a comment

Choose a reason for hiding this comment

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

LGTM! I've left you a last comment but is okay for me

@Ian2012
Copy link
Contributor Author

Ian2012 commented Oct 18, 2023

@mariajgrimaldi have in mind that an important part of the functionality of this PR will be refactored on #3

Copy link
Contributor

@mariajgrimaldi mariajgrimaldi left a comment

Choose a reason for hiding this comment

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

Just a comment left to address.

@Ian2012 Ian2012 merged commit ed0aca7 into main Oct 19, 2023
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