Support for group actions ( team and repository creation)#111
Support for group actions ( team and repository creation)#111socialyadept wants to merge 22 commits intomainfrom
Conversation
WalkthroughAdds create ResourceActions for GitHub repositories and teams (arg parsing, validation, API calls, annotations, resource/entitlements/grants assembly); updates config Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant ActionHandler as Action Handler
participant GitHub as GitHub API
participant Builder as Resource Builder
participant Ent as Entitlements/Grants
Client->>ActionHandler: Invoke Create (repo/team args)
activate ActionHandler
ActionHandler->>ActionHandler: Validate & normalize args
ActionHandler->>GitHub: Create repo/team request
activate GitHub
GitHub-->>ActionHandler: Created resource + rate-limit info
deactivate GitHub
ActionHandler->>Builder: Build internal resource representation
Builder-->>ActionHandler: Resource object
ActionHandler->>Ent: Generate entitlements & fetch grants
Ent-->>ActionHandler: Entitlements + Grants
ActionHandler-->>Client: Return success, resource, entitlements, grants
deactivate ActionHandler
Estimated Code Review Effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
socialyadept
changed the title
vipulgowda
changed the title
vipulgowda
force-pushed
the
kumail/resource-action/teams/create-delete
branch
from
6513520 to
25ba291
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@pkg/connector/repository.go`:
- Around line 654-717: The collaborators and teams fetch only the first page;
update the calls to o.client.Repositories.ListCollaborators and
o.client.Repositories.ListTeams to iterate pages until no more results:
create/reuse ListOptions (collabOpts/teamOpts) with PerPage=maxPageSize and a
for loop that sets Page, calls the list method, appends returned users/teams to
the grants logic on each iteration, breaks when resp.NextPage == 0 (or
len(results)==0), and preserve existing error logging and per-item processing
(variables: collabOpts, users, teamOpts, teams, maxPageSize,
o.client.Repositories.ListCollaborators, o.client.Repositories.ListTeams).
- Around line 601-609: The visibility check currently logs via l.Warn and leaves
newRepo.Visibility unset; replace that silent fallback by validating the value
returned from actions.GetStringArg and handling invalid inputs explicitly:
either set an explicit default (e.g., newRepo.Visibility =
github.Ptr("private")) or, preferably, return an error to the caller (e.g.,
return fmt.Errorf("invalid visibility: %q", visibility)) so the create flow
fails fast; update the branch that currently calls l.Warn (around
actions.GetStringArg / newRepo.Visibility) to perform this explicit defaulting
or error return and ensure the provided visibility is included in the error
message.
In `@pkg/connector/team.go`:
- Around line 625-653: The ListTeamMembersByID call in the loop over
teamAccessLevels can return only the first page, so implement pagination: change
the single ListTeamMembersByID call to a paginated loop that sets
opts.ListOptions.Page (starting at 1) and repeatedly calls
o.client.Teams.ListTeamMembersByID(ctx, createdTeam.GetOrganization().GetID(),
createdTeam.GetID(), opts), processing members into grants (using userResource
and grant.NewGrant as before) and advancing to resp.NextPage until resp.NextPage
== 0; keep PerPage set to maxPageSize and preserve existing error handling and
logging when each page call fails.
🧹 Nitpick comments (2)
pkg/connector/team.go (1)
536-552: Consider returning an error for invalid privacy on nested teams.When a user requests
secretprivacy for a nested team, the code logs a warning but silently changes it toclosed. This could lead to unexpected results where the user thinks they created a secret team but it's actually closed.Consider either:
- Returning an error explaining the constraint
- Including the actual privacy setting in the return values so the caller knows what was applied
Option 1: Return an error for invalid request
if isNestedTeam { // Nested teams can only be "closed" if privacy == teamPrivacySecret { - l.Warn("github-connector: secret privacy not allowed for nested teams, using closed", - zap.String("requested_privacy", privacy), - ) + return nil, nil, fmt.Errorf("nested teams cannot have 'secret' privacy, only 'closed' is allowed") } newTeam.Privacy = github.Ptr(teamPrivacyClosed)pkg/connector/repository.go (1)
506-551: Hardcoded template lists may need periodic updates.The gitignore and license template options are hardcoded. While this provides a good UX, these lists may become outdated as GitHub adds new templates.
Consider:
- Documenting that these lists are static and may need updates
- Alternatively, using GitHub's APIs (
/gitignore/templatesand/licenses) to populate these dynamicallyThis is a minor maintenance concern and acceptable for now.
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Fix all issues with AI agents
In `@pkg/connector/repository.go`:
- Around line 483-606: The visibility option currently only supports "public"
and "private" which disallows creating enterprise-only "internal" repos; update
the config options for the "visibility" Field (the StringField options where
Value is "public"/"private") to include an {Value: "internal", DisplayName:
"Internal", Name: "..."} entry and modify handleCreateRepositoryAction to accept
"internal" as a valid visibility value (extend the if check that currently
allows only "public" or "private" to also allow "internal" before setting
newRepo.Visibility). Ensure the default behavior remains unchanged and no other
validation blocks the "internal" value.
- Around line 609-620: The code currently sets newRepo.GitignoreTemplate and
newRepo.LicenseTemplate without ensuring newRepo.AutoInit is true; update the
logic in the repository creation block (symbols: newRepo, AutoInit,
GitignoreTemplate, LicenseTemplate, actions.GetBoolArg, actions.GetStringArg) to
enforce GitHub's requirement: if either gitignore_template or license_template
is provided, ensure newRepo.AutoInit is true—if the user explicitly passed
add_readme=false return an error indicating templates require auto_init=true,
otherwise set newRepo.AutoInit = github.Ptr(true) before assigning the
templates.
In `@pkg/connector/team.go`:
- Around line 450-472: The argument descriptions for the fields named
"maintainers" and "repo_names" are misleading because both are
ResourceIdSliceField (parsed as numeric resource IDs) but the text implies
usernames/full names; update the DisplayName/Description strings for the fields
defined in this block (the "maintainers" and "repo_names" Field entries using
config.Field_ResourceIdSliceField / config.ResourceIdSliceField and
AllowedResourceTypeIds referencing resourceTypeUser.Id and
resourceTypeRepository.Id) to clearly state they expect resource IDs (e.g.,
"List of user resource IDs" and "List of repository resource IDs") so callers
understand these are ID selections not textual usernames/full names.
- Around line 536-548: The privacy branch currently only sets newTeam.Privacy
for allowed values and silently ignores invalid values for non-nested teams; add
an explicit validation for the "privacy" arg when isNestedTeam is false: if
privacy is provided and is not teamPrivacySecret or teamPrivacyClosed, return an
input/argument error (consistent with how invalid parent/maintainer IDs are
handled) instead of letting the GitHub API default it. Locate the block using
actions.GetStringArg(args, "privacy"), isNestedTeam, teamPrivacySecret,
teamPrivacyClosed and newTeam.Privacy and insert the validation+early return for
invalid non-nested privacy values.
7dbd0e8 to
4d833f0
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@pkg/connector/team.go`:
- Around line 551-570: Replace the if-else chain that handles privacy returned
from actions.GetStringArg with a switch statement using boolean cases to satisfy
gocritic's ifElseChain; specifically, switch on conditions such as "ok &&
privacy != \"\" && isNestedTeam" (log via l.Warn and set newTeam.Privacy =
github.Ptr(teamPrivacyClosed)), "ok && privacy != \"\" && (privacy ==
teamPrivacySecret || privacy == teamPrivacyClosed)" (set newTeam.Privacy =
github.Ptr(privacy)), "ok && privacy != \"\"" (return the same fmt.Errorf for
invalid privacy), and finally "isNestedTeam" (default nested-team case setting
newTeam.Privacy = github.Ptr(teamPrivacyClosed)); preserve existing calls to
actions.GetStringArg, l.Warn, github.Ptr, and the error return value.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@pkg/connector/team.go`:
- Around line 426-437: The config for the "privacy" field currently sets
DefaultValue: "closed", which preempts the handler logic that relies on GitHub
API to default non-nested teams to "secret"; remove the DefaultValue entry from
the config.Field_StringField for the "privacy" field so the framework won't
inject "closed" and the handler (the team creation/update logic that documents
"Default for non-nested teams is 'secret' (handled by GitHub API)" and
explicitly sets "closed" only for nested teams) can apply the correct default
based on whether a parent team is present.
- Around line 496-505: The CreateTeam response may omit Organization causing
teamResource profile orgID=0 and later Grants (Organizations.GetByID) to fail;
after obtaining parentResourceID (via actions.RequireResourceIDArg) and orgName
(o.orgCache.GetOrgName), parse parentResourceID with parseResourceToGitHub to
extract the orgID and, if createdTeam.GetOrganization() is nil or
createdTeam.GetOrganization().GetID()==0, set createdTeam.Organization (or
inject orgID into teamResource profile) to the parsed orgID so Grants and
membership code (e.g., Grants method calling Organizations.GetByID) always
receive a non-zero orgID. Ensure this same defensive injection is applied in the
other occurrence referenced (lines ~632-636).
| Name: "privacy", | ||
| DisplayName: "Privacy", | ||
| Description: "The level of privacy this team should have.", | ||
| Field: &config.Field_StringField{ | ||
| StringField: &config.StringField{ | ||
| Options: []*config.StringFieldOption{ | ||
| {Value: "secret", Name: "Secret is only visible to org owners and team members", DisplayName: "Secret"}, | ||
| {Value: "closed", Name: "Closed is visible to all org members. When parent team is set, this is the only allowed privacy level.", DisplayName: "Closed"}, | ||
| }, | ||
| DefaultValue: "closed", | ||
| }, | ||
| }, |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's examine the file and surrounding context
cat -n pkg/connector/team.go | sed -n '400,450p'Repository: ConductorOne/baton-github
Length of output: 2027
🏁 Script executed:
# Search for handler or comment about privacy defaults
rg -i "privacy" pkg/connector/team.go -B 3 -A 3 | head -50Repository: ConductorOne/baton-github
Length of output: 1961
🏁 Script executed:
# Look for nested team logic or parent team handling
rg -i "parent.*team|nested" pkg/connector/team.go -B 2 -A 2Repository: ConductorOne/baton-github
Length of output: 3391
🏁 Script executed:
# Check if there's documentation or tests about team privacy defaults
fd -t f "team" -e go | xargs rg -l "privacy.*default|default.*privacy" 2>/dev/nullRepository: ConductorOne/baton-github
Length of output: 53
🏁 Script executed:
# Look at the full privacy handling logic in the handler
rg -A 30 "Handle privacy with constraints" pkg/connector/team.goRepository: ConductorOne/baton-github
Length of output: 1435
🏁 Script executed:
# Check if there's documentation about when config DefaultValue is applied
rg -i "DefaultValue|default.*value" pkg/connector/team.go | head -20Repository: ConductorOne/baton-github
Length of output: 124
🏁 Script executed:
# Look for how the connector processes arguments with defaults
rg -B 5 -A 10 "GetStringArg.*privacy" pkg/connector/team.goRepository: ConductorOne/baton-github
Length of output: 717
Remove the DefaultValue to respect GitHub API's default for non-nested teams.
The config's DefaultValue: "closed" contradicts the handler's documented behavior. The handler comments explicitly state: "Default for non-nested teams is 'secret' (handled by GitHub API)" and only sets privacy to "closed" for nested teams. By setting a default in the config, the framework injects "closed" before the handler runs, preventing non-nested teams from defaulting to "secret" as intended. Remove the DefaultValue and let the handler logic apply the correct defaults based on team type.
🔧 Possible fix
StringField: &config.StringField{
Options: []*config.StringFieldOption{
{Value: "secret", Name: "Secret is only visible to org owners and team members", DisplayName: "Secret"},
{Value: "closed", Name: "Closed is visible to all org members. When parent team is set, this is the only allowed privacy level.", DisplayName: "Closed"},
},
- DefaultValue: "closed",
},🤖 Prompt for AI Agents
In `@pkg/connector/team.go` around lines 426 - 437, The config for the "privacy"
field currently sets DefaultValue: "closed", which preempts the handler logic
that relies on GitHub API to default non-nested teams to "secret"; remove the
DefaultValue entry from the config.Field_StringField for the "privacy" field so
the framework won't inject "closed" and the handler (the team creation/update
logic that documents "Default for non-nested teams is 'secret' (handled by
GitHub API)" and explicitly sets "closed" only for nested teams) can apply the
correct default based on whether a parent team is present.
| parentResourceID, err := actions.RequireResourceIDArg(args, "org") | ||
| if err != nil { | ||
| return nil, nil, err | ||
| } | ||
|
|
||
| // Get the organization name from the parent resource ID | ||
| orgName, err := o.orgCache.GetOrgName(ctx, parentResourceID) | ||
| if err != nil { | ||
| return nil, nil, fmt.Errorf("failed to get organization name: %w", err) | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cd /tmp && find . -name "team.go" -type f 2>/dev/null | head -20Repository: ConductorOne/baton-github
Length of output: 51
🏁 Script executed:
find . -name "team.go" -type f 2>/dev/null | grep -E "pkg/connector"Repository: ConductorOne/baton-github
Length of output: 92
🏁 Script executed:
ls -la pkg/connector/team.go 2>/dev/null || echo "File not found at expected location"Repository: ConductorOne/baton-github
Length of output: 140
🏁 Script executed:
git ls-files | grep -i team.goRepository: ConductorOne/baton-github
Length of output: 90
🏁 Script executed:
git ls-files | head -20Repository: ConductorOne/baton-github
Length of output: 457
🏁 Script executed:
find . -type f -name "*.go" | grep -i team | head -10Repository: ConductorOne/baton-github
Length of output: 657
🏁 Script executed:
git ls-files | grep connectorRepository: ConductorOne/baton-github
Length of output: 10181
🏁 Script executed:
sed -n '490,510p' pkg/connector/team.goRepository: ConductorOne/baton-github
Length of output: 666
🏁 Script executed:
sed -n '625,645p' pkg/connector/team.goRepository: ConductorOne/baton-github
Length of output: 858
🏁 Script executed:
grep -n "func teamResource" pkg/connector/team.goRepository: ConductorOne/baton-github
Length of output: 165
🏁 Script executed:
grep -n "func parseResourceToGitHub" pkg/connector/team.goRepository: ConductorOne/baton-github
Length of output: 51
🏁 Script executed:
grep -n "parseResourceToGitHub" pkg/connector/*.goRepository: ConductorOne/baton-github
Length of output: 441
🏁 Script executed:
sed -n '480,550p' pkg/connector/team.goRepository: ConductorOne/baton-github
Length of output: 2689
🏁 Script executed:
sed -n '600,700p' pkg/connector/team.goRepository: ConductorOne/baton-github
Length of output: 2837
🏁 Script executed:
sed -n '39,120p' pkg/connector/team.goRepository: ConductorOne/baton-github
Length of output: 2414
🏁 Script executed:
sed -n '81,95p' pkg/connector/helpers.goRepository: ConductorOne/baton-github
Length of output: 543
🏁 Script executed:
grep -n "Organization" pkg/connector/team.go | head -20Repository: ConductorOne/baton-github
Length of output: 282
🏁 Script executed:
grep -n "orgID" pkg/connector/team.go | head -20Repository: ConductorOne/baton-github
Length of output: 813
🏁 Script executed:
sed -n '200,280p' pkg/connector/team.goRepository: ConductorOne/baton-github
Length of output: 2578
🏁 Script executed:
grep -n "func (o \*teamResourceType) Grants" pkg/connector/team.goRepository: ConductorOne/baton-github
Length of output: 232
🏁 Script executed:
sed -n '250,330p' pkg/connector/team.goRepository: ConductorOne/baton-github
Length of output: 2431
🏁 Script executed:
sed -n '157,200p' pkg/connector/team.goRepository: ConductorOne/baton-github
Length of output: 1342
🏁 Script executed:
sed -n '157,210p' pkg/connector/team.goRepository: ConductorOne/baton-github
Length of output: 1643
🏁 Script executed:
grep -B5 -A5 "GetProfileInt64Value" pkg/connector/team.goRepository: ConductorOne/baton-github
Length of output: 665
🏁 Script executed:
cat > /tmp/test_nil.go << 'EOF'
package main
import "fmt"
func GetID() int64 {
return 0
}
func main() {
var org *interface{} = nil
// This would panic: org.GetID()
// Simulating what happens when org is nil
if org == nil {
fmt.Println("org is nil, calling GetID() on nil would panic")
}
}
EOF
cat /tmp/test_nil.goRepository: ConductorOne/baton-github
Length of output: 336
🏁 Script executed:
grep -n "type Team struct" vendor/github.com/google/go-github/v*/github/teams.goRepository: ConductorOne/baton-github
Length of output: 222
🏁 Script executed:
sed -n '1,150p' vendor/github.com/google/go-github/v69/github/teams.go | grep -A 20 "type Team struct"Repository: ConductorOne/baton-github
Length of output: 961
🏁 Script executed:
cat pkg/connector/team_test.goRepository: ConductorOne/baton-github
Length of output: 1775
🏁 Script executed:
grep -r "Organization" vendor/github.com/google/go-github/v69/github/teams.go | head -20Repository: ConductorOne/baton-github
Length of output: 458
🏁 Script executed:
sed -n '22,80p' vendor/github.com/google/go-github/v69/github/teams.goRepository: ConductorOne/baton-github
Length of output: 2743
🏁 Script executed:
grep -n "func (o \*Organization) GetID" vendor/github.com/google/go-github/v69/github/organizations.go | head -5Repository: ConductorOne/baton-github
Length of output: 164
🏁 Script executed:
grep -A2 "func (o \*Organization) GetID" vendor/github.com/google/go-github/v69/github/organizations.goRepository: ConductorOne/baton-github
Length of output: 164
🏁 Script executed:
grep -n "Organizations.GetByID" vendor/github.com/google/go-github/v69/github/organizations.goRepository: ConductorOne/baton-github
Length of output: 164
🏁 Script executed:
curl -s "https://api.github.com/repos/google/go-github/issues" 2>/dev/null | head -1 || echo "No internet access"Repository: ConductorOne/baton-github
Length of output: 70
🏁 Script executed:
grep -B5 -A5 "Organization" vendor/github.com/google/go-github/v69/github/teams.go | head -40Repository: ConductorOne/baton-github
Length of output: 1758
🏁 Script executed:
sed -n '550,620p' pkg/connector/team.goRepository: ConductorOne/baton-github
Length of output: 2864
🏁 Script executed:
sed -n '10,60p' vendor/github.com/google/go-github/v69/github/organizations.goRepository: ConductorOne/baton-github
Length of output: 174
🏁 Script executed:
find vendor -name "organizations.go" -path "*/google/go-github/*" | head -5Repository: ConductorOne/baton-github
Length of output: 51
🏁 Script executed:
sed -n '1,50p' vendor/github.com/google/go-github/v64/github/organizations.go | head -40Repository: ConductorOne/baton-github
Length of output: 174
🏁 Script executed:
find vendor -name "*.go" -path "*/google/go-github/*" -exec grep -l "func (o \*Organization) GetID" {} \;Repository: ConductorOne/baton-github
Length of output: 200
🏁 Script executed:
grep -r "func (o \*Organization) GetID" vendor/github.com/google/go-github/*/github/ | head -3Repository: ConductorOne/baton-github
Length of output: 278
🏁 Script executed:
grep -A2 "func (o \*Organization) GetID" vendor/github.com/google/go-github/v64/github/organizations.goRepository: ConductorOne/baton-github
Length of output: 164
🏁 Script executed:
grep -n "func (t Team) String" vendor/github.com/google/go-github/v69/github/teams.goRepository: ConductorOne/baton-github
Length of output: 103
🏁 Script executed:
grep -n "GetByID" vendor/github.com/google/go-github/v64/github/organizations.go | head -10Repository: ConductorOne/baton-github
Length of output: 164
🏁 Script executed:
grep -B2 -A10 "func (s \*OrganizationsService) GetByID" vendor/github.com/google/go-github/v64/github/organizations.goRepository: ConductorOne/baton-github
Length of output: 164
🏁 Script executed:
rg "func \(o \*Organization\) GetID" -A 3 vendor/github.com/google/go-github/v64/github/Repository: ConductorOne/baton-github
Length of output: 415
🏁 Script executed:
rg "CreateTeam" vendor/github.com/google/go-github/v64/github/teams.go -A 15 | head -40Repository: ConductorOne/baton-github
Length of output: 613
🏁 Script executed:
grep -r "createdTeam\|created.*Team" pkg/connector/team.goRepository: ConductorOne/baton-github
Length of output: 350
🏁 Script executed:
grep -B5 -A5 "Organization" pkg/connector/ | grep -A5 -B5 "nil"Repository: ConductorOne/baton-github
Length of output: 105
🏁 Script executed:
rg "GetByID.*0" pkg/connector/Repository: ConductorOne/baton-github
Length of output: 51
🏁 Script executed:
rg "Organizations.GetByID" pkg/connector/team.go -B2 -A5Repository: ConductorOne/baton-github
Length of output: 306
🏁 Script executed:
git log --oneline --all pkg/connector/team.go | head -20Repository: ConductorOne/baton-github
Length of output: 944
🏁 Script executed:
git log -p --all -S "Organization" -- pkg/connector/team.go | head -100Repository: ConductorOne/baton-github
Length of output: 51
🏁 Script executed:
grep -A3 "func (o \*Organization) GetID" vendor/github.com/google/go-github/v69/github/github-accessors.goRepository: ConductorOne/baton-github
Length of output: 151
🏁 Script executed:
git log --all --oneline | grep -i "organization\|orgid\|team" | head -30Repository: ConductorOne/baton-github
Length of output: 2014
🏁 Script executed:
git show 1226c15 --statRepository: ConductorOne/baton-github
Length of output: 385
🏁 Script executed:
git show 1226c15Repository: ConductorOne/baton-github
Length of output: 1459
Ensure orgID is always populated for the created team resource.
If the CreateTeam API response omits Organization, teamResource will store orgID=0 in the profile (since team.GetOrganization().GetID() safely returns 0 when nil). This causes failures in the Grants method (line 173) when it attempts Organizations.GetByID(ctx, orgID) with orgID=0, breaking subsequent grants and membership operations.
The suggested fix is sound: extract orgID from parentResourceID using parseResourceToGitHub() and inject it into createdTeam.Organization if missing. This approach aligns with the existing pattern from commit 1226c15, which handles the same scenario for nested teams.
🔧 Suggested defensive fix
parentResourceID, err := actions.RequireResourceIDArg(args, "org")
if err != nil {
return nil, nil, err
}
+
+ orgID, err := parseResourceToGitHub(parentResourceID)
+ if err != nil {
+ return nil, nil, err
+ }
// Get the organization name from the parent resource ID
orgName, err := o.orgCache.GetOrgName(ctx, parentResourceID)
if err != nil {
return nil, nil, fmt.Errorf("failed to get organization name: %w", err)
}
@@
createdTeam, resp, err := o.client.Teams.CreateTeam(ctx, orgName, newTeam)
if err != nil {
return nil, nil, wrapGitHubError(err, resp, fmt.Sprintf("failed to create team %s in org %s", name, orgName))
}
+
+ if createdTeam.GetOrganization().GetID() == 0 {
+ createdTeam.Organization = &github.Organization{ID: github.Ptr(orgID)}
+ }
// Create the resource representation of the newly created team
teamRes, err := teamResource(createdTeam, parentResourceID)Also applies to: 632-636
🤖 Prompt for AI Agents
In `@pkg/connector/team.go` around lines 496 - 505, The CreateTeam response may
omit Organization causing teamResource profile orgID=0 and later Grants
(Organizations.GetByID) to fail; after obtaining parentResourceID (via
actions.RequireResourceIDArg) and orgName (o.orgCache.GetOrgName), parse
parentResourceID with parseResourceToGitHub to extract the orgID and, if
createdTeam.GetOrganization() is nil or
createdTeam.GetOrganization().GetID()==0, set createdTeam.Organization (or
inject orgID into teamResource profile) to the parsed orgID so Grants and
membership code (e.g., Grants method calling Organizations.GetByID) always
receive a non-zero orgID. Ensure this same defensive injection is applied in the
other occurrence referenced (lines ~632-636).
socialyadept
force-pushed
the
kumail/resource-action/teams/create-delete
branch
from
78d4706 to
669099d
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@pkg/connector/repository_test.go`:
- Line 126: mgh.Seed() calls in the tests currently ignore their error returns;
update every call (the occurrences that mirror the working example at line 86)
to capture and handle the returned error (e.g., err := mgh.Seed(); if err != nil
{ t.Fatalf("mgh.Seed failed: %v", err) } or use your test helper like
require.NoError(t, err)). Change all instances of mgh.Seed() (including the ones
flagged at the other locations) to follow this pattern so the error is checked
and the linter stops failing.
In `@pkg/connector/team_test.go`:
- Line 226: The call to mgh.Seed() is ignoring its error return; update the test
to check the error (e.g., err := mgh.Seed(); require.NoError(t, err) or if err
!= nil { t.Fatalf("mgh.Seed() failed: %v", err) }) for both occurrences of
mgh.Seed() in team_test.go (the calls around the subtests where mgh is used),
matching the error-checking style used in the earlier subtest (line with
require.NoError for seeding).
pkg/connector/repository_test.go
Outdated
|
|
||
| t.Run("should create a public repository", func(t *testing.T) { | ||
| mgh := mocks.NewMockGitHub() | ||
| mgh.Seed() |
There was a problem hiding this comment.
Fix unchecked error return from mgh.Seed().
Static analysis correctly identifies that mgh.Seed() error return values are not being checked at lines 126, 151, 178, 201, and 225. This is inconsistent with line 86 which properly captures the return values, and will cause lint pipeline failures.
🔧 Proposed fix for all affected lines
t.Run("should create a public repository", func(t *testing.T) {
mgh := mocks.NewMockGitHub()
- mgh.Seed()
+ _, _, _, _, _, _ = mgh.Seed()Apply similar fix at lines 151, 178, 201, and 225. Alternatively, if return values aren't needed:
- mgh.Seed()
+ _, _, _, _, _, err := mgh.Seed()
+ require.NoError(t, err)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| mgh.Seed() | |
| _, _, _, _, _, _ = mgh.Seed() |
🧰 Tools
🪛 GitHub Check: go-lint
[failure] 126-126:
Error return value of mgh.Seed is not checked (errcheck)
🪛 GitHub Check: verify / lint
[failure] 126-126:
Error return value of mgh.Seed is not checked (errcheck)
🤖 Prompt for AI Agents
In `@pkg/connector/repository_test.go` at line 126, mgh.Seed() calls in the tests
currently ignore their error returns; update every call (the occurrences that
mirror the working example at line 86) to capture and handle the returned error
(e.g., err := mgh.Seed(); if err != nil { t.Fatalf("mgh.Seed failed: %v", err) }
or use your test helper like require.NoError(t, err)). Change all instances of
mgh.Seed() (including the ones flagged at the other locations) to follow this
pattern so the error is checked and the linter stops failing.
pkg/connector/team_test.go
Outdated
|
|
||
| t.Run("should fail with missing required org field", func(t *testing.T) { | ||
| mgh := mocks.NewMockGitHub() | ||
| mgh.Seed() |
There was a problem hiding this comment.
Fix unchecked error return from mgh.Seed().
Static analysis correctly identifies that mgh.Seed() error return values are not being checked at lines 226 and 246. This is inconsistent with other subtests (e.g., line 64) and will cause lint pipeline failures.
🔧 Proposed fix
t.Run("should fail with missing required org field", func(t *testing.T) {
mgh := mocks.NewMockGitHub()
- mgh.Seed()
+ _, _, _, _, _, _ = mgh.Seed()Apply the same fix at line 246.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| mgh.Seed() | |
| _, _, _, _, _, _ = mgh.Seed() |
🧰 Tools
🪛 GitHub Check: go-lint
[failure] 226-226:
Error return value of mgh.Seed is not checked (errcheck)
🪛 GitHub Check: verify / lint
[failure] 226-226:
Error return value of mgh.Seed is not checked (errcheck)
🤖 Prompt for AI Agents
In `@pkg/connector/team_test.go` at line 226, The call to mgh.Seed() is ignoring
its error return; update the test to check the error (e.g., err := mgh.Seed();
require.NoError(t, err) or if err != nil { t.Fatalf("mgh.Seed() failed: %v",
err) }) for both occurrences of mgh.Seed() in team_test.go (the calls around the
subtests where mgh is used), matching the error-checking style used in the
earlier subtest (line with require.NoError for seeding).
3 participants
Summary by CodeRabbit
New Features
Bug Fixes
Tests
Chores