feat: add badges feature #2489

kyrylo-kh · 2024-05-24T18:52:21Z

Description:
This pull request implements Credly integration feature.

Changes:

Creates new badges application.
Extends documentation with badges section.
Extends set of credentials

Related documentation:
See temporarily deployed documentation https://raccoongang.github.io/credentials/badges/index.html

Related PR's:

openedx-webhooks · 2024-05-24T18:52:26Z

Thanks for the pull request, @kyrylo-kh!

What's next?

Please work through the following steps to get your changes ready for engineering review:

🔘 Get product approval

If you haven't already, check this list to see if your contribution needs to go through the product review process.

If it does, you'll need to submit a product proposal for your contribution, and have it reviewed by the Product Working Group.
- This process (including the steps you'll need to take) is documented here.
If it doesn't, simply proceed with the next step.

🔘 Provide context

To help your reviewers and other members of the community understand the purpose and larger context of your changes, feel free to add as much of the following information to the PR description as you can:

Dependencies

This PR must be merged before / after / at the same time as ...
Blockers

This PR is waiting for OEP-1234 to be accepted.
Timeline information

This PR must be merged by XX date because ...
Partner information

This is for a course on edx.org.
Supporting documentation
Relevant Open edX discussion forum threads

🔘 Get a green build

If one or more checks are failing, continue working on your changes until this is no longer the case and your build turns green.

🔘 Let us know that your PR is ready for review:

Who will review my changes?

This repository is currently maintained by @2U-aperture. Tag them in a comment and let them know that your changes are ready for review.

Where can I find more information?

If you'd like to get more details on all aspects of the review process for open source pull requests (OSPRs), check out the following resources:

When can I expect my changes to be merged?

Our goal is to get community contributions seen and reviewed as efficiently as possible.

However, the amount of time that it takes to review and merge a PR can vary significantly based on factors such as:

The size and impact of the changes that it introduces
The need for product review
Maintenance status of the parent repository

💡 As a result it may take up to several weeks or months to complete a review and merge your PR.

justinhynes · 2024-06-21T13:11:02Z

docs/badges/configuration.rst

+Go to the Credly Organization section in the admin panel and create a new item:
+
+1. to set the UUID use your Credly Organization identifier;
+2. to set the authorization token issue one in the Credly Organization's dashboard.


Nit: I think this needs to be reworded. I'm not sure what it's trying to tell me to do.

"to set the authorization token, used to sync the Credly Organization"?

justinhynes · 2024-06-21T13:18:57Z

docs/badges/configuration.rst

+Webhooks are set up on Credly management dashboard as Credly is a main initiator of the syncronization.
+
+You should be able to select an action from the list so that whenever this action happens internally, external platform will be alerted and updated.
+
+Without the syncronization, actions such as:
+
+- Update of the badge template
+- Archivation of the badge template
+
+external platform will not be aware of any changes so it might continue issuing outdated badges.


Nit: Reword suggestion, feel free to ignore.

Webhooks are configured in Credly in order to inform external systems (e.g. the Credentials IDA) when an important action has occurred. You should be able to select an action from the list so that whenever the specified action occurs internally, the external system is alerted. Without this synchronization, the external system will not be notified of any significant changes (e.g. a badge template update, or a badge template has been archived) and may incorrectly issue erroneous or outdated badges.

justinhynes · 2024-06-21T13:21:34Z

docs/badges/configuration.rst

+.. image:: ../_static/images/badges/badges-admin-credly-templates-list.png
+        :alt: Credly badge templates list
+
+For the usage a badge template must be configured and activated first.


Nit: reword suggestion
"In order to issue a badge to a learner, the badge template must be configured and activated."

Also, I find this a little unclear. Does the badge template need to be configured and activated within Credly AND Credentials? Or is this stating that it must be configured and activated in Credly first before Credentials can use it?

So, the Credentials Badges feature operates exclusively on Credly-ready (configured on the Credly side) badge templates.
To use (start their processing) such synced badge templates one must:

set it's requirements

enable it

My suggestion for improvement here could be:

For a badge template to be considered during the processing it must be configured (to have at least 1 requirement) and activated (enabled) first.

justinhynes · 2024-06-21T13:24:27Z

docs/badges/configuration.rst

+.. image:: ../_static/images/badges/badges-admin-template-requirements.png
+        :alt: Credly badge template requirements
+
+A badge template can can have multiple requirements. All badge requirements must be *fulfilled* to earn a badge.


The word can is here twice. We can remove one of them. :)

Nit: Another reword suggestion: "A badge template can have multiple requirements. All badge requirements must be fulfilled before the system will issue a badge to a learner."

justinhynes · 2024-06-21T13:26:16Z

docs/badges/configuration.rst

+Optional configuration (by default each badge requirement is assigned a separate Group).
+
+Allows putting 2 or more badge requirements as a Group.
+Requirements group is fulfilled if any of its requirements is fulfilled.
+
+    "OR" logic is applied inside a Group.


Do you think it would be helpful to link to any Credly docs here?

@justinhynes it's not related to the Credly in any way. This is a completely internal processing logic implementation.

justinhynes · 2024-06-21T13:31:25Z

docs/badges/configuration.rst

+Each badge penalty is *targeted* to 1 or more badge requirements.
+A penalty setup is similar to a badge requirement, but has different effect: it decreases badge progress for a user.
+
+When a penalty has worked all linked badge requirements are "rolled back" (user's progress for such requirements is reset).


Nit: Maybe we can reword this a bit to make it clearer?

When a penalty has been processed (detected? received?), a learner's progress towards a badge is reset.

@andrii-hantkovskyi let's do like this:

When all penalty rules have been applied, a learner's progress towards a badge is reset.

justinhynes · 2024-06-21T13:39:21Z

docs/badges/index.rst

+
+    What is Credly?
+
+    **Credly** is the end-to-end solution for creating, issuing, and managing digital credentials. Thousands of organizations use **Credly** to recognize achievement. 


Nit: Suggest a reword to something like... **Credly** is a end-to-end solution for creating, issuing, and managing digital Credentials. Organizations use **Credly** to recognize their learners' achievements.

I know this is most likely copied and pasted from Credly docs or their website, but it feels like marketing jargon in the Open edX docs.

justinhynes · 2024-06-21T13:41:43Z

docs/badges/processing.rst

+
+    The Badges application implements its extended ``UserCredential`` version (the CredlyBadge) to track external issuing state. Once the Credly badge is successfully issued the **CredlyBadge is updated with its UUID and state**.
+
+Badge revoking is optional. Business needs. Configuration. And why.


Nit: Business needs. Configuration. And why. I'm unsure what this is trying to tell me.

Was this sentence intended to be a TODO that should have been removed?

@andrii-hantkovskyi let's just remove the Business needs. Configuration. And why. part.

justinhynes · 2024-06-21T13:48:39Z

docs/badges/processing.rst

+
+.. _Configuration: configuration.html
+
+When system revokes a badge, the status of a badge will change from awarded to revoked in the admin panel (admim/badges/credly_badges) as the revoke process is synced with external platform. 


Nit: Is the admin panel here talking about Credly? Or the Credentials IDA's Django admin panel? I'm also not sure what "system" we're talking about? Is this the Credentials IDA? or Credly?

I think we can reword this to make it a bit clearer:
"When a learner's badge is revoked by Credly(?), the Credentials IDA will be notified and will update it's internal records. The status of the badge will change from awarded to revoked upon successful revocation."

Co-authored-by: Andrii <andrii.hantkovskyi@raccoongang.com>

andrii-hantkovskyi · 2024-07-10T11:02:41Z

@justinhynes Hi, just wanted to thank you for your remarks. We have gone through all of them and given appropriate responses where no fixes needed. All relevant problems have been fixed. Kindly look at the code again when time allows you to do so :)

* feat: [ACI-1031] Upgrade requirements to upstream version * fix: [ACI-1031] Fix event-bus-redis version * fix: [ACI-1031] Remove extra changes

justinhynes · 2024-07-17T12:32:31Z

Hi @andrii-hantkovskyi. Apologies, I missed your last update until today.

I will block out some time to re-review. Thanks for the heads up :)

justinhynes

This looks good to me. Thanks for the clarifications and updates. Lastly, a huge thanks to the team for 1) wanting us to to take a look in the first place, and 2) all the patience for us to review (and re-review).

openedx-webhooks · 2024-07-22T14:44:35Z

@kyrylo-kh 🎉 Your pull request was merged! Please take a moment to answer a two question survey so we can improve your experience in the future.

This reverts commit da22842.

* feat: add badges feature * chore: update dummy translations * refactor: squash migrations + text cleanup + resolve conflict * docs: documentation update --------- Co-authored-by: Andrii <andrii.hantkovskyi@raccoongang.com> Co-authored-by: Andrii Hantkovskyi <andrii.hantkovskyi@github.com> Co-authored-by: KyryloKireiev <kirillkireev888@gmail.com> Co-authored-by: wowkalucky <wowkalucky@gmail.com> Co-authored-by: GlugovGrGlib <glugov1998@gmail.com>

feat: add badges feature

f28bf31

openedx-webhooks added the open-source-contribution PR author is not from Axim or 2U label May 24, 2024

kyrylo-kh added 5 commits May 27, 2024 13:14

chore: update dummy translations

143fd9b

fix: merge conflicted migrations

09ecff8

test: fix penalties tests

58fa7b8

test: fix tests for credentials api

6fd2782

style: fix imports order

3366597

kyrylo-kh force-pushed the aci.upstream branch from 6f27b2f to 3366597 Compare May 27, 2024 14:38

style: format code

8613e01

kyrylo-kh force-pushed the aci.upstream branch from cb2a081 to 8613e01 Compare May 27, 2024 14:55

andrii-hantkovskyi force-pushed the aci.upstream branch 19 times, most recently from 5e0ae15 to 5a4911d Compare May 31, 2024 13:59

justinhynes reviewed Jun 21, 2024

View reviewed changes

kyrylo-kh and others added 2 commits July 2, 2024 14:44

test: add tests for issuers, api_cliend and commands

890c799

refactor: code cleanup & docs rewording (#180)

31da465

Co-authored-by: Andrii <andrii.hantkovskyi@raccoongang.com>

GlugovGrGlib requested a review from justinhynes July 8, 2024 19:02

deborahgu added the waiting for eng review PR is ready for review. Review and merge it, or suggest changes. label Jul 12, 2024

feat: [ACI-1031] Upgrade requirements to upstream version (#181)

7b75760

* feat: [ACI-1031] Upgrade requirements to upstream version * fix: [ACI-1031] Fix event-bus-redis version * fix: [ACI-1031] Remove extra changes

justinhynes approved these changes Jul 18, 2024

View reviewed changes

KyryloKireiev and others added 6 commits July 19, 2024 12:23

fix: [ACI-1031] Upgrade requirements

a4f06e4

fix: [ACI-1031] Resolve migrations conflict

23052ed

style: [ACI-1031] Try to fix linters

26e56ff

style: [ACI-1031] Try to fix linters v2

dea54f2

Merge remote-tracking branch 'EDX/master' into aci.upstream

748a02c

fix: regenerate requirements with python 3.8

6e8944c

GlugovGrGlib merged commit da22842 into openedx:master Jul 22, 2024
11 checks passed

GlugovGrGlib added a commit that referenced this pull request Jul 22, 2024

Revert "feat: add badge issuance with Credly backend (#2489)"

becd531

This reverts commit da22842.

GlugovGrGlib mentioned this pull request Jul 22, 2024

Revert "feat: add badge issuance with Credly backend (#2489)" #2526

Closed

GlugovGrGlib removed the waiting for eng review PR is ready for review. Review and merge it, or suggest changes. label Jul 22, 2024

KyryloKireiev mentioned this pull request Jul 23, 2024

feat: Backport adding Badge issuance to Redwood #2528

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: add badges feature #2489

feat: add badges feature #2489

kyrylo-kh commented May 24, 2024 •

edited

Loading

openedx-webhooks commented May 24, 2024 •

edited

Loading

justinhynes Jun 21, 2024

justinhynes Jun 21, 2024

justinhynes Jun 21, 2024 •

edited

Loading

wowkalucky Jul 5, 2024

justinhynes Jun 21, 2024

justinhynes Jun 21, 2024

wowkalucky Jul 5, 2024

justinhynes Jun 21, 2024

wowkalucky Jul 5, 2024

justinhynes Jun 21, 2024

justinhynes Jun 21, 2024

justinhynes Jun 21, 2024

wowkalucky Jul 5, 2024

justinhynes Jun 21, 2024

andrii-hantkovskyi commented Jul 10, 2024

justinhynes commented Jul 17, 2024

justinhynes left a comment

openedx-webhooks commented Jul 22, 2024


		What is Credly?

		Credly is the end-to-end solution for creating, issuing, and managing digital credentials. Thousands of organizations use Credly to recognize achievement.


		The Badges application implements its extended ``UserCredential`` version (the CredlyBadge) to track external issuing state. Once the Credly badge is successfully issued the CredlyBadge is updated with its UUID and state.

		Badge revoking is optional. Business needs. Configuration. And why.


		.. _Configuration: configuration.html

		When system revokes a badge, the status of a badge will change from awarded to revoked in the admin panel (admim/badges/credly_badges) as the revoke process is synced with external platform.

feat: add badges feature #2489

feat: add badges feature #2489

Conversation

kyrylo-kh commented May 24, 2024 • edited Loading

openedx-webhooks commented May 24, 2024 • edited Loading

What's next?

🔘 Get product approval

🔘 Provide context

🔘 Get a green build

🔘 Let us know that your PR is ready for review:

Who will review my changes?

Where can I find more information?

When can I expect my changes to be merged?

Choose a reason for hiding this comment

Choose a reason for hiding this comment

justinhynes Jun 21, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

andrii-hantkovskyi commented Jul 10, 2024

justinhynes commented Jul 17, 2024

justinhynes left a comment

Choose a reason for hiding this comment

openedx-webhooks commented Jul 22, 2024

kyrylo-kh commented May 24, 2024 •

edited

Loading

openedx-webhooks commented May 24, 2024 •

edited

Loading

justinhynes Jun 21, 2024 •

edited

Loading