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

fix: didx request cannot be accepted #1881

Merged

Conversation

rmnre
Copy link
Contributor

@rmnre rmnre commented Aug 1, 2022

Hi everyone,

this PR fixes a few problems which were apparently introduced by #1710:

  • A didx request which is received in response to an explicit oob invitation cannot be accepted because the state of the corresponding connection record is not updated to 'request-received'

  • The admin api incorrectly indicates that the /out-of-band/receive-invitation endpoint returns a ConnRecord instead of the new OobRecord (which is also why the OobRecord is not included in the swagger.json)

@TimoGlastra: maybe you want to take a look to make sure I didn't break the connectionless exchange feature by saving the updated connection record?

Copy link
Contributor

@dbluhm dbluhm left a comment

Choose a reason for hiding this comment

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

I haven't explicitly evaluated whether this impacts connection-less exchange but not saving the conn rec in just that one case certainly seems like a bug. This fix looks reasonable to me.

@dbluhm dbluhm requested a review from TimoGlastra August 3, 2022 15:57
@codecov-commenter
Copy link

Codecov Report

Merging #1881 (2fec3f6) into main (8e6dce2) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##             main    #1881   +/-   ##
=======================================
  Coverage   93.79%   93.79%           
=======================================
  Files         539      539           
  Lines       34079    34081    +2     
=======================================
+ Hits        31965    31967    +2     
  Misses       2114     2114           

@swcurran
Copy link
Contributor

swcurran commented Aug 3, 2022

An integration test is failing. Hopefully this is not an intermittent failure, but it needs to be investigated.

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.

4 participants