Skip to content
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

👻 Use JSON serializer everywhere #680

Merged
merged 1 commit into from
Aug 13, 2024
Merged

Conversation

mansam
Copy link
Collaborator

@mansam mansam commented Jun 28, 2024

Extends the use of the JSON serializer to the rest of the models.

@@ -77,7 +78,7 @@ loop:
}

func (r *MembershipResolver) cacheArchetypes() (err error) {
if len(r.archetypes) > 0 {
if r.archetypesCached {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Prevents repeatedly querying if the hub doesn't have any archetypes.

questionnaire.Thresholds, _ = json.Marshal(q.Thresholds)
questionnaire.RiskMessages = model.RiskMessages(q.RiskMessages)
questionnaire.Thresholds = model.Thresholds(q.Thresholds)
bytes, jErr := json.Marshal(q.Sections)
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Simpler to marshal and unmarshal the sections from the seed than to convert all of the nested structs.

@@ -17,28 +17,28 @@ var (
Questionnaire: api.Ref{
Name: questionnaire.Questionnaire1.Name,
},
Sections: []assessment.Section{
Sections: []api.Section{
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Package discrepancies for nested structs are because api.Section is defined as model.Section, so all the nested structs are from the model package.

{
Order: uint2ptr(1),
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

We made the Order required in the API, so no reason for it to continue to be a pointer.

@mansam mansam requested a review from jortel June 28, 2024 18:49
}
if m.Links != nil {
_ = json.Unmarshal(m.Links, &r.Links)
r.Labels = m.Labels
Copy link
Contributor

Choose a reason for hiding this comment

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

r.Links = []Link{} so it's not nil?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The Links field is omitempty so seemed like it was okay to let it be nil. No problem changing it if you disagree.

@@ -2080,7 +2072,7 @@ func (h *AnalysisHandler) archive(ctx *gin.Context, q *gorm.DB) (err error) {
db = db.Where("n.IssueID = i.ID")
db = db.Where("i.AnalysisID", m.ID)
db = db.Group("i.ID")
summary := []ArchivedIssue{}
summary := []model.ArchivedIssue{}
Copy link
Contributor

@jortel jortel Jun 28, 2024

Choose a reason for hiding this comment

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

type ArchivedIssue = model.ArchivedIssue

Shouldn't this be a []ArchivedIssue?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Seemed more appropriate to use the one from the model package because it's used directly in a query and stored on the model.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah. I misunderstood.

@@ -44,3 +44,55 @@ type Review struct {
ArchetypeID *uint `gorm:"uniqueIndex"`
Archetype *Archetype
}

Copy link
Contributor

Choose a reason for hiding this comment

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

//
// JSON Fields.
//

@mansam mansam closed this Jun 28, 2024
@mansam mansam reopened this Jun 28, 2024
@mansam mansam marked this pull request as draft June 28, 2024 21:08
@mansam mansam marked this pull request as ready for review July 2, 2024 12:15
Signed-off-by: Sam Lucidi <slucidi@redhat.com>
@mansam mansam merged commit bcbff9a into konveyor:main Aug 13, 2024
13 checks passed
dymurray pushed a commit that referenced this pull request Oct 11, 2024
Extends the use of the JSON serializer to the rest of the models.

Signed-off-by: Sam Lucidi <slucidi@redhat.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants