-
Notifications
You must be signed in to change notification settings - Fork 55
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
[OSE-301] Implement Create, Get, List, Delete endpoints for issuance templates #231
Conversation
Codecov Report
@@ Coverage Diff @@
## main #231 +/- ##
==========================================
- Coverage 23.33% 22.95% -0.39%
==========================================
Files 27 27
Lines 2507 2549 +42
==========================================
Hits 585 585
- Misses 1821 1863 +42
Partials 101 101
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
pkg/server/router/issuance.go
Outdated
} | ||
|
||
type CreateIssuanceTemplateRequest struct { | ||
issuing.IssuanceTemplate | ||
} | ||
|
||
func (r CreateIssuanceTemplateRequest) ToServiceRequest() *issuing.CreateIssuanceTemplateRequest { | ||
return &issuing.CreateIssuanceTemplateRequest{ | ||
ID: uuid.NewString(), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In our other objects (like schema) we don't have the UUID in the request and we create it on more layer down in the CreateSchema function which is closer to DB.
Shouldn't matter either way, but eventally we should be consistent
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great point. Moved this to the service layer.
pkg/server/router/issuance.go
Outdated
|
||
if err != nil { | ||
return framework.NewRequestError( | ||
util.LoggingErrorMsg(err, "could not get manifests"), http.StatusBadRequest) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"could not get templates"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed
pkg/server/router/issuance.go
Outdated
id := framework.GetParam(ctx, IDParam) | ||
if id == nil { | ||
return framework.NewRequestError( | ||
util.LoggingNewError("cannot delete a presentation without an ID parameter"), http.StatusBadRequest) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
template instead of presentation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
pkg/server/router/issuance.go
Outdated
|
||
if err := ir.service.DeleteIssuanceTemplate(&issuing.DeleteIssuanceTemplateRequest{ID: *id}); err != nil { | ||
return framework.NewRequestError( | ||
util.LoggingErrorMsgf(err, "could not delete presentation with id: %s", *id), http.StatusInternalServerError) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
template instead of presentation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, some minor comments
Credentials: []issuing.CredentialTemplate{ | ||
{ | ||
ID: "output_descriptor_1", | ||
Schema: "", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
remove
Issuer: issuerResp.DID.ID, | ||
Credentials: []issuing.CredentialTemplate{ | ||
{ | ||
ID: "", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
set or remove
// The template to create. | ||
IssuanceTemplate IssuanceTemplate `json:"issuanceTemplate"` | ||
} | ||
|
||
func (r CreateIssuanceTemplateRequest) IsValid() bool { | ||
return util.IsValidStruct(r) == nil |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why use this here and Validate()
below - should they be consistent?
Overview
This PR implements Create, Get, List, Delete endpoints for issuance templates.
Description
This is very similar to the existing CRUD endpoints. The only main difference is that there are validation checks for some of the fields including the
schema
,ID
andexpiry
of all theIssuanceTemplate.Credentials
objects.How Has This Been Tested?
Added tests for each of the endpoints.
Checklist
Before submitting this PR, please make sure:
References
SIP5