From c78441d6a314d88e0e0590c1e89af34165f33e9c Mon Sep 17 00:00:00 2001 From: Aravindh R Date: Mon, 30 Aug 2021 14:46:53 +0530 Subject: [PATCH] Update messages --- .../domain/membership/MembershipService.scala | 11 +-- .../domain/record/RecordSetValidations.scala | 3 +- .../vinyldns/api/route/DnsJsonProtocol.scala | 3 +- .../record/RecordSetValidationsSpec.scala | 3 +- .../api/route/VinylDNSJsonProtocolSpec.scala | 3 +- .../main/scala/vinyldns/core/Messages.scala | 73 +++++++++++++------ .../core/domain/DomainValidationErrors.scala | 10 ++- .../dns-change/dns-change-new.controller.js | 2 +- 8 files changed, 73 insertions(+), 35 deletions(-) diff --git a/modules/api/src/main/scala/vinyldns/api/domain/membership/MembershipService.scala b/modules/api/src/main/scala/vinyldns/api/domain/membership/MembershipService.scala index 0d47bace4..5eff173b3 100644 --- a/modules/api/src/main/scala/vinyldns/api/domain/membership/MembershipService.scala +++ b/modules/api/src/main/scala/vinyldns/api/domain/membership/MembershipService.scala @@ -24,6 +24,7 @@ import vinyldns.core.domain.membership.LockStatus.LockStatus import vinyldns.core.domain.zone.ZoneRepository import vinyldns.core.domain.membership._ import vinyldns.core.domain.record.RecordSetRepository +import vinyldns.core.Messages._ object MembershipService { def apply(dataAccessor: ApiDataAccessor): MembershipService = @@ -235,7 +236,7 @@ class MembershipService( .getGroupByName(name) .map { case Some(existingGroup) if existingGroup.status != GroupStatus.Deleted => - GroupAlreadyExistsError(s"Group with name $name already exists").asLeft + GroupAlreadyExistsError(GroupAlreadyExistsErrorMsg.format(name, existingGroup.email)).asLeft case _ => ().asRight } @@ -257,7 +258,7 @@ class MembershipService( .map { case Some(existingGroup) if existingGroup.status != GroupStatus.Deleted && existingGroup.id != groupId => - GroupAlreadyExistsError(s"Group with name $name already exists").asLeft + GroupAlreadyExistsError(GroupAlreadyExistsErrorMsg.format(name, existingGroup.email)).asLeft case _ => ().asRight } @@ -267,7 +268,7 @@ class MembershipService( zoneRepo .getZonesByAdminGroupId(group.id) .map { zones => - ensuring(InvalidGroupRequestError(s"${group.name} is the admin of a zone. Cannot delete."))( + ensuring(InvalidGroupRequestError(ZoneAdminError.format(group.name)))( zones.isEmpty ) } @@ -279,7 +280,7 @@ class MembershipService( .map { rsId => ensuring( InvalidGroupRequestError( - s"${group.name} is the owner for a record set including $rsId. Cannot delete." + RecordSetOwnerError.format(group.name, rsId) ) )(rsId.isEmpty) } @@ -291,7 +292,7 @@ class MembershipService( .map { zId => ensuring( InvalidGroupRequestError( - s"${group.name} has an ACL rule for a zone including $zId. Cannot delete." + ACLRuleError.format(group.name, zId) ) )(zId.isEmpty) } diff --git a/modules/api/src/main/scala/vinyldns/api/domain/record/RecordSetValidations.scala b/modules/api/src/main/scala/vinyldns/api/domain/record/RecordSetValidations.scala index add8ba515..168f27f1d 100644 --- a/modules/api/src/main/scala/vinyldns/api/domain/record/RecordSetValidations.scala +++ b/modules/api/src/main/scala/vinyldns/api/domain/record/RecordSetValidations.scala @@ -28,6 +28,7 @@ import vinyldns.core.domain.auth.AuthPrincipal import vinyldns.core.domain.membership.Group import vinyldns.core.domain.record.{RecordSet, RecordType} import vinyldns.core.domain.zone.Zone +import vinyldns.core.Messages._ import scala.util.matching.Regex @@ -316,7 +317,7 @@ object RecordSetValidations { def validRecordNameFilterLength(recordNameFilter: String): Either[Throwable, Unit] = ensuring( - InvalidRequest("recordNameFilter must contain at least two letters or numbers.") + InvalidRequest(RecordNameFilterError) ) { val searchRegex: Regex = """[a-zA-Z0-9].*[a-zA-Z0-9]+""".r searchRegex.findFirstIn(recordNameFilter).isDefined diff --git a/modules/api/src/main/scala/vinyldns/api/route/DnsJsonProtocol.scala b/modules/api/src/main/scala/vinyldns/api/route/DnsJsonProtocol.scala index 9b34f7180..8edbe5dcf 100644 --- a/modules/api/src/main/scala/vinyldns/api/route/DnsJsonProtocol.scala +++ b/modules/api/src/main/scala/vinyldns/api/route/DnsJsonProtocol.scala @@ -30,6 +30,7 @@ import vinyldns.core.domain.DomainHelpers.removeWhitespace import vinyldns.core.domain.Fqdn import vinyldns.core.domain.record._ import vinyldns.core.domain.zone._ +import vinyldns.core.Messages._ trait DnsJsonProtocol extends JsonValidation { import vinyldns.core.domain.record.RecordType._ @@ -373,7 +374,7 @@ trait DnsJsonProtocol extends JsonValidation { .required[String]("Missing NS.nsdname") .check( "NS must be less than 255 characters" -> checkDomainNameLen, - "NS data must be absolute" -> nameContainsDots + NSDataError -> nameContainsDots ) .map(Fqdn.apply) .map(NSData.apply) diff --git a/modules/api/src/test/scala/vinyldns/api/domain/record/RecordSetValidationsSpec.scala b/modules/api/src/test/scala/vinyldns/api/domain/record/RecordSetValidationsSpec.scala index e77f900a3..ac9a0df36 100644 --- a/modules/api/src/test/scala/vinyldns/api/domain/record/RecordSetValidationsSpec.scala +++ b/modules/api/src/test/scala/vinyldns/api/domain/record/RecordSetValidationsSpec.scala @@ -36,6 +36,7 @@ import vinyldns.core.TestMembershipData._ import vinyldns.core.domain.Fqdn import vinyldns.core.domain.membership.Group import vinyldns.core.domain.record._ +import vinyldns.core.Messages._ import scala.util.matching.Regex @@ -601,7 +602,7 @@ class RecordSetValidationsSpec val invalidString = "*o*" val error = leftValue(validRecordNameFilterLength(invalidString)) error shouldBe an[InvalidRequest] - error.getMessage() shouldBe "recordNameFilter must contain at least two letters or numbers." + error.getMessage() shouldBe RecordNameFilterError } } } diff --git a/modules/api/src/test/scala/vinyldns/api/route/VinylDNSJsonProtocolSpec.scala b/modules/api/src/test/scala/vinyldns/api/route/VinylDNSJsonProtocolSpec.scala index 64b8b3f8a..d566637c2 100644 --- a/modules/api/src/test/scala/vinyldns/api/route/VinylDNSJsonProtocolSpec.scala +++ b/modules/api/src/test/scala/vinyldns/api/route/VinylDNSJsonProtocolSpec.scala @@ -27,6 +27,7 @@ import vinyldns.core.domain.record._ import vinyldns.core.domain.zone.{CreateZoneInput, UpdateZoneInput, ZoneConnection} import vinyldns.core.TestRecordSetData._ import vinyldns.core.domain.Fqdn +import vinyldns.core.Messages._ class VinylDNSJsonProtocolSpec extends AnyWordSpec @@ -594,7 +595,7 @@ class VinylDNSJsonProtocolSpec ("records" -> data) val thrown = the[MappingException] thrownBy recordSetJValue.extract[RecordSet] - thrown.msg should include("NS data must be absolute") + thrown.msg should include(NSDataError) } "round trip a DS record set" in { val rs = RecordSet( diff --git a/modules/core/src/main/scala/vinyldns/core/Messages.scala b/modules/core/src/main/scala/vinyldns/core/Messages.scala index 233cbefa3..74ce1e61a 100644 --- a/modules/core/src/main/scala/vinyldns/core/Messages.scala +++ b/modules/core/src/main/scala/vinyldns/core/Messages.scala @@ -18,36 +18,65 @@ package vinyldns.core object Messages { - // When less than two letters or numbers is filled in Record Name Filter field in RecordSetSearch page - val RecordNameFilterError = "Record Name Filter field must contain at least two letters or numbers to perform a RecordSet Search." + // Error displayed when less than two letters or numbers is filled in Record Name Filter field in RecordSetSearch page + val RecordNameFilterError = + "Record Name Filter field must contain at least two letters or numbers to perform a RecordSet Search." - // When creating group with name that already exists - // s"Group with name $name already exists. Please try a different name or contact ${existingGroup.email} to be added to the group." - val GroupAlreadyExistsError = s"Group with name {TestGroup} already exists. Please try a different name or contact {test@test.com} to be added to the group." + /* + * Error displayed when attempting to create group with name that already exists + * + * Placeholders: + * 1. [string] group name + * 2. [string] group email address + */ + val GroupAlreadyExistsErrorMsg = + "Group with name %s already exists. Please try a different name or contact %s to be added to the group." - // When deleting a group being the admin of a zone - // s"${group.name} is the admin of a zone. Cannot delete. Please transfer the ownership to another group before deleting." - val ZoneAdminError = s"{TestGroup} is the admin of a zone. Cannot delete. Please transfer the ownership to another group before deleting." + /* + * Error displayed when deleting a group being the admin of a zone + * + * Placeholders: + * 1. [string] group name + */ + val ZoneAdminError = + "%s is the admin of a zone. Cannot delete. Please transfer the ownership to another group before deleting." - // When deleting a group being the owner for a record set - // s"${group.name} is the owner for a record set including $rsId. Cannot delete. Please transfer the ownership to another group before deleting. - val RecordSetOwnerError = s"{TestGroup} is the owner for a record set including {RS_ID}. Cannot delete. Please transfer the ownership to another group before deleting." + /* + * Error displayed when deleting a group being the owner for a record set + * + * Placeholders: + * 1. [string] group name + * 2. [string] record set id + */ + val RecordSetOwnerError = + "%s is the owner for a record set including %s. Cannot delete. Please transfer the ownership to another group before deleting." - // When deleting a group which has an ACL rule for a zone - // s"${group.name} has an ACL rule for a zone including $zId. Cannot delete. Please transfer the ownership to another group before deleting." - val ACLRuleError = s"{TestGroup} has an ACL rule for a zone including {Z_ID}. Cannot delete. Please transfer the ownership to another group before deleting." + /* + * Error displayed when deleting a group which has an ACL rule for a zone + * + * Placeholders: + * 1. [string] group name + * 2. [string] zone id + */ + val ACLRuleError = + "%s has an ACL rule for a zone including %s. Cannot delete. Please transfer the ownership to another group before deleting." - // When NSData field is not a positive integer + // Error displayed when NSData field is not a positive integer val NSDataError = "NS data must be a positive integer" - // When importing files other than .csv + // Error displayed when importing files other than .csv val ImportError = "Import failed. Not a valid file. File should be of ‘.csv’ type." - // When user is not authorized to make changes to the record - // s"""User "$userName" is not authorized. Contact ${ownerType.toString.toLowerCase} owner group: - // |${ownerGroupName.getOrElse(ownerGroupId)} at ${contactEmail.getOrElse("")} to make DNS changes. - // |You must be a part of the owner group to make DNS changes.""".stripMargin .replaceAll("\n", " ") - val NotAuthorizedError = s"""User {"dummy"} is not authorized. Contact {zone} owner group: {ok-group} at - {test@test.com} to make DNS changes. You must be a part of the owner group to make DNS changes.""" + /* + * Error displayed when user is not authorized to make changes to the record + * + * Placeholders: + * 1. [string] user name + * 2. [string] owner type + * 3. [string] owner group name | owner group id + * 4. [string] contact email + */ + val NotAuthorizedErrorMsg = + "User '%s' is not authorized. Contact %s owner group: %s at %s to make DNS changes." } diff --git a/modules/core/src/main/scala/vinyldns/core/domain/DomainValidationErrors.scala b/modules/core/src/main/scala/vinyldns/core/domain/DomainValidationErrors.scala index 591d5c7cd..91486ce79 100644 --- a/modules/core/src/main/scala/vinyldns/core/domain/DomainValidationErrors.scala +++ b/modules/core/src/main/scala/vinyldns/core/domain/DomainValidationErrors.scala @@ -19,6 +19,7 @@ package vinyldns.core.domain import vinyldns.core.domain.batch.OwnerType.OwnerType import vinyldns.core.domain.record.{RecordData, RecordSet, RecordType} import vinyldns.core.domain.record.RecordType.RecordType +import vinyldns.core.Messages._ // $COVERAGE-OFF$ sealed abstract class DomainValidationError(val isFatal: Boolean = true) { @@ -134,9 +135,12 @@ final case class UserIsNotAuthorizedError( ownerGroupName: Option[String] = None ) extends DomainValidationError { def message: String = - s"""User "$userName" is not authorized. Contact ${ownerType.toString.toLowerCase} owner group: - |${ownerGroupName.getOrElse(ownerGroupId)} at ${contactEmail.getOrElse("")}.""".stripMargin - .replaceAll("\n", " ") + NotAuthorizedErrorMsg.format( + userName, + ownerType.toString.toLowerCase, + ownerGroupName.getOrElse(ownerGroupId), + contactEmail.getOrElse("") + ) } final case class RecordNameNotUniqueInBatch(name: String, typ: RecordType) diff --git a/modules/portal/public/lib/dns-change/dns-change-new.controller.js b/modules/portal/public/lib/dns-change/dns-change-new.controller.js index 70848ee76..a0234c0ba 100644 --- a/modules/portal/public/lib/dns-change/dns-change-new.controller.js +++ b/modules/portal/public/lib/dns-change/dns-change-new.controller.js @@ -177,7 +177,7 @@ $scope.$apply() resolve($scope.newBatch.changes.length); } else { - reject("Import failed. Not a valid file."); + reject("Import failed. Not a valid file. File should be of ‘.csv’ type."); } } reader.readAsText(file);