From 349c594489c0662f27302f1a746ac82332332cc4 Mon Sep 17 00:00:00 2001 From: "m.huber" Date: Mon, 21 Jul 2025 21:35:33 +0200 Subject: [PATCH 1/3] Refactor: make if statements more redable and have less checks as not needed --- routers/api/v1/repo/repo.go | 62 +++++++++++-------------------------- 1 file changed, 18 insertions(+), 44 deletions(-) diff --git a/routers/api/v1/repo/repo.go b/routers/api/v1/repo/repo.go index 292b267c0114b..aed0a163a5eb5 100644 --- a/routers/api/v1/repo/repo.go +++ b/routers/api/v1/repo/repo.go @@ -772,13 +772,8 @@ func updateRepoUnits(ctx *context.APIContext, opts api.EditRepoOption) error { var units []repo_model.RepoUnit var deleteUnitTypes []unit_model.Type - currHasIssues := repo.UnitEnabled(ctx, unit_model.TypeIssues) - newHasIssues := currHasIssues if opts.HasIssues != nil { - newHasIssues = *opts.HasIssues - } - if currHasIssues || newHasIssues { - if newHasIssues && opts.ExternalTracker != nil && !unit_model.TypeExternalTracker.UnitGlobalDisabled() { + if *opts.HasIssues && opts.ExternalTracker != nil && !unit_model.TypeExternalTracker.UnitGlobalDisabled() { // Check that values are valid if !validation.IsValidExternalURL(opts.ExternalTracker.ExternalTrackerURL) { err := errors.New("External tracker URL not valid") @@ -802,7 +797,7 @@ func updateRepoUnits(ctx *context.APIContext, opts api.EditRepoOption) error { }, }) deleteUnitTypes = append(deleteUnitTypes, unit_model.TypeIssues) - } else if newHasIssues && opts.ExternalTracker == nil && !unit_model.TypeIssues.UnitGlobalDisabled() { + } else if *opts.HasIssues && opts.ExternalTracker == nil && !unit_model.TypeIssues.UnitGlobalDisabled() { // Default to built-in tracker var config *repo_model.IssuesConfig @@ -829,23 +824,15 @@ func updateRepoUnits(ctx *context.APIContext, opts api.EditRepoOption) error { Config: config, }) deleteUnitTypes = append(deleteUnitTypes, unit_model.TypeExternalTracker) - } else if !newHasIssues { - if !unit_model.TypeExternalTracker.UnitGlobalDisabled() { - deleteUnitTypes = append(deleteUnitTypes, unit_model.TypeExternalTracker) - } - if !unit_model.TypeIssues.UnitGlobalDisabled() { - deleteUnitTypes = append(deleteUnitTypes, unit_model.TypeIssues) - } + } else if !*opts.HasIssues && !unit_model.TypeExternalTracker.UnitGlobalDisabled() { + deleteUnitTypes = append(deleteUnitTypes, unit_model.TypeExternalTracker) + } else if !*opts.HasIssues && !unit_model.TypeIssues.UnitGlobalDisabled() { + deleteUnitTypes = append(deleteUnitTypes, unit_model.TypeIssues) } } - currHasWiki := repo.UnitEnabled(ctx, unit_model.TypeWiki) - newHasWiki := currHasWiki if opts.HasWiki != nil { - newHasWiki = *opts.HasWiki - } - if currHasWiki || newHasWiki { - if newHasWiki && opts.ExternalWiki != nil && !unit_model.TypeExternalWiki.UnitGlobalDisabled() { + if *opts.HasWiki && opts.ExternalWiki != nil && !unit_model.TypeExternalWiki.UnitGlobalDisabled() { // Check that values are valid if !validation.IsValidExternalURL(opts.ExternalWiki.ExternalWikiURL) { err := errors.New("External wiki URL not valid") @@ -861,7 +848,7 @@ func updateRepoUnits(ctx *context.APIContext, opts api.EditRepoOption) error { }, }) deleteUnitTypes = append(deleteUnitTypes, unit_model.TypeWiki) - } else if newHasWiki && opts.ExternalWiki == nil && !unit_model.TypeWiki.UnitGlobalDisabled() { + } else if *opts.HasWiki && opts.ExternalWiki == nil && !unit_model.TypeWiki.UnitGlobalDisabled() { config := &repo_model.UnitConfig{} units = append(units, repo_model.RepoUnit{ RepoID: repo.ID, @@ -869,23 +856,15 @@ func updateRepoUnits(ctx *context.APIContext, opts api.EditRepoOption) error { Config: config, }) deleteUnitTypes = append(deleteUnitTypes, unit_model.TypeExternalWiki) - } else if !newHasWiki { - if !unit_model.TypeExternalWiki.UnitGlobalDisabled() { - deleteUnitTypes = append(deleteUnitTypes, unit_model.TypeExternalWiki) - } - if !unit_model.TypeWiki.UnitGlobalDisabled() { - deleteUnitTypes = append(deleteUnitTypes, unit_model.TypeWiki) - } + } else if !*opts.HasWiki && !unit_model.TypeExternalWiki.UnitGlobalDisabled() { + deleteUnitTypes = append(deleteUnitTypes, unit_model.TypeExternalWiki) + } else if !*opts.HasWiki && !unit_model.TypeWiki.UnitGlobalDisabled() { + deleteUnitTypes = append(deleteUnitTypes, unit_model.TypeWiki) } } - currHasPullRequests := repo.UnitEnabled(ctx, unit_model.TypePullRequests) - newHasPullRequests := currHasPullRequests - if opts.HasPullRequests != nil { - newHasPullRequests = *opts.HasPullRequests - } - if currHasPullRequests || newHasPullRequests { - if newHasPullRequests && !unit_model.TypePullRequests.UnitGlobalDisabled() { + if opts.HasPullRequests != nil && !unit_model.TypePullRequests.UnitGlobalDisabled() { + if *opts.HasPullRequests { // We do allow setting individual PR settings through the API, so // we get the config settings and then set them // if those settings were provided in the opts. @@ -953,18 +932,13 @@ func updateRepoUnits(ctx *context.APIContext, opts api.EditRepoOption) error { Type: unit_model.TypePullRequests, Config: config, }) - } else if !newHasPullRequests && !unit_model.TypePullRequests.UnitGlobalDisabled() { + } else { deleteUnitTypes = append(deleteUnitTypes, unit_model.TypePullRequests) } } - currHasProjects := repo.UnitEnabled(ctx, unit_model.TypeProjects) - newHasProjects := currHasProjects - if opts.HasProjects != nil { - newHasProjects = *opts.HasProjects - } - if currHasProjects || newHasProjects { - if newHasProjects && !unit_model.TypeProjects.UnitGlobalDisabled() { + if opts.HasProjects != nil && !unit_model.TypeProjects.UnitGlobalDisabled() { + if *opts.HasProjects { unit, err := repo.GetUnit(ctx, unit_model.TypeProjects) var config *repo_model.ProjectsConfig if err != nil { @@ -984,7 +958,7 @@ func updateRepoUnits(ctx *context.APIContext, opts api.EditRepoOption) error { Type: unit_model.TypeProjects, Config: config, }) - } else if !newHasProjects && !unit_model.TypeProjects.UnitGlobalDisabled() { + } else { deleteUnitTypes = append(deleteUnitTypes, unit_model.TypeProjects) } } From 2e141d820a46e5f2dc46c7355aad3c6f579b8895 Mon Sep 17 00:00:00 2001 From: "m.huber" Date: Mon, 21 Jul 2025 22:26:40 +0200 Subject: [PATCH 2/3] fix --- routers/api/v1/repo/repo.go | 11 +++++++---- tests/integration/api_repo_edit_test.go | 3 +-- 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/routers/api/v1/repo/repo.go b/routers/api/v1/repo/repo.go index aed0a163a5eb5..946a3f21ebe23 100644 --- a/routers/api/v1/repo/repo.go +++ b/routers/api/v1/repo/repo.go @@ -856,10 +856,13 @@ func updateRepoUnits(ctx *context.APIContext, opts api.EditRepoOption) error { Config: config, }) deleteUnitTypes = append(deleteUnitTypes, unit_model.TypeExternalWiki) - } else if !*opts.HasWiki && !unit_model.TypeExternalWiki.UnitGlobalDisabled() { - deleteUnitTypes = append(deleteUnitTypes, unit_model.TypeExternalWiki) - } else if !*opts.HasWiki && !unit_model.TypeWiki.UnitGlobalDisabled() { - deleteUnitTypes = append(deleteUnitTypes, unit_model.TypeWiki) + } else if !*opts.HasWiki { + if !unit_model.TypeExternalWiki.UnitGlobalDisabled() { + deleteUnitTypes = append(deleteUnitTypes, unit_model.TypeExternalWiki) + } + if !unit_model.TypeWiki.UnitGlobalDisabled() { + deleteUnitTypes = append(deleteUnitTypes, unit_model.TypeWiki) + } } } diff --git a/tests/integration/api_repo_edit_test.go b/tests/integration/api_repo_edit_test.go index 7de8910ee0651..e228da26e992b 100644 --- a/tests/integration/api_repo_edit_test.go +++ b/tests/integration/api_repo_edit_test.go @@ -53,9 +53,8 @@ func getRepoEditOptionFromRepo(repo *repo_model.Repository) *api.EditRepoOption hasWiki = true } else if unit, err := repo.GetUnit(db.DefaultContext, unit_model.TypeExternalWiki); err == nil { hasWiki = true - config := unit.ExternalWikiConfig() externalWiki = &api.ExternalWiki{ - ExternalWikiURL: config.ExternalWikiURL, + ExternalWikiURL: unit.ExternalWikiConfig().ExternalWikiURL, } } defaultBranch := repo.DefaultBranch From 76ab3f35a419ae3bab700fdfcd4896a235d87eee Mon Sep 17 00:00:00 2001 From: "m.huber" Date: Mon, 21 Jul 2025 22:36:09 +0200 Subject: [PATCH 3/3] same --- routers/api/v1/repo/repo.go | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/routers/api/v1/repo/repo.go b/routers/api/v1/repo/repo.go index 946a3f21ebe23..4d5b99f1e04cf 100644 --- a/routers/api/v1/repo/repo.go +++ b/routers/api/v1/repo/repo.go @@ -824,10 +824,13 @@ func updateRepoUnits(ctx *context.APIContext, opts api.EditRepoOption) error { Config: config, }) deleteUnitTypes = append(deleteUnitTypes, unit_model.TypeExternalTracker) - } else if !*opts.HasIssues && !unit_model.TypeExternalTracker.UnitGlobalDisabled() { - deleteUnitTypes = append(deleteUnitTypes, unit_model.TypeExternalTracker) - } else if !*opts.HasIssues && !unit_model.TypeIssues.UnitGlobalDisabled() { - deleteUnitTypes = append(deleteUnitTypes, unit_model.TypeIssues) + } else if !*opts.HasIssues { + if !unit_model.TypeExternalTracker.UnitGlobalDisabled() { + deleteUnitTypes = append(deleteUnitTypes, unit_model.TypeExternalTracker) + } + if !unit_model.TypeIssues.UnitGlobalDisabled() { + deleteUnitTypes = append(deleteUnitTypes, unit_model.TypeIssues) + } } }