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

Fixes 143: Remove null fields from api responses #86

Merged
merged 1 commit into from Aug 26, 2022

Conversation

rverdile
Copy link
Contributor

@rverdile rverdile commented Aug 24, 2022

There were three fields I could find that were returning null:

  1. The bulk create error field, when there's no error.
    2. The bulk create repository field, when there is an error. leaving out because it's not causing a CI failure and it's not a simple fix
  2. The distribution versions field after a successful creation, if no versions were specified.

I changed these to:

  1. empty string
    2. empty object
  2. empty array

pkg/api/repository.go Outdated Show resolved Hide resolved
@@ -62,7 +62,7 @@ func (rc *RepositoryConfiguration) BeforeUpdate(tx *gorm.DB) (err error) {

func (rc *RepositoryConfiguration) DedupeVersions(tx *gorm.DB) (err error) {
var versionMap = make(map[string]bool)
var unique []string
var unique = make(pq.StringArray, 0)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this prevents distribution versions from returning null

@rverdile rverdile changed the title Fixes 143 - Remove null fields from api responses Fixes 143: Remove null fields from api responses Aug 24, 2022
@jlsherrill
Copy link
Member

@swadeley
Copy link
Member

Hi

Rebase please.

the ci.int.devshift check for this PR only failed on test_create_update_distro_version_is_unique, which is fixed by #85, so thats a good.

@mshriver
Copy link
Member

Because this contains openapi spec changes, we won't see the test pass in CI - the client generation is not dynamic. @swadeley can pull the openapi spec into our plugin ahead of merging this once we're confident in the content (above discussions about scope of nil values resolved).

@jlsherrill
Copy link
Member

this needs a rebase.

@mshriver
Copy link
Member

/retest

@rverdile
Copy link
Contributor Author

failure looks unrelated: swaggo/swag#1309

@swadeley swadeley merged commit 375cbce into content-services:main Aug 26, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants