https://bugzilla.wikimedia.org/show_bug.cgi?id=16278
--- Comment #4 from P.Copp <[email protected]> 2008-12-20 21:01:17 UTC --- (In reply to comment #3) > Why the ==/=== change in ApiBase.php? Forgot to mention it. Currently, if a parameter has array( 0, ...) as parameter type, the description says "Can be empty or..." instead of showing 0 as possible value. > * 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. True. I first considered using AjaxReview directly, but that would probably have been even more ugly. > * 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 Could do that, but would have to cast it to an integer later anyway, as RevisionReview expects 0 or 1 as value. > * 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'])) Hmm, I don't quite get that. Let's say we have 3 flags named A, B and C. A can have the values 0 or 1, B can have 0-2 and C 0-3. How would that work with multivalue params? Do you mean like it is done in ApiProtect and the expiry parameters? > > 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? The existing FlaggedRevs code can take the parameters from the parser output, as it displays a from on the already parsed page. I don't know if one could get all needed parameters from the *links tables, will have to ask Aaron about it. -- 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
