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

render empty oauth scopes #290

Merged
merged 2 commits into from
May 11, 2024
Merged

render empty oauth scopes #290

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
2caecdf
render empty oauth scopes
khart-twilio May 9, 2024
2ba004f
Merge remote-tracking branch 'upstream/main' into khart-scopes-zero-i…
khart-twilio May 10, 2024
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
2 changes: 1 addition & 1 deletion datamodel/high/node_builder.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -429,7 +429,7 @@ func (n *NodeBuilder) AddYAMLNode(parent *yaml.Node, entry *nodes.NodeEntry) *ya
}

p := m.ToYamlNode(n, l)
if len(p.Content) > 0 {
if p.Content != nil {
valueNode = p
}
} else if r, ok := value.(Renderable); ok {
Expand Down
2 changes: 1 addition & 1 deletion datamodel/high/v3/oauth_flow.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ type OAuthFlow struct {
AuthorizationUrl string `json:"authorizationUrl,omitempty" yaml:"authorizationUrl,omitempty"`
TokenUrl string `json:"tokenUrl,omitempty" yaml:"tokenUrl,omitempty"`
RefreshUrl string `json:"refreshUrl,omitempty" yaml:"refreshUrl,omitempty"`
Scopes *orderedmap.Map[string, string] `json:"scopes,omitempty" yaml:"scopes,omitempty"`
Scopes *orderedmap.Map[string, string] `json:"scopes,renderZero" yaml:"scopes,renderZero"`
Extensions *orderedmap.Map[string, *yaml.Node] `json:"-" yaml:"-"`
low *low.OAuthFlow
}
Expand Down
3 changes: 3 additions & 0 deletions orderedmap/builder.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,9 @@ type findValueUntyped interface {
// ToYamlNode converts the ordered map to a yaml node ready for marshalling.
func (o *Map[K, V]) ToYamlNode(n NodeBuilder, l any) *yaml.Node {
p := utils.CreateEmptyMapNode()
if o != nil {
Copy link
Member

Choose a reason for hiding this comment

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

Can you add a small test for this, as this new block will reduced coverage, if you pull the project again and update the workflow config (upgraded the codecov action) You should see the coverage on this PR.

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 merged the latest change from main branch and ran go test -v -coverprofile cover.out ./... locally and see 100% coverage. In the status on this PR I see 1 workflow awaiting approval, which I assume means you need to approve something before the coverage check can run here, but I'm unfamiliar. I'd appreciate if you clarify for me.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

whoops, nevermind I was reading the test output incorrectly. I'll try to figure it out.

p.Content = make([]*yaml.Node, 0)
}

var vn *yaml.Node

Expand Down
Loading