Better repo API unit checks (#21130)

This PR would presumably
Fix #20522
Fix #18773
Fix #19069
Fix #21077

Fix #13622

-----

1. Check whether unit type is currently enabled
2. Check if it _will_ be enabled via opt
3. Allow modification as necessary


Signed-off-by: jolheiser <john.olheiser@gmail.com>
Co-authored-by: wxiaoguang <wxiaoguang@gmail.com>
Co-authored-by: 6543 <6543@obermui.de>
tokarchuk/v1.18
John Olheiser 2 years ago committed by GitHub
parent 904b324716
commit 8cd3237a9e
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
  1. 24
      modules/structs/repo.go
  2. 31
      routers/api/v1/repo/repo.go
  3. 18
      templates/swagger/v1_json.tmpl
  4. 3
      tests/e2e/e2e_test.go

@ -151,13 +151,13 @@ type EditRepoOption struct {
Template *bool `json:"template,omitempty"` Template *bool `json:"template,omitempty"`
// either `true` to enable issues for this repository or `false` to disable them. // either `true` to enable issues for this repository or `false` to disable them.
HasIssues *bool `json:"has_issues,omitempty"` HasIssues *bool `json:"has_issues,omitempty"`
// set this structure to configure internal issue tracker (requires has_issues) // set this structure to configure internal issue tracker
InternalTracker *InternalTracker `json:"internal_tracker,omitempty"` InternalTracker *InternalTracker `json:"internal_tracker,omitempty"`
// set this structure to use external issue tracker (requires has_issues) // set this structure to use external issue tracker
ExternalTracker *ExternalTracker `json:"external_tracker,omitempty"` ExternalTracker *ExternalTracker `json:"external_tracker,omitempty"`
// either `true` to enable the wiki for this repository or `false` to disable it. // either `true` to enable the wiki for this repository or `false` to disable it.
HasWiki *bool `json:"has_wiki,omitempty"` HasWiki *bool `json:"has_wiki,omitempty"`
// set this structure to use external wiki instead of internal (requires has_wiki) // set this structure to use external wiki instead of internal
ExternalWiki *ExternalWiki `json:"external_wiki,omitempty"` ExternalWiki *ExternalWiki `json:"external_wiki,omitempty"`
// sets the default branch for this repository. // sets the default branch for this repository.
DefaultBranch *string `json:"default_branch,omitempty"` DefaultBranch *string `json:"default_branch,omitempty"`
@ -165,25 +165,25 @@ type EditRepoOption struct {
HasPullRequests *bool `json:"has_pull_requests,omitempty"` HasPullRequests *bool `json:"has_pull_requests,omitempty"`
// either `true` to enable project unit, or `false` to disable them. // either `true` to enable project unit, or `false` to disable them.
HasProjects *bool `json:"has_projects,omitempty"` HasProjects *bool `json:"has_projects,omitempty"`
// either `true` to ignore whitespace for conflicts, or `false` to not ignore whitespace. `has_pull_requests` must be `true`. // either `true` to ignore whitespace for conflicts, or `false` to not ignore whitespace.
IgnoreWhitespaceConflicts *bool `json:"ignore_whitespace_conflicts,omitempty"` IgnoreWhitespaceConflicts *bool `json:"ignore_whitespace_conflicts,omitempty"`
// either `true` to allow merging pull requests with a merge commit, or `false` to prevent merging pull requests with merge commits. `has_pull_requests` must be `true`. // either `true` to allow merging pull requests with a merge commit, or `false` to prevent merging pull requests with merge commits.
AllowMerge *bool `json:"allow_merge_commits,omitempty"` AllowMerge *bool `json:"allow_merge_commits,omitempty"`
// either `true` to allow rebase-merging pull requests, or `false` to prevent rebase-merging. `has_pull_requests` must be `true`. // either `true` to allow rebase-merging pull requests, or `false` to prevent rebase-merging.
AllowRebase *bool `json:"allow_rebase,omitempty"` AllowRebase *bool `json:"allow_rebase,omitempty"`
// either `true` to allow rebase with explicit merge commits (--no-ff), or `false` to prevent rebase with explicit merge commits. `has_pull_requests` must be `true`. // either `true` to allow rebase with explicit merge commits (--no-ff), or `false` to prevent rebase with explicit merge commits.
AllowRebaseMerge *bool `json:"allow_rebase_explicit,omitempty"` AllowRebaseMerge *bool `json:"allow_rebase_explicit,omitempty"`
// either `true` to allow squash-merging pull requests, or `false` to prevent squash-merging. `has_pull_requests` must be `true`. // either `true` to allow squash-merging pull requests, or `false` to prevent squash-merging.
AllowSquash *bool `json:"allow_squash_merge,omitempty"` AllowSquash *bool `json:"allow_squash_merge,omitempty"`
// either `true` to allow mark pr as merged manually, or `false` to prevent it. `has_pull_requests` must be `true`. // either `true` to allow mark pr as merged manually, or `false` to prevent it.
AllowManualMerge *bool `json:"allow_manual_merge,omitempty"` AllowManualMerge *bool `json:"allow_manual_merge,omitempty"`
// either `true` to enable AutodetectManualMerge, or `false` to prevent it. `has_pull_requests` must be `true`, Note: In some special cases, misjudgments can occur. // either `true` to enable AutodetectManualMerge, or `false` to prevent it. Note: In some special cases, misjudgments can occur.
AutodetectManualMerge *bool `json:"autodetect_manual_merge,omitempty"` AutodetectManualMerge *bool `json:"autodetect_manual_merge,omitempty"`
// either `true` to allow updating pull request branch by rebase, or `false` to prevent it. `has_pull_requests` must be `true`. // either `true` to allow updating pull request branch by rebase, or `false` to prevent it.
AllowRebaseUpdate *bool `json:"allow_rebase_update,omitempty"` AllowRebaseUpdate *bool `json:"allow_rebase_update,omitempty"`
// set to `true` to delete pr branch after merge by default // set to `true` to delete pr branch after merge by default
DefaultDeleteBranchAfterMerge *bool `json:"default_delete_branch_after_merge,omitempty"` DefaultDeleteBranchAfterMerge *bool `json:"default_delete_branch_after_merge,omitempty"`
// set to a merge style to be used by this repository: "merge", "rebase", "rebase-merge", or "squash". `has_pull_requests` must be `true`. // set to a merge style to be used by this repository: "merge", "rebase", "rebase-merge", or "squash".
DefaultMergeStyle *string `json:"default_merge_style,omitempty"` DefaultMergeStyle *string `json:"default_merge_style,omitempty"`
// set to `true` to archive this repository. // set to `true` to archive this repository.
Archived *bool `json:"archived,omitempty"` Archived *bool `json:"archived,omitempty"`

@ -732,8 +732,13 @@ func updateRepoUnits(ctx *context.APIContext, opts api.EditRepoOption) error {
var units []repo_model.RepoUnit var units []repo_model.RepoUnit
var deleteUnitTypes []unit_model.Type var deleteUnitTypes []unit_model.Type
currHasIssues := repo.UnitEnabledCtx(ctx, unit_model.TypeIssues)
newHasIssues := currHasIssues
if opts.HasIssues != nil { if opts.HasIssues != nil {
if *opts.HasIssues && opts.ExternalTracker != nil && !unit_model.TypeExternalTracker.UnitGlobalDisabled() { newHasIssues = *opts.HasIssues
}
if currHasIssues || newHasIssues {
if newHasIssues && opts.ExternalTracker != nil && !unit_model.TypeExternalTracker.UnitGlobalDisabled() {
// Check that values are valid // Check that values are valid
if !validation.IsValidExternalURL(opts.ExternalTracker.ExternalTrackerURL) { if !validation.IsValidExternalURL(opts.ExternalTracker.ExternalTrackerURL) {
err := fmt.Errorf("External tracker URL not valid") err := fmt.Errorf("External tracker URL not valid")
@ -756,7 +761,7 @@ func updateRepoUnits(ctx *context.APIContext, opts api.EditRepoOption) error {
}, },
}) })
deleteUnitTypes = append(deleteUnitTypes, unit_model.TypeIssues) deleteUnitTypes = append(deleteUnitTypes, unit_model.TypeIssues)
} else if *opts.HasIssues && opts.ExternalTracker == nil && !unit_model.TypeIssues.UnitGlobalDisabled() { } else if newHasIssues && opts.ExternalTracker == nil && !unit_model.TypeIssues.UnitGlobalDisabled() {
// Default to built-in tracker // Default to built-in tracker
var config *repo_model.IssuesConfig var config *repo_model.IssuesConfig
@ -783,7 +788,7 @@ func updateRepoUnits(ctx *context.APIContext, opts api.EditRepoOption) error {
Config: config, Config: config,
}) })
deleteUnitTypes = append(deleteUnitTypes, unit_model.TypeExternalTracker) deleteUnitTypes = append(deleteUnitTypes, unit_model.TypeExternalTracker)
} else if !*opts.HasIssues { } else if !newHasIssues {
if !unit_model.TypeExternalTracker.UnitGlobalDisabled() { if !unit_model.TypeExternalTracker.UnitGlobalDisabled() {
deleteUnitTypes = append(deleteUnitTypes, unit_model.TypeExternalTracker) deleteUnitTypes = append(deleteUnitTypes, unit_model.TypeExternalTracker)
} }
@ -793,8 +798,13 @@ func updateRepoUnits(ctx *context.APIContext, opts api.EditRepoOption) error {
} }
} }
currHasWiki := repo.UnitEnabledCtx(ctx, unit_model.TypeWiki)
newHasWiki := currHasWiki
if opts.HasWiki != nil { if opts.HasWiki != nil {
if *opts.HasWiki && opts.ExternalWiki != nil && !unit_model.TypeExternalWiki.UnitGlobalDisabled() { newHasWiki = *opts.HasWiki
}
if currHasWiki || newHasWiki {
if newHasWiki && opts.ExternalWiki != nil && !unit_model.TypeExternalWiki.UnitGlobalDisabled() {
// Check that values are valid // Check that values are valid
if !validation.IsValidExternalURL(opts.ExternalWiki.ExternalWikiURL) { if !validation.IsValidExternalURL(opts.ExternalWiki.ExternalWikiURL) {
err := fmt.Errorf("External wiki URL not valid") err := fmt.Errorf("External wiki URL not valid")
@ -810,7 +820,7 @@ func updateRepoUnits(ctx *context.APIContext, opts api.EditRepoOption) error {
}, },
}) })
deleteUnitTypes = append(deleteUnitTypes, unit_model.TypeWiki) deleteUnitTypes = append(deleteUnitTypes, unit_model.TypeWiki)
} else if *opts.HasWiki && opts.ExternalWiki == nil && !unit_model.TypeWiki.UnitGlobalDisabled() { } else if newHasWiki && opts.ExternalWiki == nil && !unit_model.TypeWiki.UnitGlobalDisabled() {
config := &repo_model.UnitConfig{} config := &repo_model.UnitConfig{}
units = append(units, repo_model.RepoUnit{ units = append(units, repo_model.RepoUnit{
RepoID: repo.ID, RepoID: repo.ID,
@ -818,7 +828,7 @@ func updateRepoUnits(ctx *context.APIContext, opts api.EditRepoOption) error {
Config: config, Config: config,
}) })
deleteUnitTypes = append(deleteUnitTypes, unit_model.TypeExternalWiki) deleteUnitTypes = append(deleteUnitTypes, unit_model.TypeExternalWiki)
} else if !*opts.HasWiki { } else if !newHasWiki {
if !unit_model.TypeExternalWiki.UnitGlobalDisabled() { if !unit_model.TypeExternalWiki.UnitGlobalDisabled() {
deleteUnitTypes = append(deleteUnitTypes, unit_model.TypeExternalWiki) deleteUnitTypes = append(deleteUnitTypes, unit_model.TypeExternalWiki)
} }
@ -828,8 +838,13 @@ func updateRepoUnits(ctx *context.APIContext, opts api.EditRepoOption) error {
} }
} }
currHasPullRequests := repo.UnitEnabledCtx(ctx, unit_model.TypePullRequests)
newHasPullRequests := currHasPullRequests
if opts.HasPullRequests != nil { if opts.HasPullRequests != nil {
if *opts.HasPullRequests && !unit_model.TypePullRequests.UnitGlobalDisabled() { newHasPullRequests = *opts.HasPullRequests
}
if currHasPullRequests || newHasPullRequests {
if newHasPullRequests && !unit_model.TypePullRequests.UnitGlobalDisabled() {
// We do allow setting individual PR settings through the API, so // We do allow setting individual PR settings through the API, so
// we get the config settings and then set them // we get the config settings and then set them
// if those settings were provided in the opts. // if those settings were provided in the opts.
@ -889,7 +904,7 @@ func updateRepoUnits(ctx *context.APIContext, opts api.EditRepoOption) error {
Type: unit_model.TypePullRequests, Type: unit_model.TypePullRequests,
Config: config, Config: config,
}) })
} else if !*opts.HasPullRequests && !unit_model.TypePullRequests.UnitGlobalDisabled() { } else if !newHasPullRequests && !unit_model.TypePullRequests.UnitGlobalDisabled() {
deleteUnitTypes = append(deleteUnitTypes, unit_model.TypePullRequests) deleteUnitTypes = append(deleteUnitTypes, unit_model.TypePullRequests)
} }
} }

@ -15569,32 +15569,32 @@
"type": "object", "type": "object",
"properties": { "properties": {
"allow_manual_merge": { "allow_manual_merge": {
"description": "either `true` to allow mark pr as merged manually, or `false` to prevent it. `has_pull_requests` must be `true`.", "description": "either `true` to allow mark pr as merged manually, or `false` to prevent it.",
"type": "boolean", "type": "boolean",
"x-go-name": "AllowManualMerge" "x-go-name": "AllowManualMerge"
}, },
"allow_merge_commits": { "allow_merge_commits": {
"description": "either `true` to allow merging pull requests with a merge commit, or `false` to prevent merging pull requests with merge commits. `has_pull_requests` must be `true`.", "description": "either `true` to allow merging pull requests with a merge commit, or `false` to prevent merging pull requests with merge commits.",
"type": "boolean", "type": "boolean",
"x-go-name": "AllowMerge" "x-go-name": "AllowMerge"
}, },
"allow_rebase": { "allow_rebase": {
"description": "either `true` to allow rebase-merging pull requests, or `false` to prevent rebase-merging. `has_pull_requests` must be `true`.", "description": "either `true` to allow rebase-merging pull requests, or `false` to prevent rebase-merging.",
"type": "boolean", "type": "boolean",
"x-go-name": "AllowRebase" "x-go-name": "AllowRebase"
}, },
"allow_rebase_explicit": { "allow_rebase_explicit": {
"description": "either `true` to allow rebase with explicit merge commits (--no-ff), or `false` to prevent rebase with explicit merge commits. `has_pull_requests` must be `true`.", "description": "either `true` to allow rebase with explicit merge commits (--no-ff), or `false` to prevent rebase with explicit merge commits.",
"type": "boolean", "type": "boolean",
"x-go-name": "AllowRebaseMerge" "x-go-name": "AllowRebaseMerge"
}, },
"allow_rebase_update": { "allow_rebase_update": {
"description": "either `true` to allow updating pull request branch by rebase, or `false` to prevent it. `has_pull_requests` must be `true`.", "description": "either `true` to allow updating pull request branch by rebase, or `false` to prevent it.",
"type": "boolean", "type": "boolean",
"x-go-name": "AllowRebaseUpdate" "x-go-name": "AllowRebaseUpdate"
}, },
"allow_squash_merge": { "allow_squash_merge": {
"description": "either `true` to allow squash-merging pull requests, or `false` to prevent squash-merging. `has_pull_requests` must be `true`.", "description": "either `true` to allow squash-merging pull requests, or `false` to prevent squash-merging.",
"type": "boolean", "type": "boolean",
"x-go-name": "AllowSquash" "x-go-name": "AllowSquash"
}, },
@ -15604,7 +15604,7 @@
"x-go-name": "Archived" "x-go-name": "Archived"
}, },
"autodetect_manual_merge": { "autodetect_manual_merge": {
"description": "either `true` to enable AutodetectManualMerge, or `false` to prevent it. `has_pull_requests` must be `true`, Note: In some special cases, misjudgments can occur.", "description": "either `true` to enable AutodetectManualMerge, or `false` to prevent it. Note: In some special cases, misjudgments can occur.",
"type": "boolean", "type": "boolean",
"x-go-name": "AutodetectManualMerge" "x-go-name": "AutodetectManualMerge"
}, },
@ -15619,7 +15619,7 @@
"x-go-name": "DefaultDeleteBranchAfterMerge" "x-go-name": "DefaultDeleteBranchAfterMerge"
}, },
"default_merge_style": { "default_merge_style": {
"description": "set to a merge style to be used by this repository: \"merge\", \"rebase\", \"rebase-merge\", or \"squash\". `has_pull_requests` must be `true`.", "description": "set to a merge style to be used by this repository: \"merge\", \"rebase\", \"rebase-merge\", or \"squash\".",
"type": "string", "type": "string",
"x-go-name": "DefaultMergeStyle" "x-go-name": "DefaultMergeStyle"
}, },
@ -15660,7 +15660,7 @@
"x-go-name": "HasWiki" "x-go-name": "HasWiki"
}, },
"ignore_whitespace_conflicts": { "ignore_whitespace_conflicts": {
"description": "either `true` to ignore whitespace for conflicts, or `false` to not ignore whitespace. `has_pull_requests` must be `true`.", "description": "either `true` to ignore whitespace for conflicts, or `false` to not ignore whitespace.",
"type": "boolean", "type": "boolean",
"x-go-name": "IgnoreWhitespaceConflicts" "x-go-name": "IgnoreWhitespaceConflicts"
}, },

@ -72,8 +72,7 @@ func TestMain(m *testing.M) {
os.Exit(exitVal) os.Exit(exitVal)
} }
// TestE2e should be the only test e2e necessary. It will collect all "*.test.e2e.js" // TestE2e should be the only test e2e necessary. It will collect all "*.test.e2e.js" files in this directory and build a test for each.
// files in this directory and build a test for each.
func TestE2e(t *testing.T) { func TestE2e(t *testing.T) {
// Find the paths of all e2e test files in test test directory. // Find the paths of all e2e test files in test test directory.
searchGlob := filepath.Join(filepath.Dir(setting.AppPath), "tests", "e2e", "*.test.e2e.js") searchGlob := filepath.Join(filepath.Dir(setting.AppPath), "tests", "e2e", "*.test.e2e.js")

Loading…
Cancel
Save