Skip to content

Conversation

@mariajgrimaldi
Copy link
Member

@mariajgrimaldi mariajgrimaldi commented Feb 24, 2022

Description

As part of the Hooks Extension Framework implementation plan, this PR adds a filter before the certificate rendering process starts.

Supporting information

Testing instructions

  1. Install openedx-filters library:
pip install openedx-filters==0.6.2
  1. Implement your pipeline steps in your favorite plugin. We created some as illustration in openedx-filters-samples. We'll be using those in this example.
  2. Install openedx-filters-samples
pip install -e git+https://github.com/eduNEXT/openedx-filters-samples.git@master#egg=openedx_filters_samples
  1. Configure your filters:
    With this configuration, you won't be able to:
  • Render a certificate. Instead, a different template will be rendered
OPEN_EDX_FILTERS_CONFIG = {
     "org.openedx.learning.certificate.render.started.v1": {
            "fail_silently": False,
            "pipeline": [
                "openedx_filters_samples.samples.pipeline.RenderAlternativeCertificate",
            ]
    },
}

And with this one, you'll be able to render dummy custom certificate if it doesn't already exist.

OPEN_EDX_FILTERS_CONFIG = {
    "org.openedx.learning.certificate.render.started.v1": {
        "fail_silently": False,
        "pipeline": [
            "openedx_filters_samples.samples.pipeline.RenderCustomCertificateStep"
        ]
    }
}

If you want to redirect to your custom certificate, then:

OPEN_EDX_FILTERS_CONFIG = {
    "org.openedx.learning.certificate.render.started.v1": {
        "fail_silently": False,
        "pipeline": [
            "openedx_filters_samples.samples.pipeline.RedirectToCustomCertificate"
        ]
    }
}

To override the response, use:

OPEN_EDX_FILTERS_CONFIG = {
    "org.openedx.learning.certificate.render.started.v1": {
        "fail_silently": False,
        "pipeline": [
            "openedx_filters_samples.samples.pipeline.RenderCustomResponseCertificate"
        ]
    }
}

@openedx-webhooks
Copy link

openedx-webhooks commented Feb 24, 2022

Thanks for the pull request, @mariajgrimaldi! I've created BLENDED-1114 to keep track of it in Jira. More details are on the BD-32 project page.

When this pull request is ready, tag your edX technical lead.

@openedx-webhooks openedx-webhooks added blended PR is managed through 2U's blended developmnt program waiting on author PR author needs to resolve review requests, answer questions, fix tests, etc. labels Feb 24, 2022
@mariajgrimaldi mariajgrimaldi force-pushed the MJG/certificate-render-filter branch 4 times, most recently from bd8125d to c515720 Compare March 1, 2022 18:28
@mariajgrimaldi mariajgrimaldi changed the title [WIP] [BD-32] feat: feat: add filter before certificate rendering process starts [BD-32] feat: feat: add filter before certificate rendering process starts Mar 1, 2022
@mariajgrimaldi mariajgrimaldi marked this pull request as ready for review March 1, 2022 19:55
@openedx-webhooks openedx-webhooks added needs triage and removed waiting on author PR author needs to resolve review requests, answer questions, fix tests, etc. labels Mar 1, 2022
@mariajgrimaldi mariajgrimaldi force-pushed the MJG/certificate-render-filter branch from 86b514c to 5ed0ce8 Compare March 2, 2022 12:24
@mariajgrimaldi mariajgrimaldi changed the title [BD-32] feat: feat: add filter before certificate rendering process starts [BD-32] feat: add filter before certificate rendering process starts Mar 2, 2022
@mariajgrimaldi mariajgrimaldi force-pushed the MJG/certificate-render-filter branch from 5ed0ce8 to 345c08b Compare March 2, 2022 13:02
@mariajgrimaldi mariajgrimaldi force-pushed the MJG/certificate-render-filter branch from a8bc92a to e475bae Compare March 2, 2022 16:11
Copy link
Member

Choose a reason for hiding this comment

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

looking at this, do you think that the control should necessary mean that if a filter developer wants to render an alternative template, this must mean that it is invalid?
Or is the case that if the template is valid, then I just answer the filter with a value for custom_template and don't raise anything?

Copy link
Member Author

Choose a reason for hiding this comment

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

is the case that if the template is valid, then I just answer the filter with a value for custom_template and don't raise anything?

That it's what I've been thinking. Should we change the name to RenderInvalidCertificate? Should we have these two possibilities in the other PR(s)? isn't that confusing? 🤔

Copy link
Member

Choose a reason for hiding this comment

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

Probably the most correct would be then RenderAlternativeInvalidCertificate as the exeption and leave the custom-template with no exceptions as the way to render a valid certificate with a different path

Copy link
Member Author

Choose a reason for hiding this comment

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

Done! 🥳

