Page MenuHomePhabricator

Investigate write mode flag cleanup for Flow API modules
Closed, ResolvedPublic

Description

Per https://gerrit.wikimedia.org/r/#/c/248668:

"I note that action=flow appears to ApiMain as a non-write module, even though some of its submodules are marked as write-mode."

Does this mean the main ApiMain checks done based on isWriteMode() must be duplicated in the Flow modules? Are there any bugs related to this?

In the context of the gerrit change, it would be nice for write actions to Flow to bail out when that Non-Write header is given.

Details

Related Gerrit Patches:

Event Timeline

aaron created this task.Dec 6 2015, 12:14 AM
aaron raised the priority of this task from to Needs Triage.
aaron updated the task description. (Show Details)
aaron added a project: StructuredDiscussions.
aaron added a subscriber: aaron.
Restricted Application added a project: Collaboration-Team-Triage. · View Herald TranscriptDec 6 2015, 12:14 AM
Restricted Application added subscribers: StudiesWorld, Aklapper. · View Herald Transcript
Legoktm added a subscriber: Legoktm.Jan 7 2016, 6:09 PM

We should probably split the write modules into an action=flow-write or something...

The way ApiFlow.php executes submodules is strange and duplicates code from ApiMain.php. We should refactor this to use things from ApiMain instead, so that we don't duplicate code and so that all the important checks in ApiMain are performed (token validation, write mode). Then if we need to add more checks in the future, we only need to add them to ApiMain, not also to Flow.

Change 327302 had a related patch set uploaded (by Catrope):
ApiFlow: Determine write mode on a per-submodule basis

https://gerrit.wikimedia.org/r/327302

https://gerrit.wikimedia.org/r/327302 should be helpful for Pywikibot-Flow. I think there was some discussion about the submodule write mode issue.

Change 327302 merged by jenkins-bot:
ApiFlow: Determine write mode on a per-submodule basis

https://gerrit.wikimedia.org/r/327302

Catrope closed this task as Resolved.May 9 2017, 6:18 PM
Catrope claimed this task.

(This was fixed months ago, but never closed)