feat: allow instructors to manage certificates #31265

dcoa · 2022-11-07T17:27:15Z

Description

Currently only super admin and staff are able to configure the certificates for the course.

This PR allows course instructors (admin course permissions) to manage certificates through all the actions available in the tab instructor > certificates, in this way, each instructor will be able to manage their certificates, which allows more granularity on the permissions granted to users. The staff permission is a somewhat dangerous permission, since it grants access to a wide variety of functions, with this change the instructors (who already manage the users, cohorts, and others) will also be able to manage the certificates.

This feature is used for eduNEXT due to client requests and we think is useful for the community, we put this into consideration for product reviewers (@santiagosuarezedunext @jmakowski1123 ).

Testing instructions

Enable these FEATURES in your environment (to activate all certificate options):

  CERTIFICATES_INSTRUCTOR_GENERATION: true
  CUSTOM_CERTIFICATE_TEMPLATES_ENABLED: true

Create a course with a certificate.
Create a new user without special permissions (superuser, staff).
Add the user to the course team as Admin (i.e. Instructor) via the instructor > membership tab.
The certificate tab should be visible and all the certificate operations should work (certificate generation, whitelist, self generation, etc)

C.C @MaferMazu @Alec4r @felipemontoya @Ian2012

openedx-webhooks · 2022-11-07T17:27:20Z

Thanks for the pull request, @dcoa! Please note that it may take us up to several weeks or months to complete a review and merge your PR.

Feel free to add as much of the following information to the ticket as you can:

supporting documentation
Open edX discussion forum threads
timeline information ("this must be merged by XX date", and why that is)
partner information ("this is a course on edx.org")
any other information that can help Product understand the context for the PR

All technical communication about the code itself will be done via the GitHub pull request interface. As a reminder, our process documentation is here.

Please let us know once your PR is ready for our review and all tests are green.

MaferMazu · 2022-11-25T06:23:19Z

I think this is a good feature to take into consideration, several of our clients request it. What do you think @jmakowski1123 @santiagosuarezedunext?

mphilbrick211 · 2022-12-16T17:46:23Z

@jmakowski1123 @ProductRyan - flagging for you as it looks like this is awaiting Product approval.

mphilbrick211 · 2023-01-23T23:38:58Z

Putting this back on the radar @jmakowski1123 and @ProductRyan.

mphilbrick211 · 2023-02-07T20:12:12Z

Hi @dcoa - just checking in on this to see if the tests will be run? Looks like they may need to be re-run.

dcoa · 2023-02-10T09:33:25Z

Hi @mphilbrick211 yes it's okay re-run the test

e0d · 2023-02-14T19:49:46Z

@mphilbrick211 tests running.

jmakowski1123 · 2023-02-14T20:08:09Z

Hi @dcoa ! Thanks for the contribution! As this is a feature that will impact end-users, it will need to undergo product review. To start that process, I've created a Feature Ticket, which contains a checklist of the information needed to help the reviewing product manager evaluate and make an approval decision.

We'd like to get some more information from you regarding the potential impact of this permissions change for staff users, which as you note, can be a dangerous set of permissions. For example, does the decision to extend course instructors this new role still lie with superusers and/or course staff? Are there unintended risks, such as the Instructor getting access to additional permissions that they shouldn't? How would this risk be mitigated? Can this role expansion be managed at both the course and instance level? If you could provide additional information along these lines directly in the Feature Ticket, that will enable us to start the product review! We'll use the Feature Ticket to manage all product-related communications and decisions as well. Thanks! cc @mphilbrick211

mphilbrick211 · 2023-04-05T16:47:14Z

This PR is on hold per 4/5/23 update from product roadmap ticket.

mphilbrick211 · 2023-04-25T17:44:17Z

Hi @dcoa! While this item is in product review, would you mind re-running the tests on this pull request? We've had some new ones pop up (Shellcheck) that need to be run. Thanks!

MaferMazu · 2023-05-16T02:19:06Z

Hello @mphilbrick211, any update with this PR?

mphilbrick211 · 2023-05-18T01:25:50Z

Hello @mphilbrick211, any update with this PR?

Hi @MaferMazu! Please check out this thread. This is on hold for now., though I've pinged Product again to see if they have an update.

mphilbrick211 · 2024-02-21T16:48:15Z

Hi @openedx/2u-aperture @jsnwesson! Could you please review / merge this for us? Product review is complete. Thanks!

deborahgu · 2024-02-21T19:29:53Z

added to our prioritization queue, @mphilbrick211, hopefully we will get to it next week. In the meantime if someone gets a chance to look at the conflicts that would be great.

mphilbrick211 · 2024-02-22T17:48:29Z

added to our prioritization queue, @mphilbrick211, hopefully we will get to it next week. In the meantime if someone gets a chance to look at the conflicts that would be great.

Thank you, @deborahgu!

@dcoa @MaferMazu would it be possible to resolve the branch conflicts so Aperture can review?

MaferMazu · 2024-02-23T02:13:03Z

