A missing field should not cause an exception when trying to save a translation

The workaround is commenting out the exception. Patches to do that it are welcome. Nobody is paid to maintain this module, most of our time spent maintaining it is our free time. Unless someone is funding the work or it affects our own clients, we don't guarantee any response times.

Thanks. The problem is that a drupal_set_message() will end up nowhere when for example automatically accepting jobs and they are coming through a notification callback or a cron job. Instead, it would be better to add this as a message the job which means we need to make that available in that method somehow.

+++ b/sources/content/src/Plugin/tmgmt/Source/ContentEntitySource.php
@@ -359,7 +358,19 @@ class ContentEntitySource extends SourcePluginBase implements SourcePreviewInter
+        \Drupal::logger('tmgmt_content')->warning($message, $args);
+        \Drupal::messenger()->addWarning($this->t($message, $args));
+

our UI's automatically display messages that were added while accepting, this should really not be necessary.

Having a test that does this would allow us to prove that fairly easy. Copy an existing one, delete a field before accepting.

Have a look at \Drupal\Tests\tmgmt_content\Functional\ContentTmgmtEntitySourceUiTest::testNodeTranslateTabSingleCheckout for example, the part until "$this->drupalPostForm(NULL, array(), t('Save as completed'));" creates a node with various fields, then creates a job, submits it and then accepts it.

Copy that into a new test method like testNodeTranslateMissingField(), then before you accept, delete e.g. the body field.

1. +++ b/sources/content/src/Plugin/tmgmt/Source/ContentEntitySource.php
   @@ -482,8 +482,6 @@ class ContentEntitySource extends SourcePluginBase implements SourcePreviewInter
       *   (optional) Whether to save the translation or not.
       *
   -   * @throws \Exception
   -   *   Thrown when a field or field offset is missing.
   

   the removed documentation also mentions an exception is thrown when a delta does not exist. that could in theory still happen if we attempt to set a non-sequential delta, so maybe we should be a little bit more careful about that or for now keep the @throws.

2. +++ b/sources/content/src/Plugin/tmgmt/Source/ContentEntitySource.php
   @@ -501,7 +499,17 @@ class ContentEntitySource extends SourcePluginBase implements SourcePreviewInter
    
          if (!$translation->hasField($field_name)) {
   -        throw new \Exception("Field '$field_name' does not exist on entity " . $translation->getEntityTypeId() . '/' . $translation->id());
   +        $message = 'Skipping field %field for job item <a href="@link">@item</a> because it does not exist on entity <em>@type/@id</em>.';
   +        $args = [
   +          '%field' => $field_name,
   +          '@link' => $item->toUrl()->toString(),
   

   I don't think it makes sense to link to the job item, this is already logged on the job item and has a link through that.

   Instead, as a user-facing message, we should use a linked label, with $translation->toLink()->toString(). But check with $translation->hasLinkTemplate('canonical') and don't link it if not, just use $translation->label() then.

I don't get the setting. It's not an exception anymore, just a warning, and I think it's worth to always do that, don't see the benefit of hiding the fact that some provided translations couldn't be saved.

The second part of my review in #22 wasn't applied and tests weren't added either as I asked in back in #15.

Amazing, a test :)

Thanks, merged.