JobItemForm don't check if hasTranslator and fail when Save

Added validation on job item form submit to check if the job has a translator.

Regarding the redirect to the job form, if wanted, do you have any suggestion how to do that in the job item form method?

+++ b/src/Form/JobItemForm.php
@@ -182,6 +182,9 @@ class JobItemForm extends TmgmtFormBase {
+    if (!$item->hasTranslator()) {
+      $form_state->setErrorByName('accept', $this->t('The job item could not be saved because the job does not have a translator assigned.'));
+    }
     if($item->getTranslator()){

I don't think we need to have a validation error. We just need to avoid calling the translator, which means we just need to switch the getTranslator() call below to hasTranslator().

And we will need tests, since they currently don't fail.

This core issue probably broke configration translation. We need the core issue fixed before making a patch for this issue.

I get this error.

Uploaded:

• patch including changes in JobItemForm and extended test coverage for this case
• interdiff of the two version of JobItemForm file

Looks pretty good to me. Some minor stuff

1. +++ b/sources/tmgmt_config/src/Tests/ConfigSourceUiTest.php
   @@ -117,6 +117,33 @@ class ConfigSourceUiTest extends EntityTestBase {
   +    // Request an italian translation.
   

   Maybe we can add a comment here about what we are testing.

2. +++ b/sources/tmgmt_config/src/Tests/ConfigSourceUiTest.php
   @@ -117,6 +117,33 @@ class ConfigSourceUiTest extends EntityTestBase {
   +    $this->assertText(t('One job needs to be checked out.'));
   +    $this->assertText('Article content type (English to Italian, Unprocessed)');
   ...
   +    $this->assertText(t('Job item Article content type'));
   

   We can get rid of these asserts because we already do that before.

Uploaded patch:

• added explanation comment lines
• dropped unnecessary assertion lines

Seems fine to me.

If there is no translator assigned, why do we allow editing and we are showing the save button?
Feels like a UX trap.

See also comment above from Berdir: "We just need to avoid calling the translator, which means we just need to switch the getTranslator() call below to hasTranslator()."

So the fact that we don't have a translator is definitively not an Error.
We should discuss to properly define the goal.

Just remove the else. No error, just don't call the translator. This is just a left-over of the switch to hasTranslation().

Editing before submitting is currently not very useful, but we want to change that (e.g. skip data items from being translated/sent to the translator and stuff like that)

Uploaded patch:

• dropped else block
• dropped lines in the test about the error message
• add hasTranslator() in save(), otherwise it gives an exception

I don't agree with the save() modification.
The problem is in addTranslatedData() where the Translator is called.
Check there if a translator is present.

• reset save() as the origin
• added a statement to check whether the job has a translator in JobItem::addTranslatedData()

Look good to me!