Page MenuHomePhabricator

Fix for pagination button on scores page
ClosedPublic

Authored by Nehajha on Sep 20 2017, 2:26 PM.

Details

Maniphest Tasks
T116259: Pagination is not working on Aggregated reports page
Reviewers
Niharika
bd808
Commits
rWIEG2d7ad19b1974: Fix for pagination button on scores page
Patch without arc
git checkout -b D790 && curl -L https://phabricator.wikimedia.org/D790?download=true | git apply
Summary

Added SQL_CALC_FOUND_ROWS and pagination information

Bug: T116259

Diff Detail

Repository
rWIEG wikimedia-iegreview
Branch
T116259
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 2263
Build 3660: differential-jessieJenkins
Build 3659: arc lint + arc unit

Event Timeline

Nehajha created this revision.Sep 20 2017, 2:26 PM
Nehajha added a comment.EditedSep 20 2017, 4:03 PM

From what I have gathered so far, there was no pagination information for the aggregated scores page. I have tested this patch for some 70 proposals. Please let me know if there are any mistakes. Thanks

Nehajha updated this revision to Diff 2089.Sep 20 2017, 7:25 PM

Pagination information for scores page

Nehajha edited the summary of this revision. (Show Details)Sep 22 2017, 4:20 PM

The patch is ready to be reviewed.

Build has FAILED
Link to build: https://integration.wikimedia.org/ci/job/phabricator-jessie-diffs/723/
See console output for more information: https://integration.wikimedia.org/ci/job/phabricator-jessie-diffs/723/console

You need to make sure the build passes.

Nehajha updated this revision to Diff 2125.Oct 2 2017, 6:35 PM

Comment changes

Nehajha added a comment.EditedOct 2 2017, 6:55 PM

I think I need to combine these three revisions. Only then the build will pass. Could you please tell me how to combine the three revisions.

Nehajha updated this revision to Diff 2129.Oct 3 2017, 3:21 PM

Pagination information

Finally, the build has passed. The patch is ready to be reviewed.

I didn't have in data in my app setup so I added a new campaign but I couldn't access it. I got the following error -

Details

Type: Twig_Error_Runtime
Message: An exception has been thrown during the rendering of a template ("Array to string conversion") in "proposals/queue.html" at line 27.
File: /vagrant/srv/iegreview/vendor/twig/twig/lib/Twig/Template.php
Line: 178
Trace

#0 /vagrant/srv/iegreview/vendor/twig/twig/lib/Twig/Environment.php(332) : eval()'d code(190): Twig_Template->displayBlock('content', Array, Array)
#1 /vagrant/srv/iegreview/vendor/twig/twig/lib/Twig/Template.php(340): __TwigTemplate_841cfb4224242ce4ee5c683da6ab47c7c89cdb691dd41651083e81213fdcc5a5->doDisplay(Array, Array)
#2 /vagrant/srv/iegreview/vendor/twig/twig/lib/Twig/Template.php(314): Twig_Template->displayWithErrorHandling(Array, Array)
#3 /vagrant/srv/iegreview/vendor/twig/twig/lib/Twig/Environment.php(332) : eval()'d code(25): Twig_Template->display(Array, Array)
#4 /vagrant/srv/iegreview/vendor/twig/twig/lib/Twig/Template.php(340): __TwigTemplate_c71bc6dac1074470a9ac337606eb7a61bfacdf3ab9c787a9bd5d5658f00219ba->doDisplay(Array, Array)
#5 /vagrant/srv/iegreview/vendor/twig/twig/lib/Twig/Template.php(314): Twig_Template->displayWithErrorHandling(Array, Array)
#6 /vagrant/srv/iegreview/vendor/twig/twig/lib/Twig/Environment.php(332) : eval()'d code(30): Twig_Template->display(Array, Array)
#7 /vagrant/srv/iegreview/vendor/twig/twig/lib/Twig/Template.php(340): __TwigTemplate_f1fdeffb279d706541df9830e749b0513d0d502c57e06132281b603d2dc5e5c4->doDisplay(Array, Array)
#8 /vagrant/srv/iegreview/vendor/twig/twig/lib/Twig/Template.php(314): Twig_Template->displayWithErrorHandling(Array, Array)
#9 /vagrant/srv/iegreview/vendor/twig/twig/lib/Twig/Template.php(325): Twig_Template->display(Array)
#10 /vagrant/srv/iegreview/vendor/slim/views/Slim/Views/Twig.php(91): Twig_Template->render(Array)
#11 /vagrant/srv/iegreview/vendor/slim/slim/Slim/View.php(255): Slim\Views\Twig->render('proposals/queue...', NULL)
#12 /vagrant/srv/iegreview/vendor/slim/slim/Slim/View.php(243): Slim\View->fetch('proposals/queue...', NULL)
#13 /vagrant/srv/iegreview/vendor/slim/slim/Slim/Slim.php(757): Slim\View->display('proposals/queue...')
#14 [internal function]: Slim\Slim->render('proposals/queue...', Array)
#15 /vagrant/srv/iegreview/src/Controller.php(97): call_user_func_array(Array, Array)
#16 /vagrant/srv/iegreview/src/Controllers/Proposals/Queue.php(79): Wikimedia\IEGReview\Controller->__call('render', Array)
#17 /vagrant/srv/iegreview/src/Controllers/Proposals/Queue.php(79): Wikimedia\IEGReview\Controllers\Proposals\Queue->render('proposals/queue...', Array)
#18 [internal function]: Wikimedia\IEGReview\Controllers\Proposals\Queue->handleGet('1')
#19 /vagrant/srv/iegreview/src/Controller.php(88): call_user_func_array(Array, Array)
#20 /vagrant/srv/iegreview/src/App.php(536): Wikimedia\IEGReview\Controller->__invoke('1')
#21 [internal function]: Wikimedia\IEGReview\App->Wikimedia\IEGReview\{closure}('1')
#22 /vagrant/srv/iegreview/vendor/slim/slim/Slim/Route.php(462): call_user_func_array(Object(Closure), Array)
#23 /vagrant/srv/iegreview/vendor/slim/slim/Slim/Slim.php(1326): Slim\Route->dispatch()
#24 /vagrant/srv/iegreview/vendor/slim/slim/Slim/Middleware/Flash.php(85): Slim\Slim->call()
#25 /vagrant/srv/iegreview/vendor/slim/slim/Slim/Middleware/MethodOverride.php(92): Slim\Middleware\Flash->call()
#26 /vagrant/srv/iegreview/src/HeaderMiddleware.php(65): Slim\Middleware\MethodOverride->call()
#27 /vagrant/srv/iegreview/src/CsrfMiddleware.php(66): Wikimedia\IEGReview\HeaderMiddleware->call()
#28 /vagrant/srv/iegreview/vendor/slim/slim/Slim/Middleware/PrettyExceptions.php(67): Wikimedia\IEGReview\CsrfMiddleware->call()
#29 /vagrant/srv/iegreview/vendor/slim/slim/Slim/Slim.php(1271): Slim\Middleware\PrettyExceptions->call()
#30 /vagrant/srv/iegreview/src/App.php(134): Slim\Slim->run()
#31 /vagrant/srv/iegreview/public/index.php(47): Wikimedia\IEGReview\App->run()
#32 {main}

This was even before I could test this patch so the patch has nothing to do with it. I'm baffled because it's a fresh install and clean git history. I'll try to fix this later today/tomorrow and test your patch after that.

Adding @bd808 as a reviewer too.

bd808 requested changes to this revision.Jan 1 2018, 5:52 AM

Very close to correct, but see inline comments for needed corrections.

src/Controllers/Reports/Aggregated.php
121 ↗(On Diff #2129)

This should be the search of aggregated scores from line 133. What you have here now is a search of all proposals without regard for $campaign. In testing with only one campaign in your local database you won't see the difference, but it will not work as expected with the production data.

133 ↗(On Diff #2129)

This should just return $searchResults after line 121 is corrected.

This revision now requires changes to proceed.Jan 1 2018, 5:52 AM
Nehajha updated this revision to Diff 2457.Jan 1 2018, 10:10 AM

Added pagination information

Nehajha updated this revision to Diff 2458.Jan 1 2018, 10:14 AM

Added pagination information

bd808 accepted this revision.Jan 1 2018, 7:18 PM

Awesome! You can merge this with arc patch D790 && arc land.

This revision is now accepted and ready to land.Jan 1 2018, 7:18 PM

I can't seem to merge this patch. The error was

fatal: remote error: 
Upload denied for project 'wikimedia/iegreview'

I did provide the right username and password for Gerrit. Probably, I do not have upload permissions.

bd808 added a comment.Jan 4 2018, 4:18 AM
In D790#18735, @Nehajha wrote:

I did provide the right username and password for Gerrit. Probably, I do not have upload permissions.

Darn. I somehow didn't think of that. You are of course right that you do not have push rights to this repo.

I had trouble landing it myself. This seems at least in part due to MediaWiki-Vagrant cloning the repo from the gerrit mirror. After running git remote -v set-url origin ssh://vcs@git-ssh.wikimedia.org/diffusion/WIEG/iegreview.git to change that to point to the Phab repo instead I was able to land the patch.

This revision was automatically updated to reflect the committed changes.