feat(vc): delete w3c credential record #886
Conversation
Signed-off-by: Berend Sliedrecht <berend@animo.id>
Signed-off-by: Karim Stekelenburg <karim@animo.id>
Signed-off-by: Timo Glastra <timo@animo.id>
…on#786) Signed-off-by: Karim <karim@animo.id>
Signed-off-by: Karim <karim@animo.id>
Signed-off-by: Timo Glastra <timo@animo.id>
Signed-off-by: Karim <karim@animo.id>
Signed-off-by: Karim <karim@animo.id>
karimStekelenburg
changed the title
Codecov Report
@@ Coverage Diff @@
## 0.3.0-pre #886 +/- ##
=============================================
+ Coverage 87.28% 87.32% +0.03%
=============================================
Files 562 562
Lines 13272 13274 +2
Branches 2142 2142
=============================================
+ Hits 11584 11591 +7
+ Misses 1684 1679 -5
Partials 4 4
|
|
Because the CI is still testing against Node v14 the tests are failing. Since the 0.3.0-pre branch itself already contains the Node v14 incompatible code I guess it's OK to merge this one, but I'd like to double check. @TimoGlastra, thoughts? |
|
You mean Node 12? This has already been removed in main, so if we rebase 0.3.0 with main we can see if CI now fully passess |
| } | ||
|
|
||
| public async getCredentialById(id: string): Promise<W3cVerifiableCredential> { | ||
| return (await this.w3cCredentialRepository.getById(id)).credential | ||
| public async getAllCredentialRecords(): Promise<W3cCredentialRecord[]> { |
There was a problem hiding this comment.
We don't use this in all other services/modules (just getAll instead of getAllCredentials, getAllCredentialRecords). Do you think this helps? If so we should probably change this in all methods
There was a problem hiding this comment.
I get your point in terms of consistency. However, I consciously did this because the W3cCredentialService deals with more than credential records. For example, there is a signPresentation() method and a getSignatureSuitesForCredential(). It seems a bit odd to me to specify the data model in some method names, but leave it out in others.
I'm fine with changing it though. However, in that case it would make sense to me to rename W3cCredentialService to W3cCredentialRecordService. Then at least the class name gives you a hint of what getAll() actually returns.
packages/core/src/modules/vc/__tests__/W3cCredentialService.test.ts
Outdated
Show resolved
Hide resolved
TimoGlastra
force-pushed
the
0.3.0-pre
branch
2 times, most recently
from
65f8856
to
938a889
Compare
Co-authored-by: Timo Glastra <timo@animo.id>
Signed-off-by: Karim Stekelenburg <karim@animo.id>
…c-credential-record
Signed-off-by: Karim <karim@animo.id>
Signed-off-by: Karim <karim@animo.id>
Co-authored-by: Timo Glastra <timo@animo.id>
Signed-off-by: Karim Stekelenburg <karim@animo.id>
|
Merging as only failing test is the bbs test |
TimoGlastra
changed the title
Signed-off-by: Karim <karim@animo.id>
Signed-off-by: Karim <karim@animo.id>
Signed-off-by: Karim <karim@animo.id>
Signed-off-by: Karim <karim@animo.id> Signed-off-by: Ariel Gentile <gentilester@gmail.com>
This PR adds a method to the W3cCredentialService to remove a credential record from storage. I also renamed a few properties and methods because their previous names were somewhat misleading.