Skip to content

[BD-32] feat: integrate cohort assignment filter definition#30431

Merged
mariajgrimaldi merged 1 commit intoopenedx:masterfrom
eduNEXT:MJG/cohort-assignment-filter
Jun 13, 2022
Merged

[BD-32] feat: integrate cohort assignment filter definition#30431
mariajgrimaldi merged 1 commit intoopenedx:masterfrom
eduNEXT:MJG/cohort-assignment-filter

Conversation

@mariajgrimaldi
Copy link
Member

@mariajgrimaldi mariajgrimaldi commented May 18, 2022 •
edited
Loading

Description

This PR integrates the Cohort Assignment filter definition meant to be triggered when assigning a user into a cohort.

Supporting information

Testing instructions

  1. Install openedx-filters library:
pip install openedx-filters==0.7.0
  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 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:
  • Change user from cohort org.openedx.learning.cohort.assignment.requested.v1
OPEN_EDX_FILTERS_CONFIG = {
  "org.openedx.learning.cohort.assignment.requested.v1": {
          "fail_silently": False,
          "pipeline": [
              "openedx_filters_samples.samples.pipeline.StopCohortAssignment"
          ]
      },
}

Deadline

Before nutmeg official release (this PR it's intended to be backported along with the other filters that didn't make it)

@openedx-webhooks openedx-webhooks added needs triage open-source-contribution PR author is not from Axim or 2U labels May 18, 2022
@openedx-webhooks
Copy link

openedx-webhooks commented May 18, 2022 •
edited
Loading

Thanks for the pull request, @mariajgrimaldi! I've created BLENDED-1236 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.

@mariajgrimaldi mariajgrimaldi changed the title feat: integrate cohort assignment filter definition [WIP] [BD-32] feat: integrate cohort assignment filter definition May 23, 2022
@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. open-source-contribution PR author is not from Axim or 2U and removed open-source-contribution PR author is not from Axim or 2U needs triage blended PR is managed through 2U's blended developmnt program labels May 23, 2022
@mariajgrimaldi
Copy link
Member Author

mariajgrimaldi commented May 26, 2022

How it looks stopping the cohort assignment:
cohort-assignment-not-allowed

@mariajgrimaldi mariajgrimaldi changed the title [WIP] [BD-32] feat: integrate cohort assignment filter definition [BD-32] feat: integrate cohort assignment filter definition May 30, 2022
@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 May 30, 2022
felipemontoya
felipemontoya approved these changes Jun 8, 2022
View reviewed changes
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.

Sorry @mariajgrimaldi this took me a long time to review. I tested it running the filter against my own pipeline step and made sure that all connections were working as expected.

I even made a filter that only lets certain users be assigned to the cohort and not the others to test the partial error message. All superb.

image

I think this is ready. Just a squash before merging would be in order.

@mariajgrimaldi mariajgrimaldi force-pushed the MJG/cohort-assignment-filter branch 2 times, most recently from 2a29e32 to 0b65f6b Compare June 13, 2022 15:02
@mariajgrimaldi mariajgrimaldi force-pushed the MJG/cohort-assignment-filter branch from 0b65f6b to ec69659 Compare June 13, 2022 15:28
@mariajgrimaldi
Copy link
Member Author

mariajgrimaldi commented Jun 13, 2022 •
edited
Loading

I'll be merging this now! 😄 @felipemontoya

@mariajgrimaldi mariajgrimaldi merged commit 721b635 into openedx:master Jun 13, 2022
@openedx-webhooks
Copy link

openedx-webhooks commented Jun 13, 2022

@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-pipeline-bot commented Jun 13, 2022

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-pipeline-bot commented Jun 13, 2022

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

@edx-pipeline-bot
Copy link
Contributor

edx-pipeline-bot commented Jun 13, 2022

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-pipeline-bot commented Jun 14, 2022

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

@mariajgrimaldi mariajgrimaldi mentioned this pull request Jun 14, 2022
Merged
github-actions bot added a commit that referenced this pull request Aug 8, 2022
@github-actions
merge(#30583): [BD-32] feat: Open edX Filters backport
1d61805 
<!--

🌰🌰
🌰🌰🌰🌰         🌰 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

Reviewers

@felipemontoya felipemontoya felipemontoya approved these changes

Assignees

No one assigned

Labels

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

Projects

No open projects
2U Blended Project
Archived in project

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@mariajgrimaldi @openedx-webhooks @edx-pipeline-bot @felipemontoya