Merge pull request #1423 from nspadaccino/ownership_transfer_bugfix

Ownership transfer Bug Fix

modules/api/src/it/scala/vinyldns/api/domain/record/RecordSetServiceIntegrationSpec.scala

@@ -128,7 +128,10 @@ class RecordSetServiceIntegrationSpec
     RecordSetStatus.Active,
     Instant.now.truncatedTo(ChronoUnit.MILLIS),
     None,
-    List(AAAAData("fd69:27cc:fe91::60"))
+    List(AAAAData("fd69:27cc:fe91::60")),
+    recordSetGroupChange =
+      Some(OwnerShipTransfer(ownerShipTransferStatus = OwnerShipTransferStatus.None,
+        requestedOwnerGroupId = None))
   )
   private val subTestRecordA = RecordSet(
     zone.id,

modules/api/src/main/scala/vinyldns/api/domain/record/RecordSetService.scala

@@ -27,6 +27,7 @@ import vinyldns.core.domain.zone.{Zone, ZoneCommandResult, ZoneRepository}
 import vinyldns.core.queue.MessageQueue
 import cats.data._
 import cats.effect.IO
+import org.slf4j.{Logger, LoggerFactory}
 import org.xbill.DNS.ReverseMap
 import vinyldns.api.config.{ZoneAuthConfigs, DottedHostsConfig, HighValueDomainConfig}
 import vinyldns.api.domain.DomainValidations.{validateIpv4Address, validateIpv6Address}
@@ -94,6 +95,8 @@ class RecordSetService(
   import RecordSetValidations._
   import accessValidation._
 
+  val logger: Logger = LoggerFactory.getLogger(classOf[RecordSetService])
+
   val approverOwnerShipTransferStatus = List(OwnerShipTransferStatus.ManuallyApproved , OwnerShipTransferStatus.AutoApproved, OwnerShipTransferStatus.ManuallyRejected)
   val requestorOwnerShipTransferStatus = List(OwnerShipTransferStatus.Cancelled , OwnerShipTransferStatus.Requested, OwnerShipTransferStatus.PendingReview)
 
@@ -155,8 +158,11 @@ class RecordSetService(
         && !auth.isSuper && !auth.isGroupMember(existing.ownerGroupId.getOrElse("None")))
         unchangedRecordSet(existing, recordSet).toResult else ().toResult
       _ <- if(existing.recordSetGroupChange.map(_.ownerShipTransferStatus).getOrElse("<none>") == OwnerShipTransferStatus.Cancelled
-        && !auth.isSuper)
+        && !auth.isSuper) {
-        recordSetOwnerShipApproveStatus(recordSet).toResult else ().toResult
+        recordSetOwnerShipApproveStatus(recordSet).toResult
+      } else ().toResult
+      _ = logger.info(s"updated recordsetgroupchange: ${recordSet.recordSetGroupChange}")
+      _ = logger.info(s"existing recordsetgroupchange: ${existing.recordSetGroupChange}")
       recordSet <- updateRecordSetGroupChangeStatus(recordSet, existing, zone)
       change <- RecordSetChangeGenerator.forUpdate(existing, recordSet, zone, Some(auth)).toResult
       // because changes happen to the RS in forUpdate itself, converting 1st and validating on that

modules/api/src/main/scala/vinyldns/api/domain/record/RecordSetValidations.scala

@@ -495,7 +495,7 @@ object RecordSetValidations {
                                          existing: RecordSet
                                        ): Either[Throwable, Unit] =
     Either.cond(
-      updates.recordSetGroupChange == existing.recordSetGroupChange,
+      updates.recordSetGroupChange == existing.recordSetGroupChange || existing.recordSetGroupChange.isEmpty,
       (),
       InvalidRequest("Cannot update RecordSet OwnerShip Status when zone is not shared.")
     )

modules/api/src/test/functional/tests/recordsets/update_recordset_test.py

@@ -303,7 +303,7 @@ def test_update_recordset_replace_2_records_with_1_different_record(shared_zone_
             ]
         }
         result = client.create_recordset(new_rs, status=202)
-        
+
         assert_that(result["changeType"], is_("Create"))
         assert_that(result["status"], is_("Pending"))
         assert_that(result["created"], is_not(none()))
@@ -373,7 +373,7 @@ def test_update_existing_record_set_add_record(shared_zone_test_context):
             ]
         }
         result = client.create_recordset(new_rs, status=202)
-        
+
         assert_that(result["changeType"], is_("Create"))
         assert_that(result["status"], is_("Pending"))
         assert_that(result["created"], is_not(none()))
@@ -2432,7 +2432,18 @@ def test_update_owner_group_transfer_on_non_shared_zones_in_fails(shared_zone_te
     update_rs = None
 
     try:
-        record_json = create_recordset(ok_zone, "test_update_success", "A", [{"address": "1.1.1.1"}])
+        # record_json = create_recordset(ok_zone, "test_update_success", "A", [{"address": "1.1.1.1"}], recordSetGroupChange={"ownerShipTransferStatus": None, "requestedOwnerGroupId": None})
+        record_json = {
+            "zoneId": ok_zone["id"],
+            "name": "test_update_success",
+            "type": "A",
+            "ttl": 38400,
+            "records": [
+                {"address": "1.1.1.1"}
+            ],
+            "recordSetGroupChange": {"ownerShipTransferStatus": "None",
+                                     "requestedOwnerGroupId": None}
+        }
 
         create_response = ok_client.create_recordset(record_json, status=202)
         update = ok_client.wait_until_recordset_change_status(create_response, "Complete")["recordSet"]