Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove name param from Credentials.list() #135

Merged

Conversation

kentbull
Copy link
Collaborator

@kentbull kentbull commented Nov 11, 2023

This change yesterday in KERIA removes the /identifiers/{name} prefix in the credentials querying endpoint. I learned from Phil the name argument was never used anyway. The proper way to select which credentials to have returned is with the filters.

Unfortunately this broke the credential querying code used in SignifyTS.

This PR fixes that breakage.

Copy link

codecov bot commented Nov 11, 2023

Codecov Report

Merging #135 (97b49c0) into development (031fde6) will not change coverage.
The diff coverage is 100.00%.

❗ Current head 97b49c0 differs from pull request most recent head 7a378f4. Consider uploading reports for the commit 7a378f4 to get more accurate results

@@             Coverage Diff              @@
##           development     #135   +/-   ##
============================================
  Coverage        80.94%   80.94%           
============================================
  Files               42       42           
  Lines             4173     4173           
  Branches          1028     1028           
============================================
  Hits              3378     3378           
  Misses             763      763           
  Partials            32       32           
Files Coverage Δ
src/keri/app/credentialing.ts 83.27% <100.00%> (ø)

📣 Codecov offers a browser extension for seamless coverage viewing on GitHub. Try it in Chrome or Firefox today!

@pfeairheller
Copy link
Member

Why was credential.ts rewritten as part of this PR? It doesn't seem necessary and we have another PR with changes that may be hard to merge.

@kentbull
Copy link
Collaborator Author

I updated credentials.ts because someone asked about it in Discord and I wanted to test out presentation.

I'll move it to another PR.

@kentbull
Copy link
Collaborator Author

It was also broken. It hadn't been updated to work with IPEX.

@kentbull kentbull force-pushed the credential-query-endpoint branch from 97b49c0 to 7a378f4 Compare November 13, 2023 12:44
@kentbull
Copy link
Collaborator Author

The most recent commit was removed from this branch. It's solely focused on the bugfix.

@kentbull
Copy link
Collaborator Author

I opened a separate PR merging my credentialing.ts changes with @nkongsuwan: nkongsuwan#1
We'll figure it out there.

@pfeairheller pfeairheller merged commit a9cc5b9 into WebOfTrust:development Nov 13, 2023
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants