fix: query the record by credential and proof role #1784
Conversation
berendsliedrecht
commented
Mar 1, 2024
•
berendsliedrecht
commented
- Missed in the last PR regarding issuance to self. We also have to make sure when finding the credential, or proof, exchange record, that the role is correct otherwise we will find multiple instances with the same connection id and thread id.
- Fixed some of the deprecated jest functions (just the ones I came across).
berendsliedrecht
force-pushed
the
query-record-with-role
branch
2 times, most recently
from
3959ef8 to
0eae1c6
Compare
Signed-off-by: Berend Sliedrecht <[email protected]>
berendsliedrecht
force-pushed
the
query-record-with-role
branch
from
0eae1c6 to
6f0a374
Compare
berendsliedrecht
requested review from
genaris and
TimoGlastra
and removed request for
genaris
| * @throws {RecordNotFoundError} If no record is found | ||
| * @throws {RecordDuplicateError} If multiple records are found | ||
| * @returns The credential record | ||
| */ | ||
| public getByThreadAndConnectionId( | ||
| public getByThreadIdConnectionIdAndRole( |
There was a problem hiding this comment.
I think it is a bit confusing this method to be called getByThreadIdConnectionIdAndRole if it's expecting the role before the connectionId. What about swapping the parameters, renaming it or use an options object like we usually do?
There was a problem hiding this comment.
Yeah I didn't do an object to not make too many changes. But I think that that would be the correct call (using a bunch of optional parameter causes a bug in this PR earlier and an object would've caught that).
I do think when it's an object I will rename it to getByQueryFilter or something equivalent. This would allow to have more options in the future as well. Also happy to pick another name :).
There was a problem hiding this comment.
@genaris I updated it to getByProperties and findByProperties. This'll make it expandable in the future as well. Is that good?
There was a problem hiding this comment.
Oh, I was just complaining about the order of the parameters, but like this is better of course!
Signed-off-by: Berend Sliedrecht <[email protected]>
