rancher / dashboard

The Rancher UI
https://rancher.com
Apache License 2.0
457 stars 257 forks source link

Wrong sorting of various views and columns in Rancher UI #9782

Open gaktive opened 1 year ago

gaktive commented 1 year ago

Internal reference: SURE-6951 Reported in 2.7.6

Some columns of tables in the Rancher UI sort differently than one would expect. This mainly affects the "Flat List" but sometimes also grouped views.

Problems found:

Business impact: While this is not a major issue, it bugs the users and they'd like us to look at it.

Troubleshooting steps: The issue is visible when using sorting columns in the UI; this mainly affects the "Flat List" but sometimes also grouped views.

Workaround: none

Actual behaviour: Some fields during "sort" are arranged alphabetically when different sorting algorithms might be more suitable

Expected behaviour: Better sorting for some columns as listed in the "Problems found:" above

richard-cox commented 1 year ago

In the Cluster -> Projects/Namespaces view (Group by Project), projects without namespaces are not sorted alphabetically but always at the end.

I think this was an intentional design decision. Not sure where it came from, possibly customer based

In the Workload, Deployments and co views, the Restarts column may not be sorted, although this pure number is in the table.

This comes from a summation of the container restarts. As this is a computed property sorting will not be supported once server-side pagination is implemented

cnotv commented 1 year ago

Sorting is possible in the related section of Storybook and it will need to be extended. https://rancher.github.io/storybook/?path=/story/components-table--sorting

This would mean to add rows containing the data with these cases:

Sorting utility does already exists and should be extended or defined here. Tests for sorting needs to be added, since we don't want to use Lodash. A sorting function is also present in string utils as well, which should be moved.

Other mentioned cases require some pages/models/configurations adjustments.

cnotv commented 1 year ago

@bisht-richa as extension to our talk, I'll write a couple of notes here as reminder and easier to track.

The table is automatically recognizing the type, using JS results of the typeOf the sort utility, so it will do it entry by entry. I suspect it may be brittle for some edge cases, but over all it simplify what I was thinking/planning. It throws a lot of computation but that's out of our scope.

As a test, we may use the existing ones for the function which is finally called as "composition", then extend with these parameters:

    it.only.each([
      [[{ a: 1 }, { a: 9 }], ['a'], [{ a: 1 }, { a: 9 }]],
      [[{ a: 2 }, { a: 1 }], ['a'], [{ a: 1 }, { a: 2 }]],
      [[{ a: '2' }, { a: '19' }], ['a'], [{ a: '2' }, { a: '19' }]],
      [[{ a: '19' }, { a: '2' }], ['a'], [{ a: '2' }, { a: '19' }]],
      [[{ a: undefined }, { a: '19' }], ['a'], [{ a: '19' }, { a: undefined }]],
      [[{ a: 'xx.xx.17.196' }, { a: 'xx.xx.17.2' }], ['a'], [{ a: 'xx.xx.17.2' }, { a: 'xx.xx.17.196' }]],
      [[{ a: 'xx.xx.17.196' }, { a: 'xx.xx.17.2' }], ['a'], [{ a: 'xx.xx.17.2' }, { a: 'xx.xx.17.196' }]],
      [[{ a: '6d4h' }, { a: '6m56s' }, { a: '23h' }], ['a'], [{ a: '6m56s' }, { a: '23h' }, { a: '6d4h' }]],
    ])('should sort by single property', (ary, key, expected) => {
      testSortBy(ary, key, expected);
    });

For the numbers we can convert with the Lodash util toNumber(), while for the dates we may use dayjs.format(), taking an eye to the supported formats we have coming from the user preferences as indicated in the Date component tests.

richard-cox commented 1 year ago

Following on from https://github.com/rancher/dashboard/issues/9782#issuecomment-1735809007, i think we should pause on these.

I went through all the requests, minus the one by design, and they will all be avoided when we do server side pagination leaving us back at square 1.

There's some different issues to address

cnotv commented 1 year ago

Beside there's already 2 points of 4, we can always fix it now and then improve it by moving the same function on the service (separated web?) worker.

richard-cox commented 10 months ago

I've closed the associated PR (which fixed two of the issues listed). Any fixes we make now will not apply for server side pagination (which is coming soon). I've tracked additional requests for them to support some of these based on basic columns where the value is in the resource in https://github.com/rancher/dashboard/issues/8527#issuecomment-1484891769, others will come in via https://github.com/rancher/rancher/issues/40771

Edit: @gaktive These aren't going to come for 2.8next1, so bumping to 2.8.x

richard-cox commented 4 months ago

We need to test the outstanding parts with the new vai backed api and come up with a definitive list for them to resolve