Summary

Order DiffCommits by primary key by default

Review Request #10521 — Created April 9, 2019 and submitted Sept. 22, 2019, 12:38 p.m.

Information

Owner

brennie

Repository

Review Board

Branch

release-4.0.x

Bugs

Depends On

Reviewers

Groups

reviewboard

People

Description

There are a few places where we are querying for DiffCommits but aren't
providing an ORDER BY clause, so we get them in whatever order the
database provides. SQLite returns them by primary key, but MySQL does
not, which leads to commits being out of order is certain places (such
as the CommitListField in change descriptions).

We always want DiffCommits ordered by primary key, so we make that the
default. Any query in which this is not the case can explicitly provide
an ORDER BY clause.

Testing Done

Ran unit tests.

Commits

Summary	ID
Order DiffCommits by primary key by default There are a few places where we are querying for DiffCommits but aren't providing an `ORDER BY` clause, so we get them in whatever order the database provides. SQLite returns them by primary key, but MySQL does not, which leads to commits being out of order is certain places (such as the CommitListField in change descriptions). We always want DiffCommits ordered by primary key, so we make that the default. Any query in which this is not the case can explicitly provide an `ORDER BY` clause.	53a4e07cb3954ee583c79f4d6d79cd42ca553210

Summary

Order DiffCommits by primary key by default

There are a few places where we are querying for DiffCommits but aren't providing an `ORDER BY` clause, so we get them in whatever order the database provides. SQLite returns them by primary key, but MySQL does not, which leads to commits being out of order is certain places (such as the CommitListField in change descriptions). We *always* want DiffCommits ordered by primary key, so we make that the default. Any query in which this is not the case can explicitly provide an `ORDER BY` clause.

53a4e07cb3954ee583c79f4d6d79cd42ca553210

Issues

Description	From	Last Updated
Can you add a unit test for this?	david	Aug. 26, 2019, 6:11 p.m.
Extra space at the end.	chipx86	Sept. 10, 2019, 6:55 p.m.
I'd feel more comfortable if we asserted the exact order of the commits we get back. There are many sort …	chipx86	Sept. 10, 2019, 6:55 p.m.

flake8 passed.

JSHint passed.

Change Summary:

Added unit test.

Testing Done:

Ran unit tests.

Commits:

	Summary	ID
	Order DiffCommits by primary key by default There are a few places where we are querying for DiffCommits but aren't providing an `ORDER BY` clause, so we get them in whatever order the database provides. SQLite returns them by primary key, but MySQL does not, which leads to commits being out of order is certain places (such as the CommitListField in change descriptions). We always want DiffCommits ordered by primary key, so we make that the default. Any query in which this is not the case can explicitly provide an `ORDER BY` clause.	58717f89108afb8c3e33e467f103424c69300571
	Order DiffCommits by primary key by default There are a few places where we are querying for DiffCommits but aren't providing an `ORDER BY` clause, so we get them in whatever order the database provides. SQLite returns them by primary key, but MySQL does not, which leads to commits being out of order is certain places (such as the CommitListField in change descriptions). We always want DiffCommits ordered by primary key, so we make that the default. Any query in which this is not the case can explicitly provide an `ORDER BY` clause.	5cab4d875bf6273a62dee6fb837be93462263c51

Diff:

Revision 2 (+12)

Show changes

	reviewboard/diffviewer/models/diffcommit.py
	reviewboard/diffviewer/tests/test_diffcommit.py

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

The issue has been resolved. Show all issues

Can you add a unit test for this?

brennie Aug. 26, 2019, 6:11 p.m.

The original patch does include a unit test.

reviewboard/diffviewer/tests/test_diffcommit.py (Diff revision 2)
The issue has been resolved. Show all issues
```
Extra space at the end.
```

reviewboard/diffviewer/tests/test_diffcommit.py (Diff revision 2)

The issue has been resolved. Show all issues

I'd feel more comfortable if we asserted the exact order of the commits we get back. There are many sort orders in which this statement could still be true.

Commits:

	Summary	ID
	Order DiffCommits by primary key by default There are a few places where we are querying for DiffCommits but aren't providing an `ORDER BY` clause, so we get them in whatever order the database provides. SQLite returns them by primary key, but MySQL does not, which leads to commits being out of order is certain places (such as the CommitListField in change descriptions). We always want DiffCommits ordered by primary key, so we make that the default. Any query in which this is not the case can explicitly provide an `ORDER BY` clause.	5cab4d875bf6273a62dee6fb837be93462263c51
	Order DiffCommits by primary key by default There are a few places where we are querying for DiffCommits but aren't providing an `ORDER BY` clause, so we get them in whatever order the database provides. SQLite returns them by primary key, but MySQL does not, which leads to commits being out of order is certain places (such as the CommitListField in change descriptions). We always want DiffCommits ordered by primary key, so we make that the default. Any query in which this is not the case can explicitly provide an `ORDER BY` clause.	53a4e07cb3954ee583c79f4d6d79cd42ca553210

Diff:

Revision 3 (+12)

Show changes

	reviewboard/diffviewer/models/diffcommit.py
	reviewboard/diffviewer/tests/test_diffcommit.py

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

Ship it!

```
Ship It!
```

Status: Closed (submitted)

Change Summary:

Pushed to release-4.0.x (346b662)