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

Added JWT for definitions. #262

Merged
merged 6 commits into from
Jan 23, 2023
Merged

Conversation

andresuribe87
Copy link
Contributor

Overview

This PR adds JWTs for presentation definitions. This means that it is the implementation for presentation request.

Description

A couple of changes happened with this PR.

  1. A new required field, Author, was introduced for creating presentation definitions.
  2. New JWT fields were added to the Create and Get endpoints for definitions.
  3. The StoredDefinition now stores the new Author field.

How Has This Been Tested?

Unit tests were updated, as well as integration tests.

@codecov-commenter
Copy link

codecov-commenter commented Jan 20, 2023

Codecov Report

Merging #262 (2c5f820) into main (68da36f) will decrease coverage by 0.21%.
The diff coverage is 0.00%.

@@            Coverage Diff             @@
##             main     #262      +/-   ##
==========================================
- Coverage   24.87%   24.67%   -0.21%     
==========================================
  Files          31       31              
  Lines        3063     3084      +21     
==========================================
- Hits          762      761       -1     
- Misses       2184     2206      +22     
  Partials      117      117              
Impacted Files Coverage Δ
integration/common.go 0.00% <0.00%> (ø)
pkg/server/router/presentation.go 3.38% <0.00%> (-0.08%) ⬇️
pkg/service/keystore/service.go 41.84% <0.00%> (-5.04%) ⬇️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@@ -2,12 +2,18 @@ package router

import (
"context"
crypto2 "crypto"
Copy link
Member

Choose a reason for hiding this comment

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

nit: rename alias to gocrypto or similar

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

return &model.CreatePresentationDefinitionResponse{
PresentationDefinition: storedPresentation.PresentationDefinition,
}, nil
m := &model.CreatePresentationDefinitionResponse{}
Copy link
Member

Choose a reason for hiding this comment

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

either
var m model.CreatePresentationDefinitionResponse

or

m := new(model.CreatePresentationDefinitionResponse)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

Copy link
Member

@decentralgabe decentralgabe left a comment

Choose a reason for hiding this comment

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

looks good - two small comments

@andresuribe87 andresuribe87 merged commit 66fcff5 into TBD54566975:main Jan 23, 2023
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