-
Notifications
You must be signed in to change notification settings - Fork 34
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
⚠️ Archetypes and Custom Assessments #476
Conversation
3e0153c
to
148f3a0
Compare
api/questionnaire.go
Outdated
ApplyTags []CategorizedTag `json:"applyTags,omitempty" yaml:"applyTags,omitempty"` | ||
AutoAnswerFor []CategorizedTag `json:"autoAnswerFor,omitempty" yaml:"autoAnswerFor,omitempty"` | ||
Selected bool `json:"selected,omitempty" yaml:"selected,omitempty"` | ||
} |
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.
@mansam Is there going to be an answers array on the question struct?
api/assessment.go
Outdated
Questionnaire Ref `json:"questionnaire" yaml:"questionnaire"` | ||
Sections []Section `json:"sections" yaml:"sections"` | ||
Thresholds Thresholds `json:"thresholds" yaml:"thresholds"` | ||
RiskMessages RiskMessages `json:"riskMessages" yaml:"riskMessages"` |
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.
Wondering how we might determine assessment status?
migration/v9/model/assessment.go
Outdated
Revision uint | ||
Sections JSON | ||
Thresholds JSON | ||
RiskMessages JSON |
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.
Do we need to add the Stakeholder & StakeholderGroup at the assessment level here since we can associate them to assessments within the wizard?
fa7417c
to
75172b5
Compare
api/application.go
Outdated
@@ -856,10 +1054,13 @@ type Application struct { | |||
Comments string `json:"comments"` | |||
Identities []Ref `json:"identities"` | |||
Tags []TagRef `json:"tags"` | |||
BusinessService *Ref `json:"businessService"` | |||
BusinessService *Ref `json:"businessService" yaml:"businessService"` |
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 is the yaml tag needed. IIRC, it would default to the same camel cased.
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.
I'm not sure what lead to that happening. Fixing.
// Binding is strict: unknown fields in the input will cause binding to fail. | ||
func (h *BaseHandler) BindJSON(ctx *gin.Context, r interface{}) (err error) { | ||
if ctx.Request == nil || ctx.Request.Body == nil { | ||
err = errors.New("invalid request") |
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.
missing return?
// Binding is strict: unknown fields in the input will cause binding to fail. | ||
func (h *BaseHandler) BindYAML(ctx *gin.Context, r interface{}) (err error) { | ||
if ctx.Request == nil || ctx.Request.Body == nil { | ||
err = errors.New("invalid request") |
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.
missing return here?
- key: A | ||
value: 1 | ||
- key: B | ||
value: 2 |
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 facts removed?
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.
The Application resource doesn't have a facts field, so these were just getting ignored. This PR turns on strict binding, so the unknown field causes a failure.
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.
ah, makes sense.
Seems there are yaml tags where they are not needed. |
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.
LGTM
Ensure the new scopes have been added to auth/roles. |
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.
6e7290a
to
f1754dd
Compare
Signed-off-by: Sam Lucidi <[email protected]>
5a6beca
to
dcfd9b2
Compare
No description provided.