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 2648
Build 4422: differential-jessieJenkins
Build 4421: 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
120–121

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

121

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.

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.