Skip to content
This repository has been archived by the owner on Dec 12, 2024. It is now read-only.

Fixing Cred Suspension Bug #487

Merged
merged 7 commits into from
May 26, 2023
Merged

Fixing Cred Suspension Bug #487

merged 7 commits into from
May 26, 2023

Conversation

nitro-neal
Copy link
Contributor

@nitro-neal nitro-neal commented May 25, 2023

Fixed the suspension issue not reflecting back to the user.

Updated unit tests
Created suspension integration test
Added UnRevoke and UnSuspend integration test

image image

@codecov-commenter
Copy link

codecov-commenter commented May 25, 2023

Codecov Report

Merging #487 (f7a3122) into main (712e224) will decrease coverage by 0.06%.
The diff coverage is 0.00%.

@@            Coverage Diff             @@
##             main     #487      +/-   ##
==========================================
- Coverage   20.62%   20.56%   -0.06%     
==========================================
  Files          48       48              
  Lines        5756     5772      +16     
==========================================
  Hits         1187     1187              
- Misses       4360     4376      +16     
  Partials      209      209              
Impacted Files Coverage Δ
integration/common.go 1.94% <0.00%> (-0.04%) ⬇️
pkg/service/credential/service.go 0.00% <0.00%> (ø)

@nitro-neal nitro-neal marked this pull request as ready for review May 25, 2023 18:11
@nitro-neal
Copy link
Contributor Author

Fixes - #473

func CreateVerifiableCredential(credentialInput credInputParams, revocable bool) (string, error) {
func CreateVerifiableCredential(credentialInput credInputParams) (string, error) {
return CreateVerifiableCredentialWithStatus(credentialInput, false, false)
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

new line


func TestSuspensionCreateIssuerDIDKeyIntegration(t *testing.T) {
if testing.Short() {
t.Skip("skipping integration testz")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

z

@@ -0,0 +1,233 @@
package integration
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could this be combined into a credential_status_integration_test?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it could be, but I opted for a fresh flow, fresh creds, making sure suspension works, didnt want like a revokable cred flow to 'init' something so a suspension would work

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm worried about scaling this once we move to a generic status - and not needing to support specific statuses. But fine to save that until we impl that change.

"postalCode": "78724",
"streetAddress": "123 Janktopia Ave."
},
"taxID":"123"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
"taxID":"123"
"taxId":"123"

Copy link
Contributor

@andresuribe87 andresuribe87 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor suggestion

@@ -124,6 +128,10 @@ func CreateVerifiableCredential(credentialInput credInputParams, revocable bool)
if revocable {
fileName = "credential-revocable-input.json"
}
if suspendable {
fileName = "credential-suspendable-input.json"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe worth putting them in the same json and using the templating mechanism?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added 👍 and removed these .json files

@nitro-neal nitro-neal merged commit 47db532 into main May 26, 2023
@nitro-neal nitro-neal deleted the fix-suspension-bug branch May 26, 2023 16:37
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants