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.

Sign up for GitHub

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

Jump to bottom

Order by name first and by id by default #337

Merged
merged 4 commits into from
Apr 26, 2021
Merged

Order by name first and by id by default #337

Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
e381bf2
Fixes #327
mmorel-35 Apr 22, 2021
c9c7663
id to sortable, id() to sortKey(), compareID to compareOrder
mmorel-35 Apr 22, 2021
0006693
sortKey for FServicePackage
mmorel-35 Apr 22, 2021
b16927c
Test coverage
mmorel-35 Apr 24, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
77 changes: 40 additions & 37 deletions file/types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,12 +38,12 @@ type FService struct {

// id is used for sorting.
func (s FService) id() string {
if s.ID != nil {
return *s.ID
}
if s.Name != nil {
return *s.Name
}
if s.ID != nil {
return *s.ID
}
return ""
}

Expand Down Expand Up @@ -218,12 +218,12 @@ type FRoute struct {

// id is used for sorting.
func (r FRoute) id() string {
if r.ID != nil {
return *r.ID
}
if r.Name != nil {
return *r.Name
}
if r.ID != nil {
return *r.ID
}
return ""
}

Expand All @@ -236,12 +236,12 @@ type FUpstream struct {

// id is used for sorting.
func (u FUpstream) id() string {
if u.ID != nil {
return *u.ID
}
if u.Name != nil {
return *u.Name
}
if u.ID != nil {
return *u.ID
}
return ""
}

Expand All @@ -253,12 +253,12 @@ type FTarget struct {

// id is used for sorting.
func (t FTarget) id() string {
mmorel-35 marked this conversation as resolved.
Show resolved Hide resolved
if t.ID != nil {
return *t.ID
}
if t.Target.Target != nil {
return *t.Target.Target
}
if t.ID != nil {
return *t.ID
}
return ""
}

Expand All @@ -275,12 +275,12 @@ type FCertificate struct {

// id is used for sorting.
func (c FCertificate) id() string {
if c.ID != nil {
return *c.ID
}
if c.Cert != nil {
return *c.Cert
}
if c.ID != nil {
return *c.ID
}
return ""
}

Expand All @@ -292,12 +292,12 @@ type FCACertificate struct {

// id is used for sorting.
func (c FCACertificate) id() string {
if c.ID != nil {
return *c.ID
}
if c.Cert != nil {
return *c.Cert
}
if c.ID != nil {
return *c.ID
}
return ""
}

Expand Down Expand Up @@ -446,22 +446,25 @@ func (p *FPlugin) UnmarshalJSON(b []byte) error {

// id is used for sorting.
func (p FPlugin) id() string {
if p.ID != nil {
return *p.ID
}
// concat plugin name and foreign relations
key := ""
key = *p.Name
if p.Consumer != nil {
key += *p.Consumer.ID
}
if p.Route != nil {
key += *p.Route.ID
if p.Name != nil {
Copy link
Member

Choose a reason for hiding this comment

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

non-blocker: you could simplify this. A plugin's name field can never be nil.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I thought so too, but then I realized that even if it's impossible, the sortkey() shouldn't trigger an error for that situation as it's responsability is to sort and not to validate what it is sorting. That's why I wrote it this way.

key := ""
key = *p.Name
if p.Consumer != nil {
key += *p.Consumer.ID
}
if p.Route != nil {
key += *p.Route.ID
}
if p.Service != nil {
key += *p.Service.ID
}
return key
}
if p.Service != nil {
key += *p.Service.ID
if p.ID != nil {
return *p.ID
}
return key
return ""
}

// FConsumer represents a consumer in Kong.
Expand All @@ -480,12 +483,12 @@ type FConsumer struct {

// id is used for sorting.
func (c FConsumer) id() string {
if c.ID != nil {
return *c.ID
}
if c.Username != nil {
return *c.Username
}
if c.ID != nil {
return *c.ID
}
return ""
}

Expand Down Expand Up @@ -535,12 +538,12 @@ type FServicePackage struct {

// id is used for sorting.
func (s FServicePackage) id() string {
if s.ID != nil {
return *s.ID
}
if s.Name != nil {
return *s.Name
}
if s.ID != nil {
return *s.ID
}
return ""
}
Copy link
Member

Choose a reason for hiding this comment

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

The sort key for FServiceVersion seems to be missing and we should add that.
It can be done separately or part of this PR - either is fine.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There was neither any compareID() associated with this type, it seems like it was never sorted

Copy link
Member

Choose a reason for hiding this comment

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

I've added #348 to track it.
If you are interested, please take that on once you this PR is merged in.
Thanks you!


Expand Down