#4464 Sorting PRs by "last modified" renders wrong order
Closed: Fixed 4 years ago by jlanda. Opened 4 years ago by mhonek.


Metadata Update from @pingou:
- Issue tagged with: bug

4 years ago

I need some help with this one @pingou .

PullRequest object has both updated_on and last_updated column. Ordering, is based on last_updated . The one we are rendering on PR list on the "Modified x hours ago" line is updated_on.

adding a comment on the pr, udpates updated_on, while editing that comment updates last_updated . So this is a mess and quite hard to understand what's going on.

Before proceeding, what's the idea with this two columns? Is one for the PR itself's updates and the other a general one that includes it's associated comments? If so, who is who?

And what is the intent when ordering by last_updated, ordering by PR's update time, or their comment's update time?

Thanks in advance

PullRequest object has both updated_on and last_updated column. Ordering, is based on last_updated . The one we are rendering on PR list on the "Modified x hours ago" line is updated_on.

So we likely want to change the ordering to use updated_on (we may want to update that field when the PR is flagged, which we don't do apparently atm).

adding a comment on the pr, updates updated_on, while editing that comment updates last_updated

last_updated won't get updated on editing/adding a comment as these actions won't touch the pull_requests table, so I'm wondering if, for these actions, we shouldn't update both last_updated and updated_on.

I made some research around this...

Right now...

last_updated is updated on:
- touching the table
- add pr comment
- edit pr comment
- add assignee
- add metadata

updated_on is updated on:
- add assignee
- add pr comment
- add metadata

So...

So we likely want to change the ordering to use updated_on (we may want to update that
field when the PR is flagged, which we don't do apparently atm).

I was thinking on the other way. We can't update updated_on without updating last_updated so, we want to update just last_updated on comment actions leaving updated_on ontouched and order by & show last_updated

we want to update just last_updated on comment actions

The thing is that last_updated gets updated much more often than you listed there, it gets updated everytime a PR is merged, as when a PR is merged, the merge_status of all the other opened PR is reset.
So if we rely on last_updated we end up sorting the PR on a field that is updated everytime a PR is merged.

This was the main reason why we introduced updated_on and why we should order based on this field rather than last_updated.

we want to update just last_updated on comment actions

The thing is that last_updated gets updated much more often than you listed there, it gets updated everytime a PR is merged, as when a PR is merged, the merge_status of all the other opened PR is reset.
So if we rely on last_updated we end up sorting the PR on a field that is updated everytime a PR is merged.
This was the main reason why we introduced updated_on and why we should order based on this field rather than last_updated.

What do we want to update when flags are added?

Should I keep https://pagure.io/fork/jlanda/pagure/c/d8a2c1cd66659d4b75f18781717769e7b0f92b52 or should I update last_updated when a pr flag is added?

#4743 fixes the incoherent ordering and rendering thing: now the pull requests are ordered on the same key that is rendered as updated timestamp

Metadata Update from @jlanda:
- Issue close_status updated to: Fixed
- Issue status updated to: Closed (was: Open)

4 years ago

Login to comment on this ticket.

Metadata