[Bug 16278] FlaggedRevisions extension: API "sighting" of revisions
https://bugzilla.wikimedia.org/show_bug.cgi?id=16278 --- Comment #13 from Gurch 2008-12-22 13:36:46 UTC --- Thanks to everyone who worked on this. :) -- 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 Wikibugs-l@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikibugs-l
[Bug 16278] FlaggedRevisions extension: API "sighting" of revisions
https://bugzilla.wikimedia.org/show_bug.cgi?id=16278 Aaron Schulz changed: What|Removed |Added Status|NEW |RESOLVED Resolution||FIXED --- Comment #12 from Aaron Schulz 2008-12-21 21:02:44 UTC --- Done in r44880 -- 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 Wikibugs-l@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikibugs-l
[Bug 16278] FlaggedRevisions extension: API "sighting" of revisions
https://bugzilla.wikimedia.org/show_bug.cgi?id=16278 P.Copp changed: What|Removed |Added Attachment #5603 is|0 |1 obsolete|| --- Comment #11 from P.Copp 2008-12-21 10:50:26 UTC --- Created an attachment (id=5604) --> (https://bugzilla.wikimedia.org/attachment.cgi?id=5604) Another fix: Only try parser cache, when reviewing the current revision of a page -- 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 Wikibugs-l@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikibugs-l
[Bug 16278] FlaggedRevisions extension: API "sighting" of revisions
https://bugzilla.wikimedia.org/show_bug.cgi?id=16278 --- Comment #10 from Roan Kattouw 2008-12-21 00:21:22 UTC --- (In reply to comment #4) > (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. Fixed in r44864. -- 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 Wikibugs-l@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikibugs-l
[Bug 16278] FlaggedRevisions extension: API "sighting" of revisions
https://bugzilla.wikimedia.org/show_bug.cgi?id=16278 P.Copp changed: What|Removed |Added Attachment #5602 is|0 |1 obsolete|| --- Comment #9 from P.Copp 2008-12-21 00:09:24 UTC --- Created an attachment (id=5603) --> (https://bugzilla.wikimedia.org/attachment.cgi?id=5603) adapted patch to the changes of r44861 Thanks. I changed the patch accordingly. -- 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 Wikibugs-l@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikibugs-l
[Bug 16278] FlaggedRevisions extension: API "sighting" of revisions
https://bugzilla.wikimedia.org/show_bug.cgi?id=16278 --- Comment #8 from Aaron Schulz 2008-12-20 23:50:47 UTC --- Some of these changes made in r44861. -- 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 Wikibugs-l@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikibugs-l
[Bug 16278] FlaggedRevisions extension: API "sighting" of revisions
https://bugzilla.wikimedia.org/show_bug.cgi?id=16278 --- Comment #7 from Roan Kattouw 2008-12-20 21:50:08 UTC --- (In reply to comment #6) > Created an attachment (id=5602) --> (https://bugzilla.wikimedia.org/attachment.cgi?id=5602) [details] > changed parameter approve->unapprove, simplified code for extracting > flag_parameters a bit > The API part looks sane, but of course a lot of stuff should still be pushed to the backend. -- 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 Wikibugs-l@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikibugs-l
[Bug 16278] FlaggedRevisions extension: API "sighting" of revisions
https://bugzilla.wikimedia.org/show_bug.cgi?id=16278 P.Copp changed: What|Removed |Added Attachment #5601 is|0 |1 obsolete|| --- Comment #6 from P.Copp 2008-12-20 21:47:13 UTC --- Created an attachment (id=5602) --> (https://bugzilla.wikimedia.org/attachment.cgi?id=5602) changed parameter approve->unapprove, simplified code for extracting flag_parameters a bit -- 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 Wikibugs-l@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikibugs-l
[Bug 16278] FlaggedRevisions extension: API "sighting" of revisions
https://bugzilla.wikimedia.org/show_bug.cgi?id=16278 --- Comment #5 from P.Copp 2008-12-20 21:21:02 UTC --- (In reply to comment #4) > > 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 form 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. On second thought, we can't take the data from the *links tables, because a) they aren't always consistent and b) we can also flag old revisions, for which the data of the *link tables is obviously incorrect. -- 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 Wikibugs-l@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikibugs-l
[Bug 16278] FlaggedRevisions extension: API "sighting" of revisions
https://bugzilla.wikimedia.org/show_bug.cgi?id=16278 --- Comment #4 from P.Copp 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 Wikibugs-l@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikibugs-l
[Bug 16278] FlaggedRevisions extension: API "sighting" of revisions
https://bugzilla.wikimedia.org/show_bug.cgi?id=16278 Roan Kattouw changed: What|Removed |Added Keywords||need-review, patch -- 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 Wikibugs-l@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikibugs-l
[Bug 16278] FlaggedRevisions extension: API "sighting" of revisions
https://bugzilla.wikimedia.org/show_bug.cgi?id=16278 Roan Kattouw changed: What|Removed |Added CC||roan.katt...@home.nl Keywords|need-review, patch | --- Comment #3 from Roan Kattouw 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 Wikibugs-l@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikibugs-l
[Bug 16278] FlaggedRevisions extension: API "sighting" of revisions
https://bugzilla.wikimedia.org/show_bug.cgi?id=16278 P.Copp changed: What|Removed |Added Keywords||need-review, patch -- 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 Wikibugs-l@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikibugs-l
[Bug 16278] FlaggedRevisions extension: API "sighting" of revisions
https://bugzilla.wikimedia.org/show_bug.cgi?id=16278 --- Comment #2 from P.Copp 2008-12-20 20:18:01 UTC --- Created an attachment (id=5601) --> (https://bugzilla.wikimedia.org/attachment.cgi?id=5601) patch to add ApiReview.php (action=review) 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. 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. -- 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 Wikibugs-l@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikibugs-l
[Bug 16278] FlaggedRevisions extension: API "sighting" of revisions
https://bugzilla.wikimedia.org/show_bug.cgi?id=16278 Aaron Schulz changed: What|Removed |Added CC||jschulz_4...@msn.com --- Comment #1 from Aaron Schulz 2008-12-20 00:38:41 UTC --- The somewhat recent FlaggedRevision.php refactoring should make this a bit easier. -- 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 Wikibugs-l@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikibugs-l
[Bug 16278] FlaggedRevisions extension: API "sighting" of revisions
https://bugzilla.wikimedia.org/show_bug.cgi?id=16278 Gurch changed: What|Removed |Added Blocks||16633 -- 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 Wikibugs-l@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikibugs-l
[Bug 16278] FlaggedRevisions extension: API "sighting" of revisions
https://bugzilla.wikimedia.org/show_bug.cgi?id=16278 P. Birken changed: What|Removed |Added CC||pbir...@gmail.com -- Configure bugmail: https://bugzilla.wikimedia.org/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are watching all bug changes. ___ Wikibugs-l mailing list Wikibugs-l@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikibugs-l
