digitalcredentials / learner-credential-wallet

Learner Credential Wallet is a cross-platform iOS and Android mobile application for storing and sharing digital learner credentials.
https://lcw.app
MIT License
58 stars 29 forks source link

Credential Display varies between Home and Credential Preview both iOS and Android #478

Closed bmuramatsu closed 1 year ago

bmuramatsu commented 1 year ago

This is for the test credential Dmitri just created for BCSI, and one generated by Kayode.

In both iOS and Android, v2.0.9-build 65 the text for the nominal badge name / credential name is different on the Home Screen and on the credential preview.

It looks like different fields are being rendered in each location.

Credential Preview = DCC Test Credential

Home = Badge

(This also occurs for the test credential Dmitri made on 8/10 with different value SharedCredential.txt.zip s, you can see the wrong label reads as "Documentation and SOP" when it should be "Evaluator" in the Home screenshot.

Attached is the json for the DCC Test Credential.

To reproduce add the attached credential and view the name on the Home screen and Credential Preview screen.

jennacioffi commented 1 year ago

ignore previously deleted commented - will ask any questions we have on Monday instead when we have more time to look into it :)

kayaelle commented 1 year ago

@bmuramatsu & @dmitrizagidulin - the reason this is happening is because the credentialDisplay is using AchievementCredential.name. The acceptance and list page are using AchievementSubject.Achievement.name. For the BCSI credential, the AchievementCredential.name in the spreadsheet is "Certified Evaluator" and AchievementSubject.Achievement.name doesn't have a value in the spreadsheet.

Two questions:

  1. What should the AchievementCredential.name & AchievementSubject.Achievement.name - keep in mind that AchievementCredential.name is intended to be unique per credentials and AchievementSubject.Achievement.name is the name that would typically get repeated across badges like in a badge class. These don't need to be different. In OBv2, there was only one name field But, we should consider how the LCW handles this.
  2. Should the name be "Certified Evaluator" vs "Evaluator"?

Atomic Jolt team - please hold on this until I give the go-ahead. Moving this to backlog until then. Thanks!

bmuramatsu commented 1 year ago

I think there are additional things going on in addition to what Kerri mentions. I address @kayaelle's points in 1 and 2 below.

(I should note I'm using the credential Dmitri shared on Thursday in slack, and v2.0.10-Build 65. Light Mode.)

  1. What's in the Google doc for the pre-test and test.
  2. What's in the data that Dmitri used to create the credential we've been viewing.
  3. What LCW is displaying on the Credential Preview Screen.
  4. What LCW is displaying on the Home Screen.
  5. Note that the same display of data is happening in the credentials that Kayode created as test samples.

1. What's in the Google sheet for the pre-test and test.

For BCSI there are not credential names that are unique by recipient. Then, based on the definitions provided by Kerri, column H in the spreadsheet should be labeled, achievementSubject.Achievement.name as it is "name that would typically get repeated across badges like in a badge class". I've updated that spreadsheet accordingly.

Note: I also updated the Google sheet with a non-badgr hosted Achievement.image.id. I forgot to update the data after Angela provided an alternate location.

2. What's in the data that Dmitri used to create the credential we've been viewing. From Slack, Dmitri shared a link to a sample credential: dccrequest://request?issuer=https%3A%2F%2Fissuer.example.com&vc_request_url=https://verify.dcconsortium.org/request/credential&challenge=ke12345678-0001&auth_type=bearer

I wasn't as concerned about the data when I submitted the issue. This issue focused on what I saw when displaying the credential, vs. the info used to generate the credential. Both are needed, but I didn't dig into the info itself.

The correct information does need to get added for the BCSI test, and that info is in the Google sheet, but I figured we could take care of that next pass.

I think that some of the issues of the info used to create the credential and what's displayed are interlinked.

Things to check on the next review are the text in columns G, H, I, J, L, M, O, P, Q showing up as intended?

For the purposes of development Line 2 for "Test Evaluator" or Line 3 for "Angela Consani" could be used. Line 2 has an expiration date, Line 3 does not (as Angela's credential does not expire).

3. What LCW is displaying on the Credential Preview Screen for the name. Looking at the credential Dmitri created and as shown in the screenshot and viewing the JSON of the credential, the text "Evaluator" seems to come from vc.name. (It's the only entry that "Evaluator" shows up.)

I think that's correct because that's where we'd put vc.name. Perhaps in the code achievementSubject.Achievement.name would also be displayed there if it existed in the JSON/info used to create the credential. This, I think is a related issue that doesn't need to be resolved now: What happens if there is conflicting data between VC and OBv3 fields, and what do we support?

And it seems that if achievementCredential.name existed then we'd probably want to display that instead of achievementSubject.Achievement.name per Kerri's description. Or as a related issue but one that does not need to be resolved now: Do we put the same information into both fields when we issue the credential for downstream compatibility if there is no "name that is unique per credential"?

Given all of the above I think we'd want to display achievementCredential.name if it exists, otherwise achievementSubject.achievement.name, otherwise vc.name, or suppress the display of the line entirely unless there is a achievement.image.id (or the previous data elements we were using for the image in this location) even if any of the name fields are empty. We might consider strongly encouraging one of the name fields to be non-empty.

The info we want displayed for the BCSI test is "Certified Evaluator" and comes from Column H of the Google sheet. And per 1. above I think we want to put that information into achievementSubject.Achievement.name when we issue the credential.

4. What LCW is displaying on the Home Screen. If I'm reading the JSON right, credentialSubject.achievement.name is showing up on the Home Screen as in the screenshot. The text "Documentation and SOP seems to come from credentialSubject.achievement.name. (It's the only entry that "Documentation and SOP" shows up.)

As above perhaps there's additional logic of what gets displayed there from previous contexts we've supported and need to continue to support in addition to supporting the name as per above.

5. Note that the same display of data is happening in the credentials that Kayode created as test samples.

The same mismatch between the Home Screen and Credential preview screen happens for the credentials Kayode created.

By observation (and not looking at the source code), it seems we need to both issue the correct information as the credential is issued, and know how we're displaying what's getting issued.

kayaelle commented 1 year ago

Thanks, @bmuramatsu - I've updated: https://github.com/digitalcredentials/ops/issues/10 to update both AchievementCredential.name and achievement.name to "Certified Evaluator". This issue also references an edit for the criteria.narrative.

I did a review of the spreadsheet & the badge data. If you would like to take another pass at it, please do that soon.

Once @dmitrizagidulin has updated the badge template, I'll work with Atomic Jolt team on the display issue.

kayaelle commented 1 year ago

Atomic Jolt - please use AchievementCredential.name wherever the name of the credential mentioned.

jennacioffi commented 1 year ago

PR: https://github.com/digitalcredentials/learner-credential-wallet/pull/484

kayaelle commented 1 year ago

Confirmed that displays are using AchievementCredential.name in iOS and android