Merge normal and continuous job overviews

See screenshots in #2676118: Improving continuous job form., if we do not hide the add form, we need a better UI.

Huh? This is about merging the two *job* overviews. And moving job items before/above jobs. Then there is no label question, it's simply "Jobs", like now?

I don't know how you want to merge Jobs into job items or have it hidden behind that? I'm just talking about switching the order around.

+++ b/src/Plugin/views/field/Progress.php
@@ -21,6 +21,10 @@ class Progress extends StatisticsBase {
   public function render(ResultRow $values) {
     $entity = $values->_entity;
+    // If job is continuous we don't show anything.
+    if ($entity->isContinuous()) {
+      return;
+    }

we also have to change the parent class to avoid that the data for this is loaded.

About the icon. All other states have no meaning for continuous. So I think a completely separate icon would be better.

1. diff --git a/config/install/views.view.tmgmt_continuous_job_overview.yml b/config/install/views.view.tmgmt_continuous_job_overview.yml
   deleted file mode 100644
   

   Now we have an interesting problem. We have an update function that creates this view.

   We need to make sure that updating from the previous beta version works. It doesn't need to result in the exactly same configuration, but it shouldn't break.

   So we likely need to at least disable the update function that adds that view. And maybe do some more changes.

   Test by installing beta2 (switch to that tag), then update to the latest version with your patch, run updates and see what happens.

   If it fails, fix it. If it kind of works but the is missing some filter/fields that's OK. But please post a screenshot of that.

2. +++ b/config/install/views.view.tmgmt_job_overview.yml
   @@ -883,31 +883,39 @@ display:
   -            operator_id: ''
   -            label: ''
   +            operator_id: job_type_op
   +            label: 'Job Type'
   

   We have continuous exposed as job status now, lets use that instead of adding another filter. That should already just work.

3. +++ b/sources/content/src/Plugin/tmgmt/Source/ContentEntitySource.php
   @@ -119,7 +119,6 @@ class ContentEntitySource extends SourcePluginBase implements SourcePreviewInter
          foreach ($field as $index => $field_item) {
            $format = NULL;
   -        $translatable_properties = 0;
   

   This looks like a bad rebase, make sure you merge the latest changes.

4. +++ b/src/Form/ContinuousJobForm.php
   @@ -120,7 +123,29 @@ class ContinuousJobForm extends JobForm {
   +      $translator_type = $translator_manager->createInstance($type);
   +      if ($translator_type instanceof ContinuousTranslatorInterface) {
   +        return AccessResult::allowedIfHasPermissions($account, ['create translation jobs']);
   +      }
   

   did you check that it works when you add a new translator that has this ability, without clearing any other caches?

   We might need addCacheTag('config:tmgmt_translator_list') on both the allowed and forbidden access result.

5. +++ b/src/Tests/TMGMTUiTest.php
   @@ -741,17 +741,10 @@ class TMGMTUiTest extends EntityTestBase {
        $this->drupalGet('admin/tmgmt/jobs');
   -    $this->assertNoText($continuous_job->label(), 'Continuous job is not displayed on job overview page.');
   -
   -    // Test that continuous jobs are shown in the continuous job overview.
   -    $this->drupalGet('admin/tmgmt/continuous_jobs');
   -    $this->assertText($continuous_job->label(), 'Continuous job is displayed on continuous job overview page.');
   -
   -    // Test that normal jobs are not shown in the continuous job overview.
   -    $this->assertNoText($job1->label(), 'Normal job is not displayed on continuous job overview page.');
   +    $this->assertText($continuous_job->label(), 'Continuous job is displayed on job overview page.');
   

   Instead of just removing this, we should check the job status filter now here. By default, it's shown, showing e.g. active jobs only should not show it, selecting continuous jobs should show it.

6. +++ b/src/Tests/TMGMTUiTest.php
   @@ -791,11 +789,8 @@ class TMGMTUiTest extends EntityTestBase {
   -    // Test that continuous job overview page does not have Submit link.
   -    $this->assertNoLink('Submit', 'There is no Submit link on continuous job overview.');
   +    $this->drupalGet('admin/tmgmt/jobs');
   +    $this->assertText($continuous_job_label, 'Created continuous job is displayed on job overview page.');
   

   We lost this test, we still need to check that continuous jobs can't be submitted.

   I guess it fails because we do have jobs now that can be submitted. You can fix that by switching to the continuous filter (I think submitting the form doesn't work with views, just copy the URL with the active filter).

* Yes, continuous should only be shown when there are continuous translators. This isn't even part of this issue, but I forgot to point that out in #2682771: Ignore status changes for continuous jobs. Lets fix it here.

* Everything should show up if you have a translator that supports continuous, which according to the tests, seems to be working fine.

* Yes, as already discussed, continuous jobs currently only check in their own context for existing job items. That's for #2679949: Never allow duplicate submission of an item while in process to generalize.

* Again yes, the active job item check is likely not including the new job state. Since that is also not related to this issue, lets fix that separately, with tests.

Don't renumber update functions. Just remove the content of the one we no longer have, and comment that it was removed as we no longer create this view.

1. +++ b/src/Plugin/views/filter/JobState.php
   @@ -101,4 +105,20 @@ class JobState extends ManyToOne {
   +   */
   +  public function checkIfContinuousTranslatorAvailable() {
   +    $translator_manager = \Drupal::service('plugin.manager.tmgmt.translator');
   +    $translator_plugins = $translator_manager->getDefinitions();
   +    foreach ($translator_plugins as $type => $definition) {
   +      $translator_type = $translator_manager->createInstance($type);
   +      if ($translator_type instanceof ContinuousTranslatorInterface) {
   +        return TRUE;
   +      }
   +    }
   

   can we make this available somewhere, e.g. in continuous manager, so we don't have to duplicate that code in multiple places?

2. +++ b/tmgmt.install
   @@ -57,15 +57,9 @@ function tmgmt_update_8003() {
   -  $storage_controller->create($values)->save();
   -}
   +// Update function tmgmt_update_8004() has been removed since we are not
   +// creating a separate view for continuous jobs. It has been replaced with a
   +// single job overview for both continuous and normal translation jobs.
    
   

   What I meant is to keep the function and have the comment inside.

Ok. So there's a problem with translator dependencies. They are missing and that results that in the test, we do not actually remove and re-create them.

However, after that it still fails, despite me seeing he right cache tags.

This could be a core bug. The attached patch passes for me with the also attached core patch. *Somehow*, in my testing, #cache of the links don't bubble up. Could be because above we have the content key which doesn't contain anything except #cache and children, I don't know. Note that the core patch is actually incomplete and is missing to properly merge cache tags of multiple local actions, but it's enough to confirm this.

Considering to comment this test out for now and open a follow-up. I can live with having to clear cache manually for now in this scenario.

Could use someone else confirming that the core change indeed fixes it.

Nice!

This looks good enough to me to commit it. We have some follow-ups.

beta to beta update is a bit tricky because the job state filter still has the group configuration which no longer works so well. I guess we can ignore that for now and just tell users to re-import the default view.