WebOfTrust / keria

KERI Agent in the cloud
https://keria.readthedocs.io/en/latest/
Apache License 2.0
16 stars 26 forks source link

Fix test delegation approval #248

Closed 2byrds closed 1 month ago

2byrds commented 1 month ago

Delegation Approval enpoint added. The formatting for test_aiding was updated but the only significant change is the test_identifier_delegation_end test.

2byrds commented 1 month ago

Depends on https://github.com/WebOfTrust/keripy/pull/789/files

2byrds commented 1 month ago

Per the discussion on #247 I'm going to adjust the endpoint for the approval on this PR.

2byrds commented 1 month ago

Updated per @lenkan suggestion to use a separate endpoint and a PUT instead of POST

codecov[bot] commented 1 month ago

Codecov Report

Attention: Patch coverage is 94.93671% with 16 lines in your changes are missing coverage. Please review.

Project coverage is 92.71%. Comparing base (caad67b) to head (fcaea50). Report is 11 commits behind head on main.

Files Patch % Lines
src/keria/app/aiding.py 76.59% 11 Missing :warning:
tests/app/test_aiding.py 98.13% 5 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #248 +/- ## ========================================== - Coverage 92.82% 92.71% -0.12% ========================================== Files 37 36 -1 Lines 6982 7093 +111 ========================================== + Hits 6481 6576 +95 - Misses 501 517 +16 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

lenkan commented 1 month ago

Cross posting my comment on #247 for visibility

What is the process?

  1. Delegatee create delegated inception event dip
  2. Delegator receives the inception event details out of band
  3. Delegator create interaction event with delegation information in anchor data.
  4. Delegator approves delegation by posting the same interaction event again?

What is the risk with having step 4 processed automatically?

If it is not possible to do automatically, can we add a flag to step 3 so it becomes

POST /identifiers/name/events
{ 
   ixn: [ anchor data ], 
   approveDelegation: true 
}

Another option if we absolutely need to do do another call to the server, we can move it to another endpoint? What is it that is being created or modified here? Is it a "pending delegation request"?

Note: I am not saying any of my suggestion are objectively better, just trying to trigger discussion about how we design the HTTP API of KERIA so that we can improve it as we move forward.

I think from the dev meeting today it is clear that the desired approach is to not have to do step 4 as a separate API call, but rather as a part of the call that posts the interaction event.

2byrds commented 1 month ago

Cross posting my comment on #247 for visibility

What is the process?

  1. Delegatee create delegated inception event dip
  2. Delegator receives the inception event details out of band
  3. Delegator create interaction event with delegation information in anchor data.
  4. Delegator approves delegation by posting the same interaction event again?

What is the risk with having step 4 processed automatically?

If it is not possible to do automatically, can we add a flag to step 3 so it becomes

POST /identifiers/name/events
{ 
   ixn: [ anchor data ], 
   approveDelegation: true 
}

Another option if we absolutely need to do do another call to the server, we can move it to another endpoint? What is it that is being created or modified here? Is it a "pending delegation request"?

Note: I am not saying any of my suggestion are objectively better, just trying to trigger discussion about how we design the HTTP API of KERIA so that we can improve it as we move forward.

I think from the dev meeting today it is clear that the desired approach is to not have to do step 4 as a separate API call, but rather as a part of the call that posts the interaction event.

I agree, let's hear from @pfeairheller and I'll adjust the PR accordingly

2byrds commented 1 month ago

Closing now that review comments addressed in new PR https://github.com/WebOfTrust/keria/pull/250