Skip to content
This repository was archived by the owner on Dec 12, 2024. It is now read-only.

Added did ion deactivation endpoint.#646

Open
andresuribe87 wants to merge 11 commits intoTBD54566975:mainfrom
andresuribe87:issue_340_deactivate
Open

Added did ion deactivation endpoint.#646
andresuribe87 wants to merge 11 commits intoTBD54566975:mainfrom
andresuribe87:issue_340_deactivate

Conversation

@andresuribe87
Copy link
Copy Markdown
Contributor

Overview

As part of #340, this PR adds support for deactivation. See the API documentation for more details.

Description

This PR involves a couple of changes so that we return resolution results.

How Has This Been Tested?

Unit tests.

Comment thread doc/swagger.yaml Outdated
summary: Updates a DID document.
tags:
- DecentralizedIdentityAPI
/v1/dids/{method}/{id}/deactivation:
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

this can be more restful as a delete to /v1/dids/{method}/{id}

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

We already have a that taken by SoftDelete

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

hm, I see. awkward because we both need to handle the service's concept of deletion and the did methods
I'm inclined to merge the two...
Something lke -- if the DID method supports deactivation, the DELETE to /v1/dids/{method}/{id} does both delete and deactivation. If the DID method does not support deactivation, we just do a delete.

Reason being, it's more confusing to support both separately. What do you think?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I like that, will update.

Comment thread pkg/service/did/handler.go
Comment thread pkg/service/did/ion.go Outdated
if storedDID.Deactivated {
return storedDID, nil
}
storedDID.Operations = append(storedDID.Operations, deactivateRequest)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

this seems off - shouldn't the result be the processed deactivate request?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Which "result" are you referring to?

Copy link
Copy Markdown
Contributor

@decentralgabe decentralgabe Aug 11, 2023

Choose a reason for hiding this comment

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

my thought was that we should not rely on our internal source of truth, but have some operation to wait for an anchor and have that reflected as the result. but I'm thinking that's too complex. just need to make sure these properties are correct. (they look to be correct)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I think we're eventually going to need to communicate to devs that some new state hasn't been published. That said, I agree that it's too complex for now.

I made my best to ensure that all the properties were correct.

Comment thread pkg/service/did/ion.go Outdated
@codecov-commenter
Copy link
Copy Markdown

Codecov Report

Merging #646 (da81f3d) into main (e4ce096) will increase coverage by 0.33%.
The diff coverage is 31.63%.

@@            Coverage Diff             @@
##             main     #646      +/-   ##
==========================================
+ Coverage   25.72%   26.06%   +0.33%     
==========================================
  Files          56       57       +1     
  Lines        6254     6404     +150     
==========================================
+ Hits         1609     1669      +60     
- Misses       4369     4442      +73     
- Partials      276      293      +17     
Files Changed Coverage Δ
pkg/server/router/did.go 2.79% <0.00%> (ø)
pkg/service/did/common.go 0.00% <0.00%> (ø)
pkg/service/did/handler.go 0.00% <0.00%> (ø)
pkg/service/did/key.go 0.00% <0.00%> (ø)
pkg/service/did/model.go 0.00% <ø> (ø)
pkg/service/did/service.go 0.00% <0.00%> (ø)
pkg/service/did/storage.go 56.12% <ø> (ø)
pkg/service/did/web.go 0.00% <0.00%> (ø)
pkg/service/did/ion.go 34.33% <43.57%> (+3.81%) ⬆️
pkg/server/server.go 71.25% <100.00%> (ø)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants