Page MenuHomePhabricator

Longer Maniphest tasks not fully displayed (as Phabricator defines a result set limit as 100 rows)
Closed, ResolvedPublic

Description

Reported upstream: https://secure.phabricator.com/T7454

For example, T78424 does not have a "created this task." entry.

For a workaround (passing a "before" URL parameter to see the hidden parts), see T89690#1044413 below!

Event Timeline

Redrose64 raised the priority of this task from to Needs Triage.
Redrose64 updated the task description. (Show Details)
Redrose64 added a subscriber: Redrose64.
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptFeb 17 2015, 2:16 AM
Legoktm set Security to None.Feb 17 2015, 2:22 AM
Legoktm added subscribers: Quiddity, Legoktm.
Krenair added a subscriber: Krenair.
Aklapper changed the task status from Open to Stalled.EditedFeb 17 2015, 9:23 AM

It has. It shows

Quiddity created this task.Via Web · Dec 12 2014, 8:21 PM

here. You may have to click "Older changes are hidden. Show older changes." though. If it does not show that for you, please provide a screenshot of the area at the end of the initial comment / description.

Qgil added a subscriber: Qgil.Feb 17 2015, 9:31 AM

I don't see it either.

Aklapper renamed this task from Missing info on T78424 to Missing task creation info on T78424 (for non-admins?).Feb 17 2015, 9:52 AM
Aklapper changed the task status from Stalled to Open.
Aklapper triaged this task as Low priority.
Aklapper raised the priority of this task from Low to Normal.Feb 17 2015, 9:57 AM

Uhm, "Quiddity added a project: §Collaboration-Team." and everything done before that action (except for task description) is missing indeed when I am not logged in.
Missing transparency of actions = no good.
Wondering if we can find some pattern.

My guess is we need to loop in @Christopher as adding the sprint project had weird effects?

This is a hard coded result set limit derived from PhabricatorCursorPagedPolicyAwareQuery. The "show older" only appears if the user has interacted with the timeline. As far as I can tell from the code in PhabricatorApplicationTransactionView, this is the only paging method for the timeline.

However, you can put a parameter in the URL like https://phabricator.wikimedia.org/T78424?before=33 that will show the earliest transactions without interacting.

@Christopher: PhabricatorCursorPagedPolicyAwareQuery doesn't define $rawResultLimit though, so shouldn't it default to unlimited?

@mmodell I think that the limit is defined in AphrontCursorPagerView with the property

private $pageSize = 100;

Since the timeline is using a paging query without setting this for the condition of a non-interactive user, the default result limit is 100.

Christopher edited a custom field.Feb 18 2015, 3:57 PM
Christopher closed this task as Invalid.Mar 1 2015, 5:22 PM
Christopher moved this task from Backlog to Done on the Phabricator-Sprint-Extension board.
Redrose64 reopened this task as Open.Mar 2 2015, 12:31 AM

The problem still exists, so please don't close as "invalid" without explaining why it's invalid.

This is not a Sprint extension issue. The problem is resolved by either one of the following: Interact with the task by adding a comment so that it shows the "show older" dialog or by adding a parameter to the URL as explained above. If this is in fact "a problem", it is entirely upstream. It is an invalid issue because it is not a bug, this is expected behavior from the code.

The problem (which still exists, another example below) shows when viewing a Phabricator task, not when (for example) using Wikipedia, that is why I filed it as a Phabricator problem. Of that I am certain: but I have no idea what particular part of Phabricator causes this; I don't know what Sprint extension is; it was not me that first decided that it was a Sprint extension issue.

You can't claim that it is not a bug when people are left completely in the dark about absent comments in tasks. There is no hint at all that comments are missing, there are no help links to explain the absence, there are no links to click to reveal those hidden comments; there is nothing at mw:Phabricator/Help to indicate that parts of the task might not be displayed. If I cannot see the whole task, and am expected to use some undocumented URL hack, that is a bug.

I have also recently been directed to T18691, which judging by its number must be several years old, yet the earliest visible date is Mon, Feb 2, 7:48 AM (only one month ago), so the problem at T78424 is not a one-off.

Nobody denies there is a bug. :) The discussion above was whether this is a problem with the Sprint extension for Phabricator (it's not), or with Phabricator's code base itself.
Thanks for providing another testcase!

Christopher denies there is a bug - on Sun, Mar 1, 5:22 PM, he closed this task as "Invalid", and when I asked why, he said "It is an invalid issue because it is not a bug".

I still maintain that this is not a bug. Upstream defined a result set limit as 100 rows. Unless this is increased, there is no possibility of showing earlier transactions by default. How a "bug" is defined, by my accounting, is that it is a code oversight or error. This is entirely expected behavior. And, because Phacility makes the rules for how the application works, any problems with that behavior should be filed on secure.phabricator.com. And, there is nothing that can be done about it downstream, except complain to them,

Aklapper edited a custom field.

It may be entirely expected behavior for you - but where is it documented so that I (and others) might expect it? I also do not know what Upstream is (although Aklapper apparently does), nor who Phacility might be. Please remember that I am not a Phabricator developer, nor even a MediaWiki developer. I'm just an ordinary Wikipedia editor who has been trying to help at WP:VPT for over five years now, with over 3,000 edits to that page alone. At VPT, when somebody reports a problem and we advise people that a task already exists on Phabricator that covers their problem, we would like them to be able to read the whole thread without having to resort to undocumented workarounds in order to do so.

@Redrose64: Phabricator is a software package that we use for bug tracking. "Upstream" is a reference to (the phabricator project) which is maintained by another group. Some Wikimedia developers contribute code to that project when it is appropriate, and we maintain a relationship with the maintainers of phabricator. We do not control their decisions and cannot demand much from them, however, we can ask nicely and they might help when we are asking for something sensible.

Aklapper renamed this task from Missing task creation info on T78424 (for non-admins?) to Longer Maniphest tasks not fully displayed (as Phabricator defines a result set limit as 100 rows).Mar 3 2015, 9:40 AM
Aklapper added subscribers: wctaiwan, TheDJ.
Qgil added a comment.Mar 3 2015, 9:53 AM

I just tested this in https://phab-01.wmflabs.org/T669

So yes, this is how Phabricator works: regular users won't see more than 100 actions, admins will still see everything. I agree this limit sounds unnecessary to me. Now, what do we want to request?

  • Is it possible to remove this limit locally via configuration?
  • Should we ask upstream to remove this limit in Phabricator?
  • Should we ask to add a configuration option to set this limit?

regular users won't see more than 100 actions, admins will still see everything

(Cannot confirm: As an admin I don't see everything in T18691 either. Anyway, not really important.)

  • Is it possible to remove this limit locally via configuration?

$pageSize is private in /src/view/control/AphrontCursorPagerView.php and AphrontPagerView.php. Hence no config option but there is a final public function setPageSize(). Wondering if we could call/set that somewhere via custom code (extension?) without having to patch upstream code. (Probably not...)

  • Should we ask upstream to remove this limit in Phabricator?

Apart from Maniphest, I am not sure which Phabricator applications which use AphrontCursorPagerView with the default value this would affect (as my PHP skills are a catastrophy).
Would prefer to have a basic idea about potential performance and UX implications first.

  • Should we ask to add a configuration option to set this limit?

Need to understand the implications first (see previous section) before I can answer this. :)

The task history issue is more with "policy aware" paging than with the result set limit defined in AphrontCursorPagerView. The method executeWithCursorPager(AphrontCursorPagerView $pager) is used in 22 places in Phabricator, so it is not specific to Maniphest.

The functional problem is how to page with policy to a complete task history. The specific code comment in PhabricatorApplicationTransactionView->buildEvents() lines 129-132 that talks about this is

// If the viewer has interacted with this object, we hide things from
// before their most recent interaction by default. This tends to make
// very long threads much more manageable, because you don't have to
// scroll through a lot of history and can focus on just new stuff.

If show older is enabled for all users, the grouping that enables this feature would probably break. It is kind of a trade off. The "show older" feature which recognizes users who have interacted or showing everything.

Qgil added a comment.Mar 3 2015, 11:51 AM

But one thing is the collapsible bar, which is useful and easy to understand, and another is the fact that oldest items are removed from the history even when the page is expanded (which is not useful and impossible to understand unless someone like you checks what is happening at a source code level).

Jay8g added a subscriber: Jay8g.Mar 4 2015, 3:55 AM
Legoktm raised the priority of this task from Normal to High.Mar 4 2015, 8:16 AM
Aklapper updated the task description. (Show Details)Mar 4 2015, 9:12 AM
Qgil updated the task description. (Show Details)Mar 4 2015, 9:22 AM
Qgil moved this task from Need Discussion to Wikimedia requests on the Phabricator (Upstream) board.
Qgil added a comment.Mar 4 2015, 7:42 PM

... and fixed upstream within hours. As soon as we upgrade, a "Show Older" button will appear in these circumstances. For more details, check https://secure.phabricator.com/T7454

Does this fix the issue where clicking "Show older changes" doesn't show all of them? The upstream bug doesn't seem to make this clear.

Qgil added a comment.Mar 5 2015, 8:23 AM

I guess so, since that is the point of the bug report and the patch. I'm keeping this task open until we can test it after the next upgrade.

@wctaiwan: Yes it does (tested locally with a task with >100 comments)

chasemp lowered the priority of this task from High to Normal.Mar 11 2015, 9:11 PM
mmodell closed this task as Resolved.May 6 2015, 3:06 PM
mmodell claimed this task.
Restricted Application added a subscriber: TerraCodes. · View Herald TranscriptMay 23 2016, 6:03 PM