Community
Participate
Working Groups
Build Identifier: M20090917-0800 In TaskComparator.sortByRank(...), when one of the RANK fields from the two compared tasks are null or empty, it just return 0 (zero). Like in other comparable fields, when the RANK is null or empty, it should be compared with the other value, because the other can be non null. This behavior generates a wrong order in the TaskView where tasks with null in the RANK field appears in a random position. Reproducible: Always Steps to Reproduce: 1. Create a filter with more than 3 tasks and two of than with the field RANK as null. 2. Sort the TaskView by RANK. 3. Update the RANK field of some tasks with null to a non null value. The order will remain the same.
Created attachment 159812 [details] patch soulving the problem This patch is a possible solution to this problems. The solution consists in convert to 0 the RANK of tasks with null. Simplifying the comparison and eliminating some ifs.
Shawn, is it intentional that we don't sort null values explicitly?
This may have been intentional to support categories in a sane way since the rank value would only make sense within a query of elements with the same ranking mechanism. When we added this support, we wanted to ensure that the original behavior was not affected by the addition of this sorting (i.e. categories were sorted by priority, but connectors with a rank could have their queries sorted by rank). I wonder if that really makes sense though.
The patch looks like the right fix to me. Julio, I'd be happy to apply it if you can provide a test case along with it. Shawn, wouldn't this still be backwards compatible since the rank would always be equal for all tasks that do not have it set and hence the sorter would fall through to the next criterion?
Created attachment 173836 [details] mylyn/context/zip
After looking at this again, it seems that this should work as expected and fall through to the next criterion.
I will provide a patch with a test case in org.eclipse.mylyn.tasks.tests.TaskListSorterTest Currently, in this class there is a test case called: testRankOrderSorting() I will create a new one called: testRankOrderSortingWithNullRank() OK? Suggestions?
Sounds perfect to me.
Created attachment 173945 [details] patch with test case This patch contains a test case testing null and empty RANK sorting.
Patch attached in previous comment. This patch has only the test case.
Thanks. Could you create a single patch with the both the code and test case? It makes tracking the contribution in the IP log easier.
Created attachment 173972 [details] problem solution + test case Sure. Here is the patch with both solution and test case. Thanks.
Thanks Julio. I have committed the patch to head and to the e_3_4_m_3_4_x branch.
