Page MenuHomePhabricator

Home - limit "status" queries to 100 and show 99+ if we hit that
ClosedPublic

Authored by btrahan on Dec 11 2014, 10:39 PM.
Tags
None
Referenced Files
Unknown Object (File)
Thu, Apr 25, 4:29 PM
Unknown Object (File)
Thu, Apr 25, 4:29 PM
Unknown Object (File)
Wed, Apr 24, 11:48 PM
Unknown Object (File)
Tue, Apr 16, 10:34 AM
Unknown Object (File)
Thu, Apr 11, 8:02 AM
Unknown Object (File)
Mon, Apr 8, 1:59 PM
Unknown Object (File)
Sun, Apr 7, 8:37 AM
Unknown Object (File)
Sun, Mar 31, 11:28 PM
Subscribers

Details

Summary

Fixes T6595. This diff has two issues as is... 1) the differential data fetching is pretty cheesey, but it looks like we can't just issue three separate databases to get the right data? 2) the translations break, since I am turning this into a string (and not an int) so the whole pluralization bit fails. I think 1 is okay as is and 2 needs to be fixed though I am not sure how to best do that...

Test Plan

loaded home page and it looked nice...!

Diff Detail

Repository
rP Phabricator
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

btrahan retitled this revision from to Home - limit "status" queries to 100 and show 99+ if we hit that.
btrahan updated this object.
btrahan edited the test plan for this revision. (Show Details)
btrahan added a reviewer: epriestley.
epriestley edited edge metadata.

Oh, yuck. In theory, I think the translations have to go like this, ideally:

if ($count >= LIMIT) {
  $text = pht('99+ Things');
} else {
  $text = pht('%s Thing(s)', new PhutilNumber($count));
}

You could also do this:

$text = pht('(%d) %s Thing(s)', new PhutilNumber($count), $formatted_count);

...then translate it as just '%$2s Thing', '%$2s Thing(s)' -- i.e., discard the number in the output text. But this would be trickier for translators, and if there are any languages which have a noun form for "uncountably many things" or "a huge number of things" or similar, they wouldn't be able to use it with that approach. (I don't know if any such languages exist.)

src/applications/differential/application/PhabricatorDifferentialApplication.php
104

Maybe just do MAX_STATUS_ITEMS, and then if we hit the limit just render a single "99+ Active Reviews" bucket instead of breaking it up? I don't think the individual counts are too likely to be useful once you have 100+ of them, but I'm hesitant about showing an invalid number.

This revision is now accepted and ready to land.Dec 12 2014, 12:40 AM
btrahan edited edge metadata.
  • fix string construction, using a '%s things' if we hit limit and a '%d thing(s)' if we don't
  • provide two more translations that were shown missing in testing
  • refine differential to just say 'LIMIT-1+ Active Reviews' if we hit the limit there, else standard bucketing stuff
This revision was automatically updated to reflect the committed changes.