fix: comments from review

Author: Victor Wiebe

src/procedures/AssignStoRole.ts

@@ -8,7 +8,7 @@ import {
   StoRole,
 } from '../types';
 import { PolymathError } from '../PolymathError';
-import { isValidBytes32CompliantString } from '~/utils';
+import { checkStringLength } from '../utils';
 
 export class AssignStoRole extends Procedure<AssignStoRoleProcedureArgs> {
   public type = ProcedureType.AssignStoRole;
@@ -67,7 +67,7 @@ export class AssignStoRole extends Procedure<AssignStoRoleProcedureArgs> {
         });
       }
     } else {
-      isValidBytes32CompliantString(description, 'description');
+      checkStringLength(description, 'description');
       // Delegate not found. Add them here
       await this.addTransaction(permissionModule.addDelegate, {
         tag: PolyTransactionTag.ChangePermission,

src/procedures/ModifyTieredStoData.ts

@@ -15,7 +15,7 @@ import {
   Currency,
 } from '../types';
 import { PolymathError } from '../PolymathError';
-import { areSameAddress, isValidBytes32CompliantString } from '../utils';
+import { areSameAddress, checkStringLength } from '../utils';
 import { SecurityToken, TieredSto } from '../entities';
 import { TieredStoFactory } from '../entities/factories';
 
@@ -219,7 +219,7 @@ export class ModifyTieredStoData extends Procedure<ModifyTieredStoDataProcedureA
         polyOracleAddress = currentPolyOracleAddress;
       }
 
-      isValidBytes32CompliantString(currencySymbol, 'denominated currency symbol');
+      checkStringLength(currencySymbol, 'denominated currency symbol');
 
       if (
         currencySymbol !== denominatedCurrency ||

src/procedures/SetDocument.ts

@@ -1,7 +1,7 @@
 import { Procedure } from './Procedure';
 import { ProcedureType, PolyTransactionTag, SetDocumentProcedureArgs, ErrorCode } from '../types';
 import { PolymathError } from '../PolymathError';
-import { isNotEmptyValidBytes32CompliantString } from '~/utils';
+import { checkStringLength } from '../utils';
 
 export class SetDocument extends Procedure<SetDocumentProcedureArgs> {
   public type = ProcedureType.SetDocument;
@@ -32,8 +32,8 @@ export class SetDocument extends Procedure<SetDocumentProcedureArgs> {
       });
     }
 
-    isNotEmptyValidBytes32CompliantString(name, 'name');
-    isNotEmptyValidBytes32CompliantString(documentHash, 'document hash');
+    checkStringLength(name, 'name', { minLength: 1, maxLength: 32 });
+    checkStringLength(documentHash, 'document hash', { minLength: 1, maxLength: 32 });
 
     /*
      * Transactions

src/utils/__tests__/utils.ts

@@ -1,4 +1,6 @@
-import { serialize, unserialize } from '../index';
+import { checkStringLength, serialize, unserialize } from '../index';
+import { PolymathError } from '../../PolymathError';
+import { ErrorCode } from '../../types';
 
 describe('serialize and unserialize', () => {
   const entityType = 'someEntity';
@@ -31,3 +33,46 @@ describe('serialize and unserialize', () => {
     expect(unserialize(serialize(entityType, inversePojo1))).toEqual(pojo1);
   });
 });
+
+describe('check string length', () => {
+  const testVariableType = 'myVar';
+
+  test('checkStringLength used on a non empty string, catches error above the default 32 characters', () => {
+    try {
+      checkStringLength('0123456789012345678901234567890123456789', testVariableType);
+    } catch (error) {
+      expect(error).toEqual(
+        new PolymathError({
+          code: ErrorCode.ProcedureValidationError,
+          message: `You must provide a valid ${testVariableType} up to 32 characters long`,
+        })
+      );
+    }
+  });
+
+  test('checkStringLength used on an empty string, catches error due to not meeting custom requirement of 1 - 32 characters', () => {
+    try {
+      checkStringLength('', testVariableType, { minLength: 1, maxLength: 32 });
+    } catch (error) {
+      expect(error).toEqual(
+        new PolymathError({
+          code: ErrorCode.ProcedureValidationError,
+          message: `You must provide a valid ${testVariableType} between 1 and 32 characters long`,
+        })
+      );
+    }
+  });
+
+  test('checkStringLength used on a 6 character string, catches error due to not meeting custom requirement of 1 - 5 characters', () => {
+    try {
+      checkStringLength('0123456', testVariableType, { minLength: 1, maxLength: 5 });
+    } catch (error) {
+      expect(error).toEqual(
+        new PolymathError({
+          code: ErrorCode.ProcedureValidationError,
+          message: `You must provide a valid ${testVariableType} between 1 and 5 characters long`,
+        })
+      );
+    }
+  });
+});

src/utils/index.ts

@@ -50,20 +50,27 @@ export function areSameAddress(a: string, b: string) {
   return a.toUpperCase() === b.toUpperCase();
 }
 
-export function isValidBytes32CompliantString(value: string, variableName: string) {
-  if (value.length > 32) {
+/**
+ * Check the length of a given string to ensure it meets correct bounds
+ * @param value - the string itself
+ * @param variableName - the name of the variable associated to the string itself
+ * @param opts - optional min and max length of the string. Defaults to a minimum of 0 (empty string) and a maximum of 32 characters
+ */
+export function checkStringLength(
+  value: string,
+  variableName: string,
+  opts?: { minLength: number | undefined; maxLength: number }
+) {
+  const minLength = opts && opts.minLength != undefined ? opts.minLength : 0;
+  const maxLength = opts ? opts.maxLength : 32;
+  if (value.length < minLength || value.length > maxLength) {
     throw new PolymathError({
       code: ErrorCode.ProcedureValidationError,
-      message: `You must provide a valid ${variableName} up to 32 characters long`,
-    });
-  }
-}
-
-export function isNotEmptyValidBytes32CompliantString(value: string, variableName: string) {
-  if (value.length < 1 || value.length > 32) {
-    throw new PolymathError({
-      code: ErrorCode.ProcedureValidationError,
-      message: `You must provide a valid ${variableName} between 1 and 32 characters long`,
+      message: `You must provide a valid ${variableName} ${
+        opts && opts.minLength != undefined
+          ? `between ${minLength} and ${maxLength}`
+          : `up to ${maxLength}`
+      } characters long`,
     });
   }
 }