feat: display course credentials on VC page #469

kyrylo-kh · 2025-01-02T10:34:34Z

Description:
Display course credentials along with program credentials on the Verifiable Credentials tab

Related PR's:

feat: course credentials as verifiable credentials credentials#2674

openedx-webhooks · 2025-01-02T10:34:39Z

Thanks for the pull request, @kyrylo-kh!

This repository is currently maintained by @2U-aperture.

Once you've gone through the following steps feel free to tag them in a comment and let them know that your changes are 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.

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.

codecov · 2025-01-02T10:37:15Z

Codecov Report

Attention: Patch coverage is 91.89189% with 3 lines in your changes missing coverage. Please review.

Project coverage is 74.01%. Comparing base (db6fc53) to head (68d2cfd).

Files with missing lines	Patch %	Lines
...c/components/CertificatesList/CertificatesList.jsx	88.46%	3 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #469      +/-   ##
==========================================
+ Coverage   73.25%   74.01%   +0.76%     
==========================================
  Files          28       28              
  Lines         415      431      +16     
  Branches       89      100      +11     
==========================================
+ Hits          304      319      +15     
- Misses        110      111       +1     
  Partials        1        1

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

e0d · 2025-01-15T18:05:39Z

@kyrylo-kh can you resolve the conflicts when you have a moment.

deborahgu

looks good to me, although I have one small request, nonblocking. One more aperture person is probably going to look as well.

deborahgu · 2025-01-22T16:34:59Z

