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

Fix multiple bugs in handling of branch_types.enabled #205

Conversation

barrywhart
Copy link

Fixes #183

@@ -181,7 +184,7 @@ func resourceBranchingModelsRead(ctx context.Context, d *schema.ResourceData, m
if err != nil {
return diag.FromErr(err)
}
branchingModelsReq, _ := client.Get(fmt.Sprintf("2.0/repositories/%s/%s/branching-model", owner, repo))
branchingModelsReq, _ := client.Get(fmt.Sprintf("2.0/repositories/%s/%s/branching-model/settings", owner, repo))
Copy link
Author

Choose a reason for hiding this comment

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

The old code used the wrong API. The two APIs are similar, but the old one omits disabled branches in branch_types, and it omits production if disabled. Using settings returns everything. Note that the code was already using settings for PUT operations. This change aligns GET and PUT to use the same URL.

@@ -206,6 +209,14 @@ func resourceBranchingModelsRead(ctx context.Context, d *schema.ResourceData, m
return diag.FromErr(decodeerr)
}

// Set default value for Enabled if it is nil
for _, branchType := range branchingModel.BranchTypes {
Copy link
Author

Choose a reason for hiding this comment

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

See the comment above (on BranchType) for an explanation of this code.

@@ -302,11 +313,6 @@ func flattenBranchModel(rp *BranchModel, typ string) []interface{} {
"name": rp.Name,
}

// if production branch is disabled it wont show up in response and will show up without the proerty if enabled
Copy link
Author

Choose a reason for hiding this comment

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

This workaround should no longer be needed now that we're using the settings URL.

Copy link
Owner

@DrFaust92 DrFaust92 left a comment

Choose a reason for hiding this comment

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

LGTM.

@DrFaust92 DrFaust92 merged commit 1e43554 into DrFaust92:master May 17, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Attempting to disable the branch_types for the project never seems to get "fixed".
2 participants