Add sort criteria to job overviews

Rewieved @LKS90's patch and changed from ASC to DESC to be the default sorting criteria, since we think it makes more sense.

The patch is failing because of #2503663: Date sort_expose handler is missing "order" schema. Will need retest when that issue is commited.

Providing screenshot of the UI change.

Rebased the patch manually :). We might need tests for this as well

This should pass the tests

Tests added, also added some sorting criteria which might come handy.

Seems fine, remember to add first the test-only first when uploading the patches, otherwise it changes to needs work.

Since it's a UI change, please always add screenshots. Otherwise it takes much longer to review...

+++ b/config/install/views.view.tmgmt_job_overview.yml
@@ -783,7 +783,147 @@ display:
+          exposed: true
...
+          exposed: true
...
+          exposed: true
...
+          exposed: true
...
+          exposed: true
...
+          exposed: true
...
+          exposed: true
...
+          exposed: true
...
+          exposed: true

I think that's just too many exposed filters for a UI that is still efficient in handling.

Here are the screenshots, and i disabled three that were not that usefull(sorting criterias)

If you disable these filters from being exposed, i guess you should drop them completely from the view.

I understand that we never know about a specific use case, but a good UI decides about reduction and keeps navigating through things easy. This also means not offering every possible option by default.

I really don't know if we should follow this issue. Core also never exposes sort criteria. A power user can do so if he needs it.

Checked the situation and we want to do it similarly to all Core views:

Most importantly we want to have the view sorted by default by the id / creation date.
Currently, the sort criteria is completely missing.

At the same time, please change order to first show the source and then the target language.
And while checking (separate issue plz) i also realised that the tmgmt_job_items view has no sort criteria...

okay, this is setting everything to sort it by id/creation. All those criterias are deleted now and the the source is first now. I'll create the other issue right away :).

You are still adding the sorts as exposed sorts.
I really only want to add the default sorting to the view before we discuss about anything else.
Note that your patch does not change anything like an order of source and target language with the exposed filters. You only do so in the sorts.

This only leaves us with the creation sort and the filtering are now ordered.

1. +++ b/src/Entity/ViewsData/JobViewsData.php
   @@ -34,6 +34,9 @@ class JobViewsData extends EntityViewsData {
          ),
   +      'sort' => array(
   +        'id' => 'standard',
   +      )
        );
        $data['tmgmt_job']['label'] = array(
          'title' => 'Label',
   @@ -42,6 +45,9 @@ class JobViewsData extends EntityViewsData {
   
   @@ -42,6 +45,9 @@ class JobViewsData extends EntityViewsData {
          'field' => array(
            'id' => 'tmgmt_entity_label',
          ),
   +      'sort' => array(
   +        'id' => 'standard',
   +      ),
   

   Those things are by design not sortable, they are calculated.

2. +++ b/src/Tests/TMGMTUiTest.php
   @@ -516,7 +516,12 @@ class TMGMTUiTest extends TMGMTTestBase {
        $job1 = $this->createJob();
   +    $this->drupalGet('/admin/tmgmt/jobs');
   +    $label = trim((string) $this->xpath('//table[@class="views-table views-view-table cols-9"]/tbody/tr')[0]->td[0]);
   +
        $job2 = $this->createJob();
   +    $this->drupalGet('/admin/tmgmt/jobs');
   +    $this->assertTrue($label, trim((string) $this->xpath('//table[@class="views-table views-view-table cols-9"]/tbody/tr')[0]->td[0]));
        $job1->set('translator', $translator1->id())->save();
   

   NO leading /.

Why not expose the sorting through the table settings? We don't have created in there right now but changed, but we could also sort by that by default, makes at least as much sense?

Hm, for me this is kinda two different issues.
Originally, we identified that there is no order in place at all. That's a significant bug.
And about source / target order we are also not consistent.

Adding exposed sort is like a second step feature to me and i'm not fully clear about it. I didn't want to think about it as long as the bug persist.
I think we should check what we need for users before adding more and more optional elements.

If you want to push things forward more quickly, fine with me.

im not sure if this is what we are looking for, let me know.

In general what i see is what i would have expected as the correct core-like fix before debatable UI/UX improvements...
You didn't yet provide an update about sort of the tmgmt_job_items as requested in #23.

+++ b/sources/content/src/Plugin/tmgmt/Source/ContentEntitySource.php
@@ -132,7 +132,7 @@ class ContentEntitySource extends SourcePluginBase {
-            if (is_array($value) && isset($value['#translate']) && $value['#translate'] == TRUE) {
+            if (isset($value['#translate']) && $value['#translate'] == TRUE) {

Are you really sure that $value is always an array?

And then there are so many other unrelated changes... Please provide a patch that is not mixed with other issues.

Here is the issue for tmgmt_job_items #2573101: Add sort criteria to Job Items and yes my bad the patch is not the right one :s.

Heres the real patch, (same as previous but rebased).

You are not addressing #23 yet, the patch doesn't has any sort criteria, just the default order.

As far as I understand we have to add the sort criteria similar to core, so you have to edit the view and in the table settings just check what fields do you want to make sortable. And then you can sort jobs by clicking on any field.

Thats how it was at the beginning, by that i mean patch #14.
Following

Adding exposed sort is like a second step feature to me and i'm not fully clear about it. I didn't want to think about it as long as the bug persist.
I think we should check what we need for users before adding more and more optional elements.

I think last patch is closer to what the maintainer wants. We should stick to the lastest patch and then implement the rest later.

Maybe this is a closer approach to what was originally wanted at first.

All points from discussion above are addressed and patch works well for me. But now we should choose some fields that worth to be ordered.

I'm confused ;)

#35 was correct ( and no, that it is not what #14 did)

* We make the fields sortable that *can* be sorted. Just create a few jobs and you'll see that operation links can't be shorted (results in an exception, actually) and same for progress, because that is information that we compute during display. So, all the others can but not those.
* Do not add *any* sort fields at all. Instead, configure the default sort order in the tabe settings, which, as I suggested above, should be changed DESC. The difference is that this default sort order is then visible when you go there and you know what you can sort on.

Okay, this is the approach we are looking for then.

Thanks. Committed.