We resolve the conflicts, but we need to look at this comment: openedx/platform-roadmap#230 (comment) because I'm not sure we are fulfilling that.

justinhynes · 2024-02-26T15:05:26Z

lms/djangoapps/instructor/permissions.py

@@ -15,6 +15,11 @@
 ENABLE_CERTIFICATE_GENERATION = 'instructor.enable_certificate_generation'
 GENERATE_CERTIFICATE_EXCEPTIONS = 'instructor.generate_certificate_exceptions'
 GENERATE_BULK_CERTIFICATE_EXCEPTIONS = 'instructor.generate_bulk_certificate_exceptions'
+GENERATE_EXAMPLE_CERTIFICATES = 'instructor.generate_example_certificates'


Is GENERATE_EXAMPLE_CERTIFICATES still viable these days? I thought generating example certificates were related to PDF certificates, and this functionality has been gutted from edx-platform for quite some time.

justinhynes · 2024-02-26T15:09:34Z

@dcoa I may be missing it, but according to the discussion here: openedx/platform-roadmap#230, my assumption was that there would be a waffle or configuration switch to enable this on a deployment.

I don't think we want this functionality at 2U. This PR appears like it would be enabled and give access to this functionality in the instructor dashboard by default to all members of the course team.

Can it be gated and configurable?

justinhynes

Is there any configuration/setting that gates this configuration for Open edX instances where we don't want course teams to have access to this functionality?

justinhynes · 2024-02-26T15:12:31Z

lms/static/certificates/sass/_components.scss

@@ -499,7 +499,6 @@
    }

    // hide the fancy
-    .accomplishment-signatories .signatory-signature,


What is the purpose of this change? Was this supposed to be included?

jmakowski1123 · 2024-02-27T19:27:45Z

@dcoa I may be missing it, but according to the discussion here: openedx/platform-roadmap#230, my assumption was that there would be a waffle or configuration switch to enable this on a deployment.

I don't think we want this functionality at 2U. This PR appears like it would be enabled and give access to this functionality in the instructor dashboard by default to all members of the course team.

Can it be gated and configurable?

Yes, the requirement in the product review was to implement behind a feature flag. Maybe it was simply overlooked and can be added now?

dcoa · 2024-02-27T20:31:44Z

Hi @justinhynes,

As we explained here, we haven't applied the change that PWG asked for but we will work on that.

We resolve the conflicts, but we need to look at this comment: openedx/platform-roadmap#230 (comment) because I'm not sure we are fulfilling that.

dcoa · 2024-03-13T00:03:38Z

hi, @justinhynes, I create the flag.

Define permissions using bridgekeeper for views related to certificates. Add support to instructor certificates generation. If user has instructor permissions over a course, the staff permissions are also granted.

deborahgu · 2024-03-14T18:55:44Z

thanks, @dcoa, we'll review now that the flag exists.

justinhynes · 2024-03-19T14:25:23Z

I plan on taking another look at this today, thanks for the heads up @dcoa :)

justinhynes

This looks good! I'll plan on merging this today too.

openedx-webhooks · 2024-03-20T11:46:18Z

@dcoa 🎉 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 · 2024-03-20T12:54:38Z

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

edx-pipeline-bot · 2024-03-20T13:14:50Z

2U Release Notice: This PR has been deployed to the edX production environment.

openedx-webhooks added the open-source-contribution PR author is not from Axim or 2U label Nov 7, 2022

dcoa force-pushed the dcoa/intructor-certificates-generation branch from 305196d to 4d4483d Compare November 8, 2022 15:24

dcoa marked this pull request as ready for review November 8, 2022 15:31

jmakowski1123 added the product review PR requires product review before merging label Nov 8, 2022

openedx-webhooks removed the product review PR requires product review before merging label Nov 11, 2022

felipemontoya added the product review PR requires product review before merging label Nov 11, 2022

openedx-webhooks removed the product review PR requires product review before merging label Nov 13, 2022

jmakowski1123 added the product review PR requires product review before merging label Dec 12, 2022

dcoa force-pushed the dcoa/intructor-certificates-generation branch from 4d4483d to 58bde8b Compare February 10, 2023 09:31

mphilbrick211 added the needs test run Author's first PR to this repository, awaiting test authorization from Axim label Feb 10, 2023

jmakowski1123 mentioned this pull request Feb 14, 2023

[Product Pull Request] feat: allow instructors to manage certificates openedx/platform-roadmap#230

Open

6 tasks

e0d removed the needs test run Author's first PR to this repository, awaiting test authorization from Axim label Feb 16, 2023

mphilbrick211 requested a review from a team February 21, 2024 16:47

mphilbrick211 added the waiting for eng review PR is ready for review. Review and merge it, or suggest changes. label Feb 21, 2024

deborahgu added the needs maintainer attention Issue or PR specifically needs the attention of the maintainer. label Feb 21, 2024

justinhynes reviewed Feb 26, 2024

View reviewed changes

justinhynes requested changes Feb 26, 2024

View reviewed changes

justinhynes reviewed Feb 26, 2024

View reviewed changes

hurtstotouchfire added waiting on author PR author needs to resolve review requests, answer questions, fix tests, etc. and removed waiting for eng review PR is ready for review. Review and merge it, or suggest changes. labels Feb 27, 2024

dcoa force-pushed the dcoa/intructor-certificates-generation branch 2 times, most recently from 634aec2 to 4594dfc Compare March 11, 2024 03:02

dcoa requested a review from justinhynes March 13, 2024 00:03

arbrandes self-requested a review March 13, 2024 14:12

jsnwesson removed the waiting on author PR author needs to resolve review requests, answer questions, fix tests, etc. label Mar 13, 2024

dcoa force-pushed the dcoa/intructor-certificates-generation branch from 2d81e4a to 7a8cbf7 Compare March 13, 2024 21:32

Squirrel18 and others added 6 commits March 14, 2024 11:40

feat: Add context to certificate webview.

7bb3817

Define permissions using bridgekeeper for views related to certificates. Add support to instructor certificates generation. If user has instructor permissions over a course, the staff permissions are also granted.

fix: rename local parameters to fix pylint redefined-outer-name alert

850b4f9

feat: create a setting to enable certificate panel for instructors

38814f6

fix: remove unnecessary permisions

b557e69

fix: update persmissions and testing

1d2608e

style: fix pylint issues

417d2bc

dcoa force-pushed the dcoa/intructor-certificates-generation branch from 7a8cbf7 to 417d2bc Compare March 14, 2024 00:40

justinhynes approved these changes Mar 20, 2024

View reviewed changes

justinhynes merged commit 490a3d4 into openedx:master Mar 20, 2024
67 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: allow instructors to manage certificates #31265

feat: allow instructors to manage certificates #31265

dcoa commented Nov 7, 2022 •

edited

Loading

openedx-webhooks commented Nov 7, 2022 •

edited

Loading

MaferMazu commented Nov 25, 2022

mphilbrick211 commented Dec 16, 2022

mphilbrick211 commented Jan 23, 2023

mphilbrick211 commented Feb 7, 2023

dcoa commented Feb 10, 2023

e0d commented Feb 14, 2023

jmakowski1123 commented Feb 14, 2023

mphilbrick211 commented Apr 5, 2023

mphilbrick211 commented Apr 25, 2023

MaferMazu commented May 16, 2023

mphilbrick211 commented May 18, 2023

mphilbrick211 commented Feb 21, 2024

deborahgu commented Feb 21, 2024

mphilbrick211 commented Feb 22, 2024

MaferMazu commented Feb 23, 2024

justinhynes Feb 26, 2024

justinhynes commented Feb 26, 2024

justinhynes left a comment

justinhynes Feb 26, 2024

jmakowski1123 commented Feb 27, 2024

dcoa commented Feb 27, 2024

dcoa commented Mar 13, 2024

deborahgu commented Mar 14, 2024

justinhynes commented Mar 19, 2024

justinhynes left a comment

openedx-webhooks commented Mar 20, 2024

edx-pipeline-bot commented Mar 20, 2024

edx-pipeline-bot commented Mar 20, 2024

feat: allow instructors to manage certificates #31265

feat: allow instructors to manage certificates #31265

Conversation

dcoa commented Nov 7, 2022 • edited Loading

Description

Testing instructions

openedx-webhooks commented Nov 7, 2022 • edited Loading

MaferMazu commented Nov 25, 2022

mphilbrick211 commented Dec 16, 2022

mphilbrick211 commented Jan 23, 2023

mphilbrick211 commented Feb 7, 2023

dcoa commented Feb 10, 2023

e0d commented Feb 14, 2023

jmakowski1123 commented Feb 14, 2023

mphilbrick211 commented Apr 5, 2023

mphilbrick211 commented Apr 25, 2023

MaferMazu commented May 16, 2023

mphilbrick211 commented May 18, 2023

mphilbrick211 commented Feb 21, 2024

deborahgu commented Feb 21, 2024

mphilbrick211 commented Feb 22, 2024

MaferMazu commented Feb 23, 2024

justinhynes Feb 26, 2024

Choose a reason for hiding this comment

justinhynes commented Feb 26, 2024

justinhynes left a comment

Choose a reason for hiding this comment

justinhynes Feb 26, 2024

Choose a reason for hiding this comment

jmakowski1123 commented Feb 27, 2024

dcoa commented Feb 27, 2024

dcoa commented Mar 13, 2024

deborahgu commented Mar 14, 2024

justinhynes commented Mar 19, 2024

justinhynes left a comment

Choose a reason for hiding this comment

openedx-webhooks commented Mar 20, 2024

edx-pipeline-bot commented Mar 20, 2024

edx-pipeline-bot commented Mar 20, 2024

dcoa commented Nov 7, 2022 •

edited

Loading

openedx-webhooks commented Nov 7, 2022 •

edited

Loading