https://bugzilla.wikimedia.org/show_bug.cgi?id=16278
Roan Kattouw <[email protected]> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |[email protected] Keywords|need-review, patch | --- Comment #3 from Roan Kattouw <[email protected]> 2008-12-20 20:39:59 UTC --- (In reply to comment #2) > Created an attachment (id=5601) --> (https://bugzilla.wikimedia.org/attachment.cgi?id=5601) [details] > patch to add ApiReview.php (action=review) > Why the ==/=== change in ApiBase.php? > The proposed solution adds an API module ApiReview.php, which essentially > imitates the behavior of RevisionReview::AjaxReview. To make it work, it has > to > make two changes to other FlaggedRevs files: > *The code within FlaggedArticle::addQuickReview to generate the template and > image parameters had to be factored out to its own function to avoid code > duplication. > *RevisionReview::submit had to be changed to a public function to be > accessible > from the api. > I haven't reviewed the FlaggedRevs changes, Aaron should do that. Remarks on ApiReview.php: * You're doing an awful lot of permissions checking and validation that's probably duplicated in RevisionView::AjaxReview(). Instead of duplicating this code, it should be moved to a backend function called by both AjaxReview and ApiReview. * Use a boolean parameter instead of a 0/1 parameter for approve ** You can do this with 'approve' => false ** Setting &approve=anything produces $params['approve'] === true, not setting &approve produces $params['approve'] === false * The variable parameter thing kind of gives me the creeps here. It would be nicer to replace all the flag_* parameters by one multivalue parameter 'flags', with a variable list of allowed values: 'flags' => array( ApiBase :: PARAM_ISMULTI => true, ApiBase :: PARAM_TYPE => $allowed_flags ), where $allowed_flags is an array of allowed flags. You can then get these by iterating over $params['flags'], or by using $flags = array_flip($params['flags']); and using if(isset($flags['foo'])) > Caveat: The revision being reviewed has to be parsed/fetched from parser cache > on API call. Don't know if this will be a perfomance problem. > It looks kind of pointless to me: you're only parsing the page to see which images and templates are used, right? Why not get that info from the imagelinks and templatelinks tables? Does the existing FlaggedRevs code parse stuff too? -- Configure bugmail: https://bugzilla.wikimedia.org/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are on the CC list for the bug. _______________________________________________ Wikibugs-l mailing list [email protected] https://lists.wikimedia.org/mailman/listinfo/wikibugs-l
