Issue #3239940: UniqueBirthLogConstraint allows creating two birth logs for the same child

modules/log/birth/src/Plugin/Validation/Constraint/UniqueBirthLogConstraintValidator.php

@@ -43,31 +43,33 @@ class UniqueBirthLogConstraintValidator extends ConstraintValidator implements C
    * {@inheritdoc}
    */
   public function validate($value, Constraint $constraint) {
-    /** @var \Drupal\Core\Field\FieldItemListInterface[] $value */
+    /** @var \Drupal\Core\Field\EntityReferenceFieldItemList $value */
     /** @var \Drupal\farm_birth\Plugin\Validation\Constraint\UniqueBirthLogConstraint $constraint */
-    foreach ($value as $item) {
+    foreach ($value->referencedEntities() as $delta => $asset) {
 
-      // Get the referenced asset ID.
-      $item_value = $item->getValue();
-      $asset_id = $item_value['target_id'] ?? FALSE;
-
-      // If there is no asset, skip.
-      if (empty($asset_id)) {
-        continue;
+      // If the log is not new, skip validation.
+      // A birth log exits so there is no need to check if one can be created.
+      /** @var \Drupal\log\Entity\LogInterface $log */
+      $log = $value->getParent()->getValue();
+      if (!$log->isNew()) {
+        return;
       }
 
-      // Perform an entity query to find logs that reference the asset.
+      // Query the number of birth logs that reference the asset.
       // We do not check access to ensure that all matching logs are found.
-      $query = $this->entityTypeManager->getStorage('log')->getQuery()
+      $count = $this->entityTypeManager->getStorage('log')->getAggregateQuery()
         ->accessCheck(FALSE)
         ->condition('type', 'birth')
-        ->condition('asset', $asset_id);
-      $ids = $query->execute();
+        ->condition('asset', $asset->id())
+        ->count()
+        ->execute();
 
-      // If more than 1 birth logs reference the asset, add a violation.
-      if (count($ids) > 1) {
-        $asset = $this->entityTypeManager->getStorage('asset')->load($asset_id);
-        $this->context->addViolation($constraint->message, ['%child' => $asset->label()]);
+      // If more than 0 birth logs reference the asset, add a violation.
+      if ($count > 0) {
+        $this->context->buildViolation($constraint->message, ['%child' => $asset->label()])
+          ->atPath((string) $delta . '.target_id')
+          ->setInvalidValue($asset->id())
+          ->addViolation();
       }
     }
   }

modules/log/birth/tests/src/Kernel/BirthTest.php

@@ -27,6 +27,7 @@ class BirthTest extends KernelTestBase {
     'farm_birth',
     'farm_entity',
     'farm_entity_fields',
+    'farm_entity_views',
     'farm_field',
     'farm_id_tag',
     'farm_log',
@@ -35,6 +36,7 @@ class BirthTest extends KernelTestBase {
     'image',
     'options',
     'state_machine',
+    'system',
     'user',
     'taxonomy',
     'text',
@@ -51,6 +53,7 @@ class BirthTest extends KernelTestBase {
     $this->installEntitySchema('taxonomy_term');
     $this->installEntitySchema('user');
     $this->installConfig([
+      'farm_entity_views',
       'farm_animal',
       'farm_animal_type',
       'farm_birth',
@@ -162,25 +165,33 @@ class BirthTest extends KernelTestBase {
       'timestamp' => \Drupal::time()->getRequestTime(),
       'asset' => [['target_id' => $asset->id()]],
     ]);
-    $log1->save();
 
-    // Confirm that there were no validation errors.
+    // Confirm that there are no validation errors.
     $errors = $log1->validate();
     $this->assertCount(0, $errors);
 
+    $log1->save();
+
     // Create a second birth log that references the asset.
     $log2 = Log::create([
       'type' => 'birth',
       'timestamp' => \Drupal::time()->getRequestTime(),
       'asset' => [['target_id' => $asset->id()]],
     ]);
-    $log2->save();
 
     // Confirm that validation fails.
     $errors = $log2->validate();
     $this->assertCount(1, $errors);
     $this->assertEquals(new FormattableMarkup('%child already has a birth log. More than one birth log cannot reference the same child.', ['%child' => $asset->label()]), $errors[0]->getMessage());
-    $this->assertEquals('asset', $errors[0]->getPropertyPath());
+    $this->assertEquals('asset.0.target_id', $errors[0]->getPropertyPath());
+
+    // Try updating the original birth log.
+    $log1->set('name', $this->randomMachineName());
+
+    // Confirm there are no validation errors.
+    $errors = $log1->validate();
+    $this->assertCount(0, $errors);
+    $log1->save();
   }
 
 }