action=upload does not have correct parameters marked as "required"
Open, Needs TriagePublicBUG REPORT
Actions

Description

Steps to Reproduce:
query the parameter info on sandbox for the upload module via
https://en.wikipedia.org/wiki/Special:ApiSandbox#action=paraminfo&format=json&modules=upload

Actual Results:
the JSON result shows the "required" field is only shown for "token" and its value is "" empty.

Expected Results:
According to recent discussion I had on IRC for wikimedia-commons, at a minimum it seems "filename" should also have a "required": true along with "token" ?
But there might be other fields that should have "required" set to true, I am unsure.
Generated docs I referred to are https://commons.wikimedia.org/w/api.php?action=help&modules=upload

In fact, I'm not sure if other APIs and modules are correctly setting their "required" field for parameters. This probably needs more thorough review at higher task level for some maintenance work by the API teams, so please tag this bug accordingly to sub-tasks or epics as necessary.

Event Timeline

Thadguidry created this task.Sep 22 2020, 7:11 PM

Restricted Application added a subscriber: Aklapper. · View Herald TranscriptSep 22 2020, 7:11 PM

Thadguidry updated the task description. (Show Details)Sep 22 2020, 7:12 PM

Reedy added a project: MediaWiki-API.Sep 22 2020, 7:18 PM

Restricted Application added a project: Platform Engineering. · View Herald TranscriptSep 22 2020, 7:18 PM

I think this is because there are some functions of the action=upload API module that don't need a filename. So as such, it's wrong to list it unconditionally in the help pages

But it will be enforced in code if it is needed for your workflow

Similar will occur for other modules, where they can be used in various different workflows, and as such, the parameters aren't necessarily always unconditionally needed

From line 476 onwards (the code before this is the aformentioned "upload stash" capability):

		// The following modules all require the filename parameter to be set
		if ( $this->mParams['filename'] === null ) {
			$this->dieWithError( [ 'apierror-missingparam', 'filename' ] );
		}

So while annotating it with required would not make sense, maybe annotating/improving the description to note when it is required

In the longer term, maybe the "upload stash" functionality should be in a seperate module etc, but that's a different point

holger.knust moved this task from Inbox to Feature Requests to Review on the Platform Engineering board.Sep 22 2020, 8:07 PM

DannyS712 added a subscriber: DannyS712.Sep 22 2020, 8:16 PM

action=upload does not have correct parameters marked as "required"Open, Needs TriagePublicBUG REPORTActions

Description

Event Timeline

action=upload does not have correct parameters marked as "required"
Open, Needs TriagePublicBUG REPORT
Actions