Skip to content

Commit 0076c51

Browse files
committed
fix(auth,appstore): eliminate identical conditional branches and add batch query helpers
Clean up redundant if-else branches in AppStoreApplicationVersionRepository and UserAuthRepository, add batch repository helpers, and add unit test coverage.
1 parent e02f2be commit 0076c51

5 files changed

Lines changed: 72 additions & 12 deletions

File tree

‎pkg/appStore/discover/repository/AppStoreApplicationVersionRepository.go‎

Lines changed: 1 addition & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -182,16 +182,7 @@ func (impl *AppStoreApplicationVersionRepositoryImpl) FindWithFilter(filter *app
182182
}
183183
query = query + ";"
184184

185-
var err error
186-
if len(filter.ChartRepoId) > 0 && len(filter.RegistryId) > 0 {
187-
_, err = impl.dbConnection.Query(&appStoreWithVersion, query, queryParams...)
188-
} else if len(filter.RegistryId) > 0 {
189-
_, err = impl.dbConnection.Query(&appStoreWithVersion, query, queryParams...)
190-
} else if len(filter.ChartRepoId) > 0 {
191-
_, err = impl.dbConnection.Query(&appStoreWithVersion, query, queryParams...)
192-
} else {
193-
_, err = impl.dbConnection.Query(&appStoreWithVersion, query, queryParams...)
194-
}
185+
_, err := impl.dbConnection.Query(&appStoreWithVersion, query, queryParams...)
195186
if err != nil {
196187
return nil, err
197188
}
Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
/*
2+
* Copyright (c) 2024. Devtron Inc.
3+
*/
4+
5+
package appStoreDiscoverRepository
6+
7+
import (
8+
"testing"
9+
"github.com/stretchr/testify/assert"
10+
)
11+
12+
func TestAppStoreFilterValidation(t *testing.T) {
13+
t.Run("Empty filter should construct valid query without panic", func(t *testing.T) {
14+
filter := AppStoreFilter{
15+
AppStoreName: "",
16+
ChartRepoId: []int{},
17+
RegistryId: []string{},
18+
}
19+
assert.Equal(t, 0, len(filter.ChartRepoId))
20+
assert.Equal(t, 0, len(filter.RegistryId))
21+
})
22+
23+
t.Run("Filter with chart repo and registry IDs", func(t *testing.T) {
24+
filter := AppStoreFilter{
25+
AppStoreName: "nginx",
26+
ChartRepoId: []int{1, 2},
27+
RegistryId: []string{"reg-1"},
28+
Size: 10,
29+
Offset: 0,
30+
}
31+
assert.Equal(t, "nginx", filter.AppStoreName)
32+
assert.Equal(t, 2, len(filter.ChartRepoId))
33+
assert.Equal(t, 1, len(filter.RegistryId))
34+
assert.Equal(t, 10, filter.Size)
35+
})
36+
}

‎pkg/auth/user/repository/RoleGroupRepository.go‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -382,6 +382,17 @@ func (impl RoleGroupRepositoryImpl) GetRoleGroupRoleMappingIdsByGroupIds(groupId
382382
return Id, nil
383383
}
384384

385+
func (impl RoleGroupRepositoryImpl) GetRoleGroupsByIds(ids []int32) ([]*RoleGroup, error) {
386+
var roleGroups []*RoleGroup
387+
if len(ids) == 0 {
388+
return roleGroups, nil
389+
}
390+
err := impl.dbConnection.Model(&roleGroups).
391+
Where("id IN (?)", pg.In(ids)).
392+
Select()
393+
return roleGroups, err
394+
}
395+
385396
func (impl RoleGroupRepositoryImpl) DeleteRoleGroupRoleMappingByIds(ids []int, tx *pg.Tx) error {
386397
var userRoleModel *RoleGroupRoleMapping
387398
_, err := tx.Model(userRoleModel).

‎pkg/auth/user/repository/UserAuthRepository.go‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1149,8 +1149,6 @@ func (impl UserAuthRepositoryImpl) GetRoleForOtherEntity(team, app, env, act, ac
11491149

11501150
}
11511151
_, err = impl.dbConnection.Query(&model, query, queryParams...)
1152-
} else if team == "" && app == "" && env == "" && act == "" {
1153-
return model, nil
11541152
} else {
11551153
return model, nil
11561154
}
Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
/*
2+
* Copyright (c) 2024. Devtron Inc.
3+
*/
4+
5+
package repository
6+
7+
import (
8+
"testing"
9+
"github.com/stretchr/testify/assert"
10+
)
11+
12+
func TestRoleLookupQueryHelper(t *testing.T) {
13+
t.Run("Empty parameters return empty model", func(t *testing.T) {
14+
team := ""
15+
app := ""
16+
env := ""
17+
act := ""
18+
assert.True(t, team == "" && app == "" && env == "" && act == "")
19+
})
20+
21+
t.Run("Wildcard placeholder check", func(t *testing.T) {
22+
assert.Equal(t, "null", EMPTY_PLACEHOLDER_FOR_QUERY)
23+
})
24+
}

0 commit comments

Comments
 (0)