Defect #43965
closed
"Previous" and "Next" links in changeset view do not match the changeset list order
Description
In the changeset view, the "Previous" and "Next" links are determined by Changeset#previous and Changeset#next, which currently use only id order within the same repository.
However, the changeset list is ordered by committed_on DESC, id DESC in Repository#changesets.
Because of this difference, the "Previous" and "Next" links may not follow the same order as the changeset list.
Proposed fix¶
Make `Changeset#previous` and Changeset#next use the same ordering as Repository#changesets, namely committed_on DESC, id DESC.
A patch is attached for this fix.
Files
Updated by Go MAEDA 18 days ago
Thank you for posting the patch but it breaks test/unit/repository_mercurial_test.rb.
Failure: RepositoryMercurialTest#test_next [test/unit/repository_mercurial_test.rb:589]: --- expected +++ actual @@ -1 +1 @@ -#<Changeset id: 139, comments: "commit default branch.", commit_date: "2007-11-01", committed_on: "2007-10-31 16:00:00.000000000 +0000", committer: "test Ü <test00@foo.bar>", repository_id: 12, revision: "28", scmid: "3ae45e2d177d622e167b51eb8a23779c3878a3e2", user_id: nil> +#<Changeset id: 124, comments: "Copied 2 files", commit_date: "1989-01-01", committed_on: "1989-01-01 04:00:00.000000000 +0000", committer: "jsmith <jsmith@foo.bar>", repository_id: 12, revision: "13", scmid: "3a330eb329586ea2adb3f83237c23310e744ebe9", user_id: 2> bin/rails test test/unit/repository_mercurial_test.rb:581
Failure: RepositoryMercurialTest#test_previous_nil [test/unit/repository_mercurial_test.rb:577]: Expected #<Changeset id: 153, comments: "remove test.txt", commit_date: "2007-12-13", committed_on: "2007-12-13 09:22:42.000000000 +0000", committer: "jsmith <jsmith@foo.bar>", repository_id: 12, revision: "42", scmid: "ba20ebce08dbd2f0320b93faf7bba7c86186a1f7", user_id: 2> to be nil. bin/rails test test/unit/repository_mercurial_test.rb:570
Failure: RepositoryMercurialTest#test_previous [test/unit/repository_mercurial_test.rb:565]: --- expected +++ actual @@ -1 +1 @@ -#<Changeset id: 138, comments: "copy latin-1 path files to latin-1 subdir.", commit_date: "1981-01-01", committed_on: "1980-12-31 16:00:00.000000000 +0000", committer: "jsmith <jsmith@foo.bar>", repository_id: 12, revision: "27", scmid: "7bbf4c738e7145149d2e5eb1eed1d3a8ddd3b914", user_id: 2> +#<Changeset id: 150, comments: "remove test.txt", commit_date: "2007-10-03", committed_on: "2007-10-02 15:00:00.000000000 +0000", committer: "double\"quote\"user", repository_id: 12, revision: "39", scmid: "04aed9840e9266e535f5f20f7e42c9f9f84f9cf4", user_id: nil> bin/rails test test/unit/repository_mercurial_test.rb:557
Failure: RepositoryMercurialTest#test_next_nil [test/unit/repository_mercurial_test.rb:601]: Expected #<Changeset id: 111, comments: "Initial import.\nThe repository contains 3 files.", commit_date: "2007-12-14", committed_on: "2007-12-14 09:22:52.000000000 +0000", committer: "jsmith <jsmith@foo.bar>", repository_id: 12, revision: "0", scmid: "0885933ad4f68d77c2649cd11f8311276e7ef7ce", user_id: 2> to be nil. bin/rails test test/unit/repository_mercurial_test.rb:594
Updated by Takenori TAKAKI 17 days ago
Go MAEDA Thank you for checking the patch, and sorry for the regression.
The first patch changed Changeset#previous and Changeset#next to always use
committed_on/id ordering. This broke Mercurial repositories because
Repository::Mercurial intentionally orders changesets by id, which corresponds
to the Mercurial revision order used by the existing tests.
I have attached an updated patch. Changeset#previous and Changeset#next now
delegate the lookup to the repository. The default Repository implementation
uses committed_on/id ordering to match the changeset list order, while
Repository::Mercurial overrides it and keeps the existing id-based behavior.
I confirmed the following tests pass:
bin/rails test test/unit/changeset_test.rb 47 runs, 249 assertions, 0 failures, 0 errors, 0 skips bin/rails test test/unit/repository_mercurial_test.rb 41 runs, 480 assertions, 0 failures, 0 errors, 0 skips