@mariajgrimaldi mariajgrimaldi force-pushed the MJG/certificate-render-filter branch 2 times, most recently from 643d91e to 7b21501 Compare April 7, 2022 16:33
Copy link
Member

@felipemontoya felipemontoya left a comment

Choose a reason for hiding this comment

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

This is good, I think we are ready to merge

@felipemontoya
Copy link
Member

Maybe a final squash

@mariajgrimaldi mariajgrimaldi force-pushed the MJG/certificate-render-filter branch from 7b21501 to e614bf8 Compare April 11, 2022 12:58
@mariajgrimaldi mariajgrimaldi force-pushed the MJG/certificate-render-filter branch 3 times, most recently from f58ff1c to 7e29ca2 Compare May 10, 2022 17:03
@mariajgrimaldi mariajgrimaldi force-pushed the MJG/certificate-render-filter branch from 7e29ca2 to 7f974d1 Compare May 10, 2022 17:59
@mariajgrimaldi mariajgrimaldi merged commit 695d234 into openedx:master May 10, 2022
@openedx-webhooks
Copy link

@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.

@edx-pipeline-bot
Copy link
Contributor

EdX Release Notice: This PR has been deployed to the staging environment in preparation for a release to production.

@edx-pipeline-bot
Copy link
Contributor

EdX Release Notice: This PR has been deployed to the production environment.

github-actions bot added a commit that referenced this pull request Aug 8, 2022
<!--

🌰🌰
🌰🌰🌰🌰         🌰 Note: the Nutmeg master branch has been created.  Please consider whether your change
    🌰🌰🌰🌰     should also be applied to Nutmeg. If so, make another pull request against the
🌰🌰🌰🌰         open-release/nutmeg.master branch, or ping @nedbat for help or questions.
🌰🌰

Please give your pull request a short but descriptive title.
Use conventional commits to separate and summarize commits logically:
https://open-edx-proposals.readthedocs.io/en/latest/oep-0051-bp-conventional-commits.html

Use this template as a guide. Omit sections that don't apply. You may link to information rather than copy it.
More details about the template are at openedx/openedx-proposals#180
(link will be updated when that document merges)
-->

## Description

Backport filters that didn't make it to nutmeg release:

**Add filter before certificate creation starts**

(cherry picked from commit e8fa890)

**Add cohort change filter before moving users from cohorts**

(cherry picked from commit 465e5c0)

**Add filter before certificate rendering process starts**

(cherry picked from commit 7f974d1)

**Add filter before course dashboard rendering process starts**

(cherry picked from commit 895a649)

**Add filter before course about rendering process starts**

(cherry picked from commit ccfa0b4)

**Integrate cohort assignment filter definition to cohort model**

(cherry picked from commit ec69659)

## Supporting information

Refer to the BTR wg github issue for the rationale behind this PR: openedx/wg-build-test-release#187

## Testing instructions

1. Install the needed library release: `openedx-filters==0.7.0`
2. Install the samples library: 
`pip install git+https://github.com/eduNEXT/openedx-filters-samples.git@master#egg=openedx_filters_samples`
3. Then, configure each filter. If you want to test all the filters simultaneously, use this configuration and try to do each operation the filter is related to; the filter sample step will stop the operation.
```
OPEN_EDX_FILTERS_CONFIG = {
    "org.openedx.learning.certificate.creation.requested.v1": {
        "fail_silently": False,
        "pipeline": [
            "openedx_filters_samples.samples.pipeline.StopCertificateCreation"
        ]
    },
    "org.openedx.learning.cohort.change.requested.v1": {
        "fail_silently": False,
        "pipeline": [
            "openedx_filters_samples.samples.pipeline.StopCohortChange"
        ]
    },
    "org.openedx.learning.certificate.render.started.v1": {
        "fail_silently": False,
        "pipeline": [
            "openedx_filters_samples.samples.pipeline.RenderAlternativeCertificate",
        ]
    },
    "org.openedx.learning.dashboard.render.started.v1": {
        "fail_silently": False,
        "pipeline": [
            "openedx_filters_samples.samples.pipeline.RenderAlternativeDashboard",
        ]
    },
    "org.openedx.learning.course_about.render.started.v1": {
        "fail_silently": False,
        "pipeline": [
            "openedx_filters_samples.samples.pipeline.RenderAlternativeCourseAbout",
        ]
    },
    "org.openedx.learning.cohort.assignment.requested.v1": {
        "fail_silently": False,
        "pipeline": [
            "openedx_filters_samples.samples.pipeline.StopCohortAssignment"
        ]
    },
}
```

Please, for detailed instructions on how to test each filter, refer to each of these PR(s):

Filter for certificate creation:
#29949
Filter for cohort change:
#29964
Filter for certificate rendering:
#29976
Filter for dashboard rendering:
#29994
Filter for course about rendering:
#29996
Filter for cohort assignment:
#30431

## Deadline

For the next nutmeg release.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

blended PR is managed through 2U's blended developmnt program merged

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants