[WIP] feat: [ACI-240] Badges documentation #48

wowkalucky · 2023-12-13T10:45:02Z

See compiled live the Badges documentation updates for convenience.

GlugovGrGlib · 2023-12-13T13:32:42Z

docs/badges/configuration.rst

+.. code-block:: sh
+
+    # credentials settings:
+    BADGES_CONFIG = {


To discuss: do we have any cases for configuring this in the runtime (e.g., through admin panel) using this configuration from settings as fallback?

GlugovGrGlib · 2023-12-13T13:39:01Z

docs/badges/collecting.rst

+On `BADGE_REQUIREMENTS_COMPLETE` signal:
+
+- awarding handler creates `UserCredential`_ record;
+- emits external event about the badge awarding fact;


Suggested change

- emits external event about the badge awarding fact;

- emits external event `org.openedx.learning.badge.awarded.v1`;

GlugovGrGlib · 2023-12-13T13:44:10Z

docs/badges/collecting.rst

+Configured `Badge Collector`_ is auto-subscribed for any updates in `Fulfillment`_ records.
+
+
+Badge completion analysis


Suggested change

Badge completion analysis

Badge completion processing

GlugovGrGlib · 2023-12-13T13:44:16Z

docs/badges/collecting.rst

+- emits external event about the badge awarding fact;
+
+
+Badge revocation analysis


Suggested change

Badge revocation analysis

Badge revocation processing

GlugovGrGlib · 2023-12-13T18:13:03Z

docs/badges/configuration.rst

+Badges management
+-----------------
+
+Badges management includes the following life-cycle stages or maintenance activities:


I believe it will be good to add here state diagram for displaying lifecycle, something like this may be modified, and used in the documentation

https://showme.redstarplugin.com/s/s:4SIKdM94

GlugovGrGlib · 2023-12-13T18:32:49Z

docs/badges/details.rst

+- `COURSE_GRADE_NOW_FAILED` (LMS)
+
+
+Data models


Please check this ERD, it's better to have a visual representation of the data model

https://dbdiagram.io/d/Open-edX-Credly-Badges-645cd5efdca9fb07c4e805a4

ERD for the Credentials service can be found there - https://lucid.app/lucidchart/ac49c90e-9718-4f37-9135-f3c0c9a62017/edit?invitationId=inv_8f93357e-0a72-440f-a91a-19f94e78d2a5&page=pt-o3H4-vyK.#

GlugovGrGlib · 2023-12-13T18:33:55Z

docs/badges/distribution.rst

+
+Each backend may be responsible for:
+
+- communication between external service and Open edX (Credentials);


Suggested change

- communication between external service and Open edX (Credentials);

- communication between external badge issuing service and Open edX (Credentials);

scottrish · 2023-12-15T01:12:45Z

I find it a little confusing to read 'badge' being used in some places to refer to the badge template, and in others to refer to the issued badge. Could we more clearly define these as 'badge template' and either 'badge' or 'issued badge'? I think most backends make this distinction: Credly - badge template and badge; Badgr - badge class and assertion.

feanil · 2023-12-21T21:33:17Z

docs/badges/configuration.rst

+
+    BADGES_CONFIG["events"]["outgoing"]["topics"] = ["badges"]
+
+By default these event types are dynamically auto-registered to EventBus producer in addition to already present in `EVENT_BUS_PRODUCER_CONFIG`.


This is very convenient but I could see it also being surprising when trying to debug and understand where events are coming from. Walking into credentials from other apps, you would not expect other config to be auto-registering events. The flip-side of having to do this manually is not great either as that could lead to a different set of errors during setup. Not a blocker but I wish there was a better way for this to be explicit without making setup more error prone.

At the very least when we add the relevant code, we should make sure to add comments near where the event bus defaults are configured to so that operators know that some events come from the BADGES_CONFIG as well.

Yeah... I still doubt it. I don't like implicit events extension either.

Technically, there are no required events for the feature - we can simplify everything:

no implicit auto-registration for badges-specific events in favor of manual explicit event-bus producer registration for new badge-events;

there will be incoming event types filter only;

I'll update the draft later. Everything should become more transparent then.

justinhynes · 2024-01-22T19:18:11Z

docs/badges/details.rst

+Badges feature extends the set of already `published events`_ with its own set of new public events:
+
+- `BADGE_AWARDED` (Credentials)
+- `BADGE_REVOKED` (Credentials)


Have the new events been proposed or accepted yet?

These two are planned as a part of the openedx/openedx-events#303.
WIP.

justinhynes · 2024-01-22T19:36:13Z

.github/workflows/documentation.yml

+on:
+  push:
+    branches: [aci.documentation]


Will this eventually be carried over to the Credentials project for the master branch? Or is this just temporary for this feature branch/fork?

Nope. It's temporary, just for internal convenience.

justinhynes · 2024-01-23T20:48:38Z

docs/badges/collecting.rst

+
+On `BADGE_REQUIREMENTS_NOT_COMPLETE` signal:
+
+- revocation handler updates `UserCredential`_ record's status to `revoked`;


Will badges have proprietary statuses? Are they using the same awarded and revoked statuses that course and program credentials issued use?

The Badge credential type is planned as the UserCredential descendant (so, internal statuses will be consistent). There will be a special extended type specific for Credly though, but it will live in the Credly "backend" plugin.
Generally, we want to split "core", simple, self-contained badges functionality, and Credly-specific extensions.

justinhynes · 2024-01-24T12:57:14Z

docs/badges/collecting.rst

+
+This is an alternative (reverse) pipeline step for requirements analysis with `revoke` effect set.
+
+- User's `Fulfillment`_ may be updated, so related Requirement stops being fulfilled.


What would cause us to reconsider fulfillment requirements? Would this be caused by a failing grade? Are there any other actions that would be cause for us to reconsider revocation of the badge?

The Requirement entity is aimed to implement a unified approach for future more complex badge rules. Currently, it will be a pretty straightforward usage - a single event leads to the badge earning/revocation.

justinhynes · 2024-01-24T13:18:59Z

docs/badges/configuration.rst

+- uuid (unique identifier, auto-generated)
+- name (verbose label - currently, out of localization)
+- icon(image)


Should status be part of these properties -- inactive, active, and archived?

justinhynes · 2024-01-24T13:30:28Z

docs/badges/configuration.rst

+Revocation setup (reverse effect)
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+During badge template configuration additional decision must be made: wether badges of the given type are `revocable` if required conditions are not the case anymore.


nit: wether --> whether

justinhynes · 2024-01-24T13:51:32Z

docs/badges/details.rst

+Requirement
+~~~~~~~~~~~
+
+**Requirement** describes an association between a BadgeTemplate and an Event.


Event can be an overloaded term -- is the Event referenced here separate from the event bus and the Open edX events?

Yeah, needs a more specific term here. This will be clarified.

justinhynes · 2024-01-24T15:17:10Z

docs/badges/distribution.rst

+Overview
+~~~~~~~~
+
+Credly badges distribution backend is implemented as a baked-in plugin.


When I read plugin, I think "external requirement/library optionally installed". Just confirming, this is not that -- this functionality will be part of the base Credentials service, but is totally optional if the Open edX Operator wants to take advantage of it.

Also -- is the Credly functionality optional from the rest of the Badges implementation? Or will it require the Operator to have a Credly account for any Badge functionality?

On one hand, we want Credly extra functions to be optional, but still a part of the standard codebase (disabled by default, but available if needed out of the box). Currently, it is designed as a separate installable credentials application (which is prepared to be extracted into a separate repo, but is baked-in within its parental badges app and pre-added to requirements).
This is a matter of discussion.
The idea behind such "complexity":

to provide a live example of an extension for the badges app;

to have the ability to extract (if needed in the future) from the main repo;

design badges extensions with respect to the core Badges configuration (this is in progress);

justinhynes · 2024-01-24T15:29:42Z

docs/badges/distribution.rst

+            """
+            - uuid: <ORGANIZATION-ID>
+            - name: <ORGANIZATION-NAME>
+            - api_key: <API-KEY>


I'm not familiar with Credly and thus know nothing about how they manage their API keys and the permissions/actions they allow.

How secure does this API key need to be? Is this something that will be configured (and thus visible) in Django Admin? Or will they need to be more secure (and managed/treated like a secret)?

It is just how Credly's API dev documentation requires. Nothing special from us here.
We agreed with the stakeholders that we treat credential service admins as reliable/responsible persons (at least at this point).

justinhynes · 2024-01-24T16:22:32Z

docs/badges/distribution.rst

+- on award:
+    - credentials.apps.badges.distribution.credly.handlers.notify_user_awarding
+    - credentials.apps.badges.distribution.credly.handlers.issue_badge
+- on revoke:
+    - credentials.apps.badges.distribution.credly.handlers.notify_user_revocation
+    - credentials.apps.badges.distribution.credly.handlers.revoke_badge


There are times when partners of ours don't want to message learners' when credentials are awarded, mostly because the system is designed in a way to award Credentials when they earn a passing grade (and likely haven't truly finished the course).

Will these notifications be optional?

User notifications are still being discussed.
In this docs, it is mostly a hook for a discussion. An option to think about.
We update the docs based on decisions made.

justinhynes · 2024-01-24T16:26:38Z

docs/badges/distribution.rst

+
+    recipient_email: <user-email>
+    issued_to_first_name: <str>
+    issued_to_last_name: <str>


Names are hard, should we be looking at the full_name fields versus First/Last? Or is this a requirement on the Credly side?

justinhynes · 2024-01-24T16:27:21Z

docs/badges/distribution.rst

+    PUT /organizations/<organization_id>/badges/<badge_id>/revoke
+    {
+        "reason": "Check bounced",
+        "suppress_revoke_notification_email": false


Maybe this answers my above question about optionally notifying learners about a badge being awarded?

Yes, user notification is a topic for the final requirements.

justinhynes · 2024-01-24T16:58:34Z

docs/badges/processing.rst

+
+.. note::
+    Events async processing happens in a separate specific (out of the standard Django request-response flow) worker process.
+    See EventBus documentation for details.


Do you think it's worth linking to Event Bus documentation? https://openedx.atlassian.net/wiki/spaces/AC/pages/3127869624/Event+Bus+Project

I think so.
There is still a dedicated section within this documentation itself.

justinhynes · 2024-01-24T17:00:54Z

docs/badges/processing.rst

+
+Once event-related requirements set is clear, Processor updates/creates corresponding `Fulfillments`_'s attributes for the event-learner.
+
+.. _public events: https://github.com/openedx/openedx-events/blob/main/openedx_events/tooling.py


Is this supposed to be linked to somewhere? Or is there a mismatch between the public signals placeholder and the public events link defined here?

We'll revise the consistency of terminology through these docs. Yeah, it's crucial not to produce misconception here.

justinhynes · 2024-01-24T17:01:47Z

docs/badges/quickstart.rst

+Quick Start
+===========
+
+    Learners *earn* badges based on Open edX platform activity.


Should we expand this with a sentence-or-two about what a badge is? Might be useful for people that dive right into the Quick Start docs.

justinhynes · 2024-01-24T17:19:01Z

docs/badges/_puml/badge-awarding-flow-sequence.puml

+ebus <- badges : Emits\n**BADGE_AWARDED**
+lms -> ebus : Receives\n**BADGE_AWARDED**
+
+rnote over backend
+    **CredlyBadgeCollector**
+    **handles event**
+    - analyses Fulfillment(s)
+    - awards CredlyBadge(s)
+    - emits awarding event
+endrnote
+
+ebus <- backend : Emits\n**BADGE_AWARDED**
+lms -> ebus : Receives\n**BADGE_AWARDED**


I'm a little confused by this part. Are we potentially going to be emitting the BADGE_AWARDED event twice? Or are these separate event types -- one tied to native badges and another for Credly badges?

Thanks, it seems that this piece requires optimization/clarification. Noted.

wowkalucky requested a review from GlugovGrGlib December 13, 2023 10:45

wowkalucky self-assigned this Dec 13, 2023

GlugovGrGlib reviewed Dec 13, 2023

View reviewed changes

feanil reviewed Dec 21, 2023

View reviewed changes

wowkalucky changed the title ~~[WIP]Aci.documentation~~ [WIP] feat: [ACI-240] Badges documentation Jan 2, 2024

wowkalucky requested a review from GlugovGrGlib January 4, 2024 13:57

wowkalucky added the documentation label Jan 22, 2024

justinhynes reviewed Jan 22, 2024

View reviewed changes

justinhynes reviewed Jan 23, 2024

View reviewed changes

justinhynes reviewed Jan 24, 2024

View reviewed changes

wowkalucky force-pushed the aci.main branch from c37fe79 to 78f6d69 Compare March 13, 2024 15:48

wowkalucky force-pushed the aci.documentation branch 4 times, most recently from 8afa23d to 3aa6219 Compare April 19, 2024 17:52

wowkalucky added 15 commits April 25, 2024 00:04

feat: add documentation publication

486a422

chore: switch to master configuration

f3ed397

doc: describe badges main concepts

ea5bb74

doc: rework quick start

9cbeaf5

fix: add new theme to requirements

f2298d6

fix: update Quick Start page

7ebd8c4

feat: add Settings page

00aa563

doc: add event bus consumers section

4faa353

doc: add Configuration page and Configuration examples

a362009

doc: add EXCEPT rules examples

1fefce4

doc: mark not relevant pages for removal

f9c03ca

doc: update the Processing page

fbddbf6

doc: add progress records visualization

2f1d963

docs: add EXCLUDED_KEY_PATHS option description

365ab6b

docs: add grouped requirements examples

7609686

wowkalucky force-pushed the aci.documentation branch from d7ee9c3 to 7609686 Compare April 24, 2024 22:06

wowkalucky added 8 commits April 25, 2024 17:07

fix: grouped requirements examples

68da22d

Merge branch 'aci.main' into aci.documentation

2eb7346

doc: actualize requirement groups configuration

9421dee

Merge branch 'aci.main' into aci.documentation

1458744

docs: add penalties configuration

2624c76

feat: the latest BA documentation updates

800039c

chore: updrade documentation to upstream version

191e043

chore: refactor Badges documentation index

6b72f93

	- emits external event about the badge awarding fact;
	- emits external event `org.openedx.learning.badge.awarded.v1`;

		Configured `Badge Collector`_ is auto-subscribed for any updates in `Fulfillment`_ records.


		Badge completion analysis

		- emits external event about the badge awarding fact;


		Badge revocation analysis


		Each backend may be responsible for:

		- communication between external service and Open edX (Credentials);

	- communication between external service and Open edX (Credentials);
	- communication between external badge issuing service and Open edX (Credentials);


		BADGES_CONFIG["events"]["outgoing"]["topics"] = ["badges"]

		By default these event types are dynamically auto-registered to EventBus producer in addition to already present in `EVENT_BUS_PRODUCER_CONFIG`.


		On `BADGE_REQUIREMENTS_NOT_COMPLETE` signal:

		- revocation handler updates `UserCredential`_ record's status to `revoked`;


		This is an alternative (reverse) pipeline step for requirements analysis with `revoke` effect set.

		- User's `Fulfillment`_ may be updated, so related Requirement stops being fulfilled.


		Once event-related requirements set is clear, Processor updates/creates corresponding `Fulfillments`_'s attributes for the event-learner.

		.. _public events: https://github.com/openedx/openedx-events/blob/main/openedx_events/tooling.py

[WIP] feat: [ACI-240] Badges documentation #48

Are you sure you want to change the base?

[WIP] feat: [ACI-240] Badges documentation #48

Conversation

wowkalucky commented Dec 13, 2023 • edited Loading

GlugovGrGlib Dec 13, 2023 • 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

scottrish commented Dec 15, 2023

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

justinhynes Jan 22, 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

Choose a reason for hiding this comment

justinhynes Jan 24, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

justinhynes Jan 24, 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

Choose a reason for hiding this comment

wowkalucky commented Dec 13, 2023 •

edited

Loading

GlugovGrGlib Dec 13, 2023 •

edited

Loading

justinhynes Jan 22, 2024 •

edited

Loading

justinhynes Jan 24, 2024 •

edited

Loading

justinhynes Jan 24, 2024 •

edited

Loading