-
Notifications
You must be signed in to change notification settings - Fork 53
Create UserLite Struct and add CreatedBy field in Device Struct #312
Conversation
626c620
to
b044c40
Compare
@@ -68,6 +68,21 @@ type User struct { | |||
Staff bool `json:"staff,omitempty"` | |||
} | |||
|
|||
// UserLite is an abbreviated listing of an Equinix Metal user | |||
type UserLite struct { |
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.
Let's embed the Href
type instead of adding the URL
field. This will allow for common methods for all resources that use the Href pattern. See #269 for more details.
type UserLite struct { | |
type UserLite struct { | |
*Href `json:",inline"` |
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.
Sounds good to me.
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.
Two things I'm finding.
- If you use this along side the URL field we have defined, whichever comes first gets the href data, and the other gets nothing and returns nil or a blank.
- If you do *Href first and get it populated, to use it the code looks weird:
fmt.println(device.CreatedBy.Href.Href)
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 URL field would be removed. The Href embedding will be populated.
In practice we wouldn't access Href
via u.Href.Href
, we would use the accessor method of the Href
type, GetHref()
. (this method is in the following PR: https://github.com/packethost/packngo/pull/280/files#diff-ebde46360e5c028cc3bde9d49c49bfb6038d13cd340af1d2d61845323ec284fbR59). https://go.dev/doc/effective_go#embedding has some details on the topic.
With the Href
type methods proposed or discussed in that PR (SetHref GetHref), we will be able to fill in some gaps/inconsistencies of the API.
An example of these inconsistencies:
When an object returns {field: {href:"/field/id"}
, we can use ?include=field
to return that nested object.
However, the object returned in field
may (or may not, this is inconsistent) omit the href
field. The experience around .Href
will change based on query parameters. We can heal this with a GetHref
that could rebuild the URL based on the ID. Likewise, we can offer .GetID()
based on the basename of the path in the Href
field when the child object was not ?include
'd.
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.
Got it, since I'm assuming we don't want to break User.URL as part of this PR, we'll just do this methodology in UserLite as it's new.
Signed-off-by: cprivite <[email protected]>
Signed-off-by: cprivite <[email protected]>
Co-authored-by: Marques Johansson <[email protected]> Signed-off-by: cprivite <[email protected]>
Signed-off-by: cprivite <[email protected]>
de14c3a
to
2908145
Compare
metal-cli is unable to display the created_by field on devices. This is due to packngo not having that field in the device struct. This PR adds the created_by field to the device struct and also adds a new UserLite struct that is used for that created_by field.