src/components/CertificatesList/CertificatesList.jsx

  const [
    verifiableCredentialIssuanceData,
    setVerifiableCredentialIssuanceData,
  ] = useState({});

  useEffect(() => {
-    getProgramCertificates()
+    getCertificates()


There's a previously existing bug here, that you're not introducing. However, since it is in the verifiable credentials rendering codecode, it would be really nice if you fixed. (Nonblocking because it's a pre-existing bug in the verifiable credentials code, and presumably only turns up in certain configurations of instances, but still.)

On an out-of-the-box configuration, verifiable credentials shows up in the learner record as a tab whether or not they are configured or available:

Selecting that tab generates a runtime error, visible in development, pasted below.

I haven't looked closely enough to see what is going on to make it so the verifiable credentials is rendering and causing an error when clicked, but surely if it's not configured to run correctly, it shouldn't render at all.

can't access property "length", certificates.program_credentials is undefined CertificatesList@webpack-internal:///./src/components/CertificatesList/CertificatesList.jsx:57:31 renderWithHooks@webpack-internal:///./node_modules/react-dom/cjs/react-dom.development.js:14985:27 updateFunctionComponent@webpack-internal:///./node_modules/react-dom/cjs/react-dom.development.js:17356:20 beginWork@webpack-internal:///./node_modules/react-dom/cjs/react-dom.development.js:19063:16 callCallback@webpack-internal:///./node_modules/react-dom/cjs/react-dom.development.js:3945:14 invokeGuardedCallbackDev@webpack-internal:///./node_modules/react-dom/cjs/react-dom.development.js:3994:16 invokeGuardedCallback@webpack-internal:///./node_modules/react-dom/cjs/react-dom.development.js:4056:31 beginWork$1@webpack-internal:///./node_modules/react-dom/cjs/react-dom.development.js:23959:28 performUnitOfWork@webpack-internal:///./node_modules/react-dom/cjs/react-dom.development.js:22771:12 workLoopSync@webpack-internal:///./node_modules/react-dom/cjs/react-dom.development.js:22702:22 renderRootSync@webpack-internal:///./node_modules/react-dom/cjs/react-dom.development.js:22665:7 performSyncWorkOnRoot@webpack-internal:///./node_modules/react-dom/cjs/react-dom.development.js:22288:18 flushSyncCallbackQueueImpl/<@webpack-internal:///./node_modules/react-dom/cjs/react-dom.development.js:11327:26 unstable_runWithPriority@webpack-internal:///./node_modules/scheduler/cjs/scheduler.development.js:468:12 runWithPriority$1@webpack-internal:///./node_modules/react-dom/cjs/react-dom.development.js:11276:10 flushSyncCallbackQueueImpl@webpack-internal:///./node_modules/react-dom/cjs/react-dom.development.js:11322:26 flushSyncCallbackQueue@webpack-internal:///./node_modules/react-dom/cjs/react-dom.development.js:11309:3 scheduleUpdateOnFiber@webpack-internal:///./node_modules/react-dom/cjs/react-dom.development.js:21888:9 dispatchAction@webpack-internal:///./node_modules/react-dom/cjs/react-dom.development.js:16139:26 CertificatesList/</<@webpack-internal:///./src/components/CertificatesList/CertificatesList.jsx:61:22

To be honest I can't reproduce this error locally. When I switch tab to Verifiable Credentials I see only one message in console

Warning: Failed prop type: CertificateModal: prop type `data` is invalid; it must be a function, usually from the `prop-types` package, but received `undefined`.This often happens because of typos such as `PropTypes.function` instead of `PropTypes.func`.

Could you tell me how to reproduce it?

okay, I tracked this down. If you have the out-of-the-box dev configuration that you get from source (learner record MFE has ENABLE_VERIFIABLE_CREDENTIALS='true', credentials IDA does not have ENABLE_VERIFIABLE_CREDENTIALS = True), you get this effect. If you explicitly disable ENABLE_VERIFIABLE_CREDENTIALS on the MFE you obviously don't see the 2nd tab, and if you explicitly enable ENABLE_VERIFIABLE_CREDENTIALS the credentials ida it works fine.

So i still would call this a bug just in terms of exception handling, but it's a bug it's only ever going to affect developers. In a production environment, if you have those 2 settings not matching, you've done something wrong.

tl;dr I'd love it if y'all opened to take it internally to make it so that if that circumstance happens you get some kind of graceful failure, just in case a production operator does the dumb thing. But it's definitely not a blocker here.

I fixed it. When you set ENABLE_VERIFIABLE_CREDENTIALS on MFE, but not set on backend, you'll see such error once you will go to VC tab

Diff: https://github.com/openedx/frontend-app-learner-record/compare/46049a6218238876055d3c6a51dbbc8a6fd52075..68d2cfd7e81c107c834bf3b4b2c77e8ff27099c2

jsnwesson · 2025-01-22T16:48:40Z

package.json

@@ -46,6 +46,9 @@
    "@fortawesome/react-fontawesome": "0.2.2",
    "@openedx/frontend-plugin-framework": "^1.2.0",
    "@openedx/paragon": "^22.2.2",
+    "ajv": "^8.12.0",
+    "ajv-keywords": "^5.1.0",
+    "axios": "^1.7.9",


axios is already brought in as a dependency through frontend-platform, is this version requiring anything new that can't be obtained from the one in frontend-platform?

Deleted this package from package.json
https://github.com/openedx/frontend-app-learner-record/compare/140ec21935491fbe9a1523a62f3314cc3480bac7..4f0a0eaa7f56173c8e88064f482bb1fece045256

jsnwesson · 2025-01-22T16:51:39Z

package.json

-    "regenerator-runtime": "0.14.1"
+    "regenerator-runtime": "0.14.1",
+    "schema-utils": "^4.2.0",
+    "webpack-dev-server": "^4.15.2"


similarly with axios, frontend-build has webpack-dev-server already as a dependency, which I can see already exists in Learner Record as a dependency

Deleted this package from package.json
https://github.com/openedx/frontend-app-learner-record/compare/140ec21935491fbe9a1523a62f3314cc3480bac7..4f0a0eaa7f56173c8e88064f482bb1fece045256

jsnwesson · 2025-01-22T17:02:46Z

src/components/CertificatesList/messages.js

+  courseCredentialsDescription: {
+    id: 'credentials.description',
+    defaultMessage:
+      'A certificate for a course will appear once you have completed course.',


Suggested change

'A certificate for a course will appear once you have completed course.',

'A course certificate will appear once you have completed a course.',

This is only a suggestion, but I was trying to rewrite this so that the end of the sentence made it clear that a certificate would be received once the learner finished any course.
Another way to write this is:
"A certificate for a course will appear one you have completed the course."

Changed text to A course certificate will appear once you have completed a course.
https://github.com/openedx/frontend-app-learner-record/compare/140ec21935491fbe9a1523a62f3314cc3480bac7..4f0a0eaa7f56173c8e88064f482bb1fece045256

jsnwesson

Aside from some comments about dependencies and a string message, this looks good to me!

deborahgu · 2025-01-27T17:18:02Z

looks good to me.

openedx-webhooks added the open-source-contribution PR author is not from Axim or 2U label Jan 2, 2025

kyrylo-kh marked this pull request as draft January 2, 2025 10:34

kyrylo-kh mentioned this pull request Jan 2, 2025

feat: course credentials as verifiable credentials openedx/credentials#2674

Open

kyrylo-kh force-pushed the feat/display-course-credentials branch from c6d3174 to 140ec21 Compare January 2, 2025 11:19

kyrylo-kh marked this pull request as ready for review January 15, 2025 15:25

deborahgu approved these changes Jan 22, 2025

View reviewed changes

jsnwesson reviewed Jan 22, 2025

View reviewed changes

kyrylo-kh force-pushed the feat/display-course-credentials branch 2 times, most recently from 4f0a0ea to 46049a6 Compare January 27, 2025 09:30

feat: display course credentials on VC page

68d2cfd

kyrylo-kh force-pushed the feat/display-course-credentials branch from 46049a6 to 68d2cfd Compare January 28, 2025 09:14

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: display course credentials on VC page #469

feat: display course credentials on VC page #469

kyrylo-kh commented Jan 2, 2025

openedx-webhooks commented Jan 2, 2025 •

edited

Loading

🔘 Get product approval

🔘 Provide context

🔘 Get a green build

Where can I find more information?

When can I expect my changes to be merged?

codecov bot commented Jan 2, 2025 •

edited

Loading

e0d commented Jan 15, 2025

deborahgu left a comment

deborahgu Jan 22, 2025 •

edited

Loading

kyrylo-kh Jan 27, 2025

deborahgu Jan 27, 2025

kyrylo-kh Jan 28, 2025

jsnwesson Jan 22, 2025

kyrylo-kh Jan 27, 2025

jsnwesson Jan 22, 2025

kyrylo-kh Jan 27, 2025

jsnwesson Jan 22, 2025

kyrylo-kh Jan 27, 2025

jsnwesson left a comment

deborahgu commented Jan 27, 2025

	'A certificate for a course will appear once you have completed course.',
	'A course certificate will appear once you have completed a course.',

feat: display course credentials on VC page #469

Are you sure you want to change the base?

feat: display course credentials on VC page #469

Conversation

kyrylo-kh commented Jan 2, 2025

openedx-webhooks commented Jan 2, 2025 • edited Loading

🔘 Get product approval

🔘 Provide context

🔘 Get a green build

Where can I find more information?

When can I expect my changes to be merged?

codecov bot commented Jan 2, 2025 • edited Loading

Codecov Report

e0d commented Jan 15, 2025

deborahgu left a comment

Choose a reason for hiding this comment

deborahgu Jan 22, 2025 • 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

jsnwesson left a comment

Choose a reason for hiding this comment

deborahgu commented Jan 27, 2025

openedx-webhooks commented Jan 2, 2025 •

edited

Loading

codecov bot commented Jan 2, 2025 •

edited

Loading

deborahgu Jan 22, 2025 •

edited

Loading