Skip to content
This repository has been archived by the owner on Jun 21, 2024. It is now read-only.

Create UserLite Struct and add CreatedBy field in Device Struct #312

Merged
merged 4 commits into from
Nov 24, 2021

Conversation

cprivitere
Copy link
Contributor

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.

@@ -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 {
Copy link
Contributor

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.

Suggested change
type UserLite struct {
type UserLite struct {
*Href `json:",inline"`

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sounds good to me.

Copy link
Contributor Author

@cprivitere cprivitere Nov 22, 2021

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.

  1. 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.
  2. If you do *Href first and get it populated, to use it the code looks weird:
fmt.println(device.CreatedBy.Href.Href)

Copy link
Contributor

@displague displague Nov 23, 2021

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.

Copy link
Contributor Author

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.

@displague displague merged commit ce59639 into master Nov 24, 2021
@displague displague deleted the Adding_CreatedBy branch November 24, 2021 22:22
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants