[Product Pull Request] feat: allow instructors to manage certificates

[jmakowski1123]

For Contributing Author:

This is the Primary Product Ticket for the following community contribution: New feature that will allow course instructors to manage certificates.

Checklist prior to undergoing Product Review:

The following information is required in order for Product Managers to be able to review your pull request:

• Explanation of the problem being solved
• Description of how users will be impacted, and which users will be impacted
• Screenshots or video showing the functionality or fix, before and after
• Reproduction steps and/or testing steps

Only if necessary:

• If necessary, links to corresponding configuration changes
• If necessary, links to corresponding enablement changes, particularly waffle/toggle status details

Related PRs

openedx/edx-platform#31265

---

For Product Manager doing the review:

What criteria should be analyzed from Product to approve a PR?

• The problem being solved by the feature or fix is clear.
• There is clarity on how the change or fix will impact the end user.
• It is clear that the change will not negatively impact users or other areas of the platform.
• The change is implemented comprehensively.
• Any changes to UI use the current, standard Paragon Design System: https://paragon-openedx.netlify.app/

[github-actions]

Thanks for your submission, @openedx/open-edx-project-managers will review shortly.

[jmakowski1123]

Product information from the original PR:

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

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

  CERTIFICATES_INSTRUCTOR_GENERATION: true
  CUSTOM_CERTIFICATE_TEMPLATES_ENABLED: true

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

[Ian2012]

Hi @jmakowski1123, I'm part of the eduNEXT team, you can find answers to your inquiries below:

Does the decision to extend course instructors this new role still lie with superusers and/or course staff?

There should be a feature flag to enable this functionality at the instance level, allowing superusers to have control of it on their installation. (we don't recommend granting course staff that control over the course)

Are there unintended risks, such as the Instructor getting access to additional permissions that they shouldn't?

No, there are such no risks as the Open edX permissions modified are very granular and straightforward. The permissions changed are:

• ENABLE_CERTIFICATE_GENERATION
• GENERATE_CERTIFICATE_EXCEPTIONS
• GENERATE_BULK_CERTIFICATE_EXCEPTIONS
• GENERATE_EXAMPLE_CERTIFICATES
• START_CERTIFICATE_GENERATION
• START_CERTIFICATE_REGENERATION
• CERTIFICATE_EXCEPTION_VIEW
• CERTIFICATE_INVALIDATION_VIEW

How would this risk be mitigated?

Related tests have been modified as this functionality is expected.

Can this role expansion be managed at both the course and instance levels?

Not at the moment, but it makes sense that this feature can be enabled at the instance level, but I'm not sure if it's really necessary to have control over it at the course level. @jmakowski1123 what do you think?

cc @dcoa @mariajgrimaldi

[Ian2012]

Reviewing the product review undergo list:

Description of how users will be impacted, and which users will be impacted.

At the current state, it will affect course instructors as they will be able to have control over the certificate, which makes sense since usually instructors have more contact with the students and can solve certificate inquiries and issues easily.

[jmakowski1123]

Thanks very much for the additional information, @Ian2012 ! I agree, I think it makes sense for this to implemented at the Instance level (rather than the course level).

I'll pass this ticket over to Claire Zhang for final review, who is the product manager for certificate feature enhancements.

[mphilbrick211]

Hi @jmakowski1123 @czhang0912! Just following up on this. Any updates?

[czhang0912]

@mphilbrick211 As Kelly mentioned, we would like to hold any new features on certificate now.

[felipemontoya]

@czhang0912 could you give us any more information on this?

• Why are we holding features related to certificates?
• Is there a new/different product that instances using openedx should be using for certs where this feature is no longer necessary?
• Any information on how long the hold would last?

[mphilbrick211]

Hi @czhang0912 - any update on this? Also flagging Felipe's questions above. Thanks!

[mphilbrick211]

Hi @czhang0912! Friendly ping on this :)

[mphilbrick211]

My apologies, @czhang0912 - I remembered this was on hold. Is that still the case? Should these PRs be closed for now?

[mphilbrick211]

@czhang0912 could you give us any more information on this?

• Why are we holding features related to certificates?
• Is there a new/different product that instances using openedx should be using for certs where this feature is no longer necessary?
• Any information on how long the hold would last?

@czhang0912 - friendly ping on Felipe's questions :)

[jmakowski1123]

Claire is no longer the right PM for this review.

@cablaa77 , flagging this one for you because it's an expansion of a permission to course instructors. However, it only touches the LMS side, which I'm not sure if you are, or have, considered in your current R&P proposal. It's being implemented by adding a permission to a role, which seems ok to me in the schema of R&P evolution. Do you see any red flags here?

[jmakowski1123]

Hi @jmakowski1123, I'm part of the eduNEXT team, you can find answers to your inquiries below:

Does the decision to extend course instructors this new role still lie with superusers and/or course staff?

There should be a feature flag to enable this functionality at the instance level, allowing superusers to have control of it on their installation. (we don't recommend granting course staff that control over the course)

Are there unintended risks, such as the Instructor getting access to additional permissions that they shouldn't?

No, there are such no risks as the Open edX permissions modified are very granular and straightforward. The permissions changed are:

• ENABLE_CERTIFICATE_GENERATION
• GENERATE_CERTIFICATE_EXCEPTIONS
• GENERATE_BULK_CERTIFICATE_EXCEPTIONS
• GENERATE_EXAMPLE_CERTIFICATES
• START_CERTIFICATE_GENERATION
• START_CERTIFICATE_REGENERATION
• CERTIFICATE_EXCEPTION_VIEW
• CERTIFICATE_INVALIDATION_VIEW

How would this risk be mitigated?

Related tests have been modified as this functionality is expected.

Can this role expansion be managed at both the course and instance levels?

Not at the moment, but it makes sense that this feature can be enabled at the instance level, but I'm not sure if it's really necessary to have control over it at the course level. @jmakowski1123 what do you think?

cc @dcoa @mariajgrimaldi

I think Instance level is the way to go. Course level is too fine-grained.

[jmakowski1123]

I don't see any issues with this PR. It will be compatible within the framework of the new R&P schema, simply adding permissions to a role.

Let's implement for now at the instance level, as this removes the complexity of decision-making at the course level. If control needs to be more fine-grained at the course level, let's see if that bubbles up once users are using it.

Down the road, we may want to make the decision to have this permission set by default. However, we haven't done extensive analysis yet of the permissions needed for LMS roles, so that could come later. Implementing behind a flag now is fine.

@cablaa77 if no concerns on your end (let us know by Dec. 20), I consider this approved.

[Asespinel]

Hi @jmakowski1123 do you have a new date for the product review? So we' can make sure to align our efforts accordingly for the next sprints in case any change is needed.

[ali-hugo]

@jmakowski1123 I noticed this is still tagged as "needs product review", but from the thread it seems like it should perhaps be moved to "product review done".

---

EDIT:
I updated the status. Please move it back if necessary. 🙂

[itsjeyd]

@jmakowski1123 @mphilbrick211 It looks like this is done; the PR for it (openedx/edx-platform#31265) has been merged. Can we close this ticket and move it to Shipped on the roadmap?