1260556 - Mozreview sorts toplevel files before subdirectories during review, but then it *re-sorts them intermixed* with subdirectories when presenting final review feedback

Status: RESOLVED INVALID

(MozReview Graveyard :: General, defect)

Description

[Daniel Holbert [:dholbert]]

STR:
 1. Look at a mozreview page for a patch that touches some files in a directory, as well as in some subdirectories of that directory.

 2. Notice that the initial view sorts toplevel files first, and subdirectories afterwards.

 3. Add some review comments on toplevel files and on files in the subdirectories.

 4. Look at the sort order on the final page with the preview of your review comments.

ACTUAL RESULTS:
The comments on toplevel files are listed *last*, whereas on the previous page they were listed *first*.

EXPECTED RESULTS:
Consistent ordering.


This has tripped me up a few times, in cases where I've initially typed "Per my first comment above, ..." when really the first comment ends up being my last comment after MozReview reshuffles things in the final presentation of the comments.

Comment 1

[Daniel Holbert [:dholbert]]

Attachment: MozReview Request: Bug 1260556: This is just a test commit to attempt to demonstrate a mozreview bug. r?

Review commit: https://reviewboard.mozilla.org/r/43043/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/43043/

Comment 2

[Daniel Holbert [:dholbert]]

Comment on attachment 8736025 [details]
MozReview Request: Bug 1260556: This is just a test commit to attempt to demonstrate a mozreview bug. r?

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/43043/diff/1-2/

Comment 3

[Daniel Holbert [:dholbert]]

More concrete STR, using the review request that I've attached to this bug:
 1) Visit https://reviewboard.mozilla.org/r/43043/diff/2/
 2) Notice that layout/moz.build is listed first, and layout/base/moz.build is listed second. That's the order in which it has you review the changes.
 3) Add review notes for each of these files (and others if you like).
 4) Click "Finish Review" and look at the final ordering of your review comments.

Now mozreview lists your layout/base/moz.build comment first, and your layout/moz.build feedback second. (They've switched order.)

Comment 4

[Daniel Holbert [:dholbert]]

Attachment: screenshot 1: ordering when you do the review (mozreview sorts toplevel file first)

Comment 5

[Daniel Holbert [:dholbert]]

Attachment: screenshot 2: ordering after you click "finish review" (mozreview sorts toplevel file second now)

Comment 6

[Daniel Holbert [:dholbert]]

https://reviewboard.mozilla.org/r/43043/#review39637

[Here's a sample review demonstrating the confusion that this bug causes].

::: layout/base/moz.build:7
(Diff revision 2)
>  # vim: set filetype=python:
>  # This Source Code Form is subject to the terms of the Mozilla Public
>  # License, v. 2.0. If a copy of the MPL was not distributed with this
>  # file, You can obtain one at http://mozilla.org/MPL/2.0/.
>  
> +# Here are some edits

Same problem as explained above.

::: layout/moz.build:8
(Diff revision 2)
>  # This Source Code Form is subject to the terms of the Mozilla Public
>  # License, v. 2.0. If a copy of the MPL was not distributed with this
>  # file, You can obtain one at http://mozilla.org/MPL/2.0/.
>  
> +# Here are some sample edits to this file.
> +# Edit edit edit.

[This is the first file that I see and the first review comment that I type. The other review comments all refer back to this one.]
There's a problem with this code! Here's a big long explanation of the problem and the way that this should be fixed.

::: layout/svg/moz.build:7
(Diff revision 2)
>  # vim: set filetype=python:
>  # This Source Code Form is subject to the terms of the Mozilla Public
>  # License, v. 2.0. If a copy of the MPL was not distributed with this
>  # file, You can obtain one at http://mozilla.org/MPL/2.0/.
>  
> +# Edits edits edits.

Same problem as explained above.

::: layout/tables/moz.build:7
(Diff revision 2)
>  # vim: set filetype=python:
>  # This Source Code Form is subject to the terms of the Mozilla Public
>  # License, v. 2.0. If a copy of the MPL was not distributed with this
>  # file, You can obtain one at http://mozilla.org/MPL/2.0/.
>  
> +# Sample edits to this file now.

Same problem as explained above.

Comment 7

[BMO Automation]

MozReview is now obsolete. Please use Phabricator instead. Closing this bug.