Repository Configuration API has psuedo-required parameters

[jolheiser]

• Gitea version (or commit ref): latest
• Git version: try
• Database (use [x]):
  • PostgreSQL
  • MySQL
  • MSSQL
  • SQLite
• Can you reproduce the bug at https://try.gitea.io:
  • Yes (provide example URL)
  • No

Description

Related #13620

In the repository configuration API, we have pseudo-required parameters which are misleading and can be confusing for users.

My own quote below, the API skips entire sections if you fail to specify that the unit is enabled, regardless of whether it was already enabled or not.

Basically, because you didn't specify "has_pull_requests": true the API skipped the PR-related values entirely. 🙃

gitea/routers/api/v1/repo/repo.go

Lines 684 to 728 in 5a991ec

	if opts.HasPullRequests != nil {
	if *opts.HasPullRequests && !models.UnitTypePullRequests.UnitGlobalDisabled() {
	// 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.
	unit, err := repo.GetUnit(models.UnitTypePullRequests)
	var config *models.PullRequestsConfig
	if err != nil {
	// Unit type doesn't exist so we make a new config file with default values
	config = &models.PullRequestsConfig{
	IgnoreWhitespaceConflicts: false,
	AllowMerge: true,
	AllowRebase: true,
	AllowRebaseMerge: true,
	AllowSquash: true,
	}
	} else {
	config = unit.PullRequestsConfig()
	}
	if opts.IgnoreWhitespaceConflicts != nil {
	config.IgnoreWhitespaceConflicts = *opts.IgnoreWhitespaceConflicts
	}
	if opts.AllowMerge != nil {
	config.AllowMerge = *opts.AllowMerge
	}
	if opts.AllowRebase != nil {
	config.AllowRebase = *opts.AllowRebase
	}
	if opts.AllowRebaseMerge != nil {
	config.AllowRebaseMerge = *opts.AllowRebaseMerge
	}
	if opts.AllowSquash != nil {
	config.AllowSquash = *opts.AllowSquash
	}
	units = append(units, models.RepoUnit{
	RepoID: repo.ID,
	Type: models.UnitTypePullRequests,
	Config: config,
	})
	} else if !*opts.HasPullRequests && !models.UnitTypePullRequests.UnitGlobalDisabled() {
	deleteUnitTypes = append(deleteUnitTypes, models.UnitTypePullRequests)
	}
	}

This is somewhat contradictory to the description of the API route itself.

Edit a repository's properties. Only fields that are set will be changed.

[khmarbaise]

Can you tell me where the description for the api is maintained or is it generated from source code?

[6543]

Generated by comments in the sourcecode

Example:

gitea/routers/api/v1/repo/collaborators.go

Lines 21 to 47 in b651409

	// swagger:operation GET /repos/{owner}/{repo}/collaborators repository repoListCollaborators
	// ---
	// summary: List a repository's collaborators
	// produces:
	// - application/json
	// parameters:
	// - name: owner
	// in: path
	// description: owner of the repo
	// type: string
	// required: true
	// - name: repo
	// in: path
	// description: name of the repo
	// type: string
	// required: true
	// - name: page
	// in: query
	// description: page number of results to return (1-based)
	// type: integer
	// - name: limit
	// in: query
	// description: page size of results
	// type: integer
	// responses:
	// "200":
	// "$ref": "#/responses/UserList"

[khmarbaise]

I would take the opportunity to check if I can fix that...

[khmarbaise]

So after I have taken a deeper look into the swagger docs I'm a little bit unsure because the summary is very short part (of course) ... The question is: Could the following be the solution which from my point of view is a bit too short and misleading as well:

	// swagger:operation PATCH /repos/{owner}/{repo} repository repoEdit
	// ---
	// summary: Edit a repository's properties.
	// produces:
	// - application/json
	// parameters:

The previous one:

// summary": "Edit a repository's properties. Only fields that are set will be changed.

Does not describe/mentioned the dependency between the fields like HasPullRequests ...
I could off course enhance the summary but it violates from my point of view the idea of summary... ?

One solution could be the following, but I'm not satisfied with that: What do you think?

	// swagger:operation PATCH /repos/{owner}/{repo} repository repoEdit
	// ---
	// summary: Edit a repository's properties. Please check different fields for dependencies between fields.
	// produces:
	// - application/json
	// parameters: