Follow up to #2044435: Convert pager.inc to a service 

+++ b/core/lib/Drupal/Core/Pager/PagerManagerInterface.php
@@ -0,0 +1,153 @@
+   *   $where = "status = 1";
+   *   $total = mymodule_select("SELECT COUNT(*) FROM data " . $where)->result();
+   *   $num_per_page = \Drupal::config('mymodule.settings')->get('num_per_page');

@catch:

I'm sure this is copy/pasted but there's no need for $where to be a variable here. Would be good to update the docs to use selectquery or similar (in a follow-up, appreciate this is probably copy and paste from the old example).

CommentFileSizeAuthor
#4 3087486-4.patch896 bytesAkashKumar07
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

kim.pepper created an issue. See original summary.

kim.pepper’s picture

kim.pepper
Primary language English
Location 🏄‍♂️🇦🇺Sydney, Australia
CreditAttribution: kim.pepper at PreviousNext commented
Issue summary: View changes
kim.pepper’s picture

kim.pepper
Primary language English
Location 🏄‍♂️🇦🇺Sydney, Australia
CreditAttribution: kim.pepper at PreviousNext commented
Issue tags: +Novice, +API Documentation
AkashKumar07’s picture

AkashKumar07 CreditAttribution: AkashKumar07 at OpenSense Labs commented
Status: Active » Needs review
FileSize
3087486-4.patch896 bytes
naresh_bavaskar’s picture

naresh_bavaskar
Location Pune, Maharashtra
CreditAttribution: naresh_bavaskar as a volunteer and at QED42 commented
Assigned: Unassigned » naresh_bavaskar
naresh_bavaskar’s picture

naresh_bavaskar
Location Pune, Maharashtra
CreditAttribution: naresh_bavaskar as a volunteer and at QED42 commented

The above patch fine for me.

naresh_bavaskar’s picture

naresh_bavaskar
Location Pune, Maharashtra
CreditAttribution: naresh_bavaskar as a volunteer and at QED42 commented
Status: Needs review » Reviewed & tested by the community
naresh_bavaskar’s picture

naresh_bavaskar
Location Pune, Maharashtra
CreditAttribution: naresh_bavaskar as a volunteer and at QED42 commented
Assigned: naresh_bavaskar » Unassigned
kim.pepper’s picture

kim.pepper
Primary language English
Location 🏄‍♂️🇦🇺Sydney, Australia
CreditAttribution: kim.pepper at PreviousNext commented

+1 rtbc

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 4: 3087486-4.patch, failed testing. View results

Krzysztof Domański’s picture

Krzysztof Domański
Location Poland
CreditAttribution: Krzysztof Domański at abventor commented
Status: Needs work » Reviewed & tested by the community

Random test failure #3103492: Random fail in WidgetUploadTest.

Gayathri J’s picture

Gayathri J CreditAttribution: Gayathri J at UniMity Solutions Pvt Limited commented

Hi team #4 patch is fine for me we can add WHERE condition directly looking good.

alexpott credited catch.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott at Thunder commented
Status: Reviewed & tested by the community » Fixed

Crediting @catch as it was a comment from @catch that led to this patch. Crediting @kim.pepper for creating the issue and reviewing it.

Backported to 8.8.x as a docs fix.

Committed and pushed fb4c658da8 to 9.0.x and 54ee2173f2 to 8.9.x and 0bd9bf2d37 to 8.8.x. Thanks!

  • alexpott committed fb4c658 on 9.0.x 
    Issue #3087486 by AkashkumarOSL, kim.pepper, catch: Update docs in...

  • alexpott committed 54ee217 on 8.9.x 
    Issue #3087486 by AkashkumarOSL, kim.pepper, catch: Update docs in...

  • alexpott committed 0bd9bf2 on 8.8.x 
    Issue #3087486 by AkashkumarOSL, kim.pepper, catch: Update docs in...

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.