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

🐛 Only marshal assessment sections if not empty #506

Merged
merged 1 commit into from
Oct 5, 2023

Conversation

mansam
Copy link
Collaborator

@mansam mansam commented Oct 5, 2023

Marshalling the Assessment resource's Sections field unconditionally results in populating the model's Sections with the byte string null, which breaks the test for empty sections, making it always look like the assessment is being imported with sections "as-is". To fix this, only marshal the sections in the Assessment's Model() method if they are not empty. Additionally, replace the if m.Sections == nil test with a test for length, which works on nil as well as empty slices.

Signed-off-by: Sam Lucidi <slucidi@redhat.com>
@mansam mansam requested a review from jortel October 5, 2023 20:22
Copy link
Contributor

@jortel jortel left a comment

Choose a reason for hiding this comment

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

LGTM

@mansam mansam merged commit e30dbbc into konveyor:main Oct 5, 2023
10 checks passed
aufi pushed a commit to aufi/tackle2-hub that referenced this pull request Oct 30, 2023
Marshalling the Assessment resource's `Sections` field unconditionally
results in populating the model's `Sections` with the byte string
`null`, which breaks the test for empty sections, making it always look
like the assessment is being imported with sections "as-is". To fix
this, only marshal the sections in the Assessment's `Model()` method if
they are not empty. Additionally, replace the `if m.Sections == nil`
test with a test for length, which works on nil as well as empty slices.

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