Added validation framework basics.

finnow-api/source/transaction/api.d

@@ -224,8 +224,7 @@ immutable DEFAULT_DRAFT_PAGE = PageRequest(1, 10, [Sort("draft.id", SortDir.DESC
 void handleGetDrafts(ref ServerHttpRequest request, ref ServerHttpResponse response) {
     ProfileDataSource ds = getProfileDataSource(request);
     PageRequest pr = PageRequest.parse(request, DEFAULT_DRAFT_PAGE);
-    bool shouldFetchTemplates = request.getParamAs!bool("template", false);
-    Page!TransactionDraftListItem page = getDrafts(ds, pr, shouldFetchTemplates);
+    Page!TransactionDraftListItem page = getDrafts(ds, pr);
     writeJsonBody(response, page);
 }
 

finnow-api/source/transaction/data.d

@@ -57,6 +57,7 @@ interface TransactionRepository {
 interface TransactionDraftRepository {
     Page!TransactionDraftListItem findAllDrafts(in PageRequest pr);
     Page!TransactionDraftListItem findAllTemplates(in PageRequest pr);
+    Page!TransactionDraftListItem findAll(in PageRequest pr);
     Optional!TransactionDraftResponse findById(ulong id);
     TransactionDraftResponse insert(in TransactionDraftPayload data);
     void linkAttachment(ulong draftId, ulong attachmentId);

finnow-api/source/transaction/data_impl_sqlite.d

@@ -631,11 +631,15 @@ class SqliteTransactionDraftRepository : TransactionDraftRepository {
     }
     
     Page!TransactionDraftListItem findAllDrafts(in PageRequest pr) {
-        return findAllInternal(pr, DraftType.DRAFT);
+        return findAllInternal(pr, DraftType.DRAFT.toOptional);
     }
 
     Page!TransactionDraftListItem findAllTemplates(in PageRequest pr) {
-        return findAllInternal(pr, DraftType.TEMPLATE);
+        return findAllInternal(pr, DraftType.TEMPLATE.toOptional);
+    }
+
+    Page!TransactionDraftListItem findAll(in PageRequest pr) {
+        return findAllInternal(pr, Optional!DraftType.empty());
     }
 
     private static enum DraftType {
@@ -643,14 +647,16 @@ class SqliteTransactionDraftRepository : TransactionDraftRepository {
         TEMPLATE
     }
 
-    private Page!TransactionDraftListItem findAllInternal(in PageRequest pr, DraftType type) {
+    private Page!TransactionDraftListItem findAllInternal(in PageRequest pr, Optional!DraftType type) {
         QueryBuilder qb = getBuilderForDraftsList();
         addSelectsForDraftsList(qb);
         qb.groupBy("draft.id");
-        if (type == DraftType.DRAFT) {
-            qb.where("template_name IS NULL");
-        } else {
-            qb.where("template_name IS NOT NULL");
+        if (type) {
+            if (type.value == DraftType.DRAFT) {
+                qb.where("template_name IS NULL");
+            } else {
+                qb.where("template_name IS NOT NULL");
+            }
         }
         string query = qb.build() ~ "\n" ~ pr.toSql();
         TransactionDraftListItem[] results = util.sqlite.findAll(db, query, &parseDraftListItem);

finnow-api/source/transaction/service.d

@@ -16,6 +16,8 @@ import account.data;
 import util.money;
 import util.pagination;
 import util.data;
+import util.validation.transaction;
+import util.validation.draft;
 import attachment.data;
 import attachment.dto;
 
@@ -182,76 +184,6 @@ void deleteTransaction(ProfileDataSource ds, ulong transactionId) {
     });
 }
 
-private void validateTransactionPayload(
-    TransactionVendorRepository vendorRepo,
-    TransactionCategoryRepository categoryRepo,
-    AccountRepository accountRepo,
-    in AddTransactionPayload payload
-) {
-    if (!payload.creditedAccountId && !payload.debitedAccountId) {
-        throw new HttpStatusException(HttpStatus.BAD_REQUEST, "At least one account must be linked.");
-    }
-    if (
-        payload.creditedAccountId &&
-        payload.debitedAccountId &&
-        payload.creditedAccountId.value == payload.debitedAccountId.value
-    ) {
-        throw new HttpStatusException(HttpStatus.BAD_REQUEST, "Cannot link the same account as both credit and debit.");
-    }
-    if (payload.amount == 0) {
-        throw new HttpStatusException(HttpStatus.BAD_REQUEST, "Amount should be greater than 0.");
-    }
-    SysTime now = Clock.currTime(UTC());
-    SysTime timestamp;
-    try {
-        timestamp = SysTime.fromISOExtString(payload.timestamp, UTC());
-    } catch (TimeException e) {
-        throw new HttpStatusException(HttpStatus.BAD_REQUEST, "Invalid timestamp format. Expected ISO-8601 datetime.");
-    }
-    if (timestamp > now) {
-        throw new HttpStatusException(HttpStatus.BAD_REQUEST, "Cannot create transaction in the future.");
-    }
-    if (payload.vendorId && !vendorRepo.existsById(payload.vendorId.value)) {
-        throw new HttpStatusException(HttpStatus.BAD_REQUEST, "Vendor doesn't exist.");
-    }
-    if (payload.categoryId && !categoryRepo.existsById(payload.categoryId.value)) {
-        throw new HttpStatusException(HttpStatus.BAD_REQUEST, "Category doesn't exist.");
-    }
-    if (payload.creditedAccountId && !accountRepo.existsById(payload.creditedAccountId.value)) {
-        throw new HttpStatusException(HttpStatus.BAD_REQUEST, "Credited account doesn't exist.");
-    }
-    if (payload.debitedAccountId && !accountRepo.existsById(payload.debitedAccountId.value)) {
-        throw new HttpStatusException(HttpStatus.BAD_REQUEST, "Debited account doesn't exist.");
-    }
-    foreach (tag; payload.tags) {
-        import std.regex;
-        auto r = ctRegex!(`^[a-z0-9-_]{3,32}$`);
-        if (!matchFirst(tag, r)) {
-            throw new HttpStatusException(HttpStatus.BAD_REQUEST, "Invalid tag: \"" ~ tag ~ "\".");
-        }
-    }
-    if (payload.lineItems.length > 0) {
-        long lineItemsTotal = 0;
-        foreach (lineItem; payload.lineItems) {
-            if (lineItem.categoryId && !categoryRepo.existsById(lineItem.categoryId.value)) {
-                throw new HttpStatusException(HttpStatus.BAD_REQUEST, "Line item's category doesn't exist.");
-            }
-            if (lineItem.quantity == 0) {
-                throw new HttpStatusException(HttpStatus.BAD_REQUEST, "Line item's quantity should greater than zero.");
-            }
-            for (ulong i = 0; i < lineItem.quantity; i++) {
-                lineItemsTotal += lineItem.valuePerItem;
-            }
-        }
-        if (lineItemsTotal != payload.amount) {
-            throw new HttpStatusException(
-                HttpStatus.BAD_REQUEST,
-                "Total of all line items doesn't equal the transaction's total."
-            );
-        }
-    }
-}
-
 /** 
  * Helper function to add / remove attachments for a transaction.
  */
@@ -501,13 +433,8 @@ void deleteCategory(ProfileDataSource ds, ulong categoryId) {
 
 // Draft services
 
-Page!TransactionDraftListItem getDrafts(ProfileDataSource ds, in PageRequest pr, bool shouldFetchTemplates) {
-    if (shouldFetchTemplates) {
-        return ds.getTransactionDraftRepository()
-            .findAllTemplates(pr);
-    }
-    return ds.getTransactionDraftRepository()
-        .findAllDrafts(pr);
+Page!TransactionDraftListItem getDrafts(ProfileDataSource ds, in PageRequest pr) {
+    return ds.getTransactionDraftRepository().findAll(pr);
 }
 
 TransactionDraftResponse getDraft(ProfileDataSource ds, ulong draftId) {
@@ -530,7 +457,12 @@ TransactionDraftResponse addDraft(ProfileDataSource ds, in TransactionDraftPaylo
     TransactionDraftRepository draftRepo = ds.getTransactionDraftRepository();
     AttachmentRepository attachmentRepo = ds.getAttachmentRepository();
     
-    validateDraftPayload(payload);
+    validateDraftPayload(
+        ds.getTransactionVendorRepository(),
+        ds.getTransactionCategoryRepository(),
+        ds.getAccountRepository(),
+        payload
+    );
     SysTime now = Clock.currTime(UTC());
 
     ulong draftId;
@@ -552,7 +484,12 @@ TransactionDraftResponse updateDraft(
     TransactionDraftRepository draftRepo = ds.getTransactionDraftRepository();
     AttachmentRepository attachmentRepo = ds.getAttachmentRepository();
 
-    validateDraftPayload(payload);
+    validateDraftPayload(
+        ds.getTransactionVendorRepository(),
+        ds.getTransactionCategoryRepository(),
+        ds.getAccountRepository(),
+        payload
+    );
     SysTime now = Clock.currTime(UTC());
 
     ds.doTransaction(() {
@@ -577,10 +514,6 @@ void deleteDraft(ProfileDataSource ds, ulong draftId) {
     });
 }
 
-private void validateDraftPayload(in TransactionDraftPayload payload) {
-    // TODO!
-}
-
 private void updateDraftAttachments(
     ulong draftId,
     SysTime timestamp,

finnow-api/source/util/validation/common.d

@@ -0,0 +1,46 @@
+module util.validation.common;
+
+import handy_http_primitives.optional;
+import std.datetime;
+
+struct ValidationError {
+    string field;
+    string message;
+}
+
+interface ValidationRule(T) {
+    Optional!ValidationError validate(in T payload);
+}
+
+ValidationError[] applyValidationRules(T)(ValidationRule!(T)[] rules, in T payload) {
+    import std.array;
+    auto app = appender!(ValidationError[]);
+    foreach (rule; rules) {
+        auto result = rule.validate(payload);
+        if (result) app ~= result.value;
+    }
+    return app[];
+}
+
+// Helper functions:
+
+void validateTags(in string[] tags) {
+    import std.regex;
+    import handy_http_primitives: HttpStatus, HttpStatusException;
+    foreach (tag; tags) {
+        import std.regex;
+        auto r = ctRegex!(`^[a-z0-9-_]{3,32}$`);
+        if (!matchFirst(tag, r)) {
+            throw new HttpStatusException(HttpStatus.BAD_REQUEST, "Invalid tag: \"" ~ tag ~ "\".");
+        }
+    }
+}
+
+SysTime validateTimestampFormat(string timestampStr) {
+    import handy_http_primitives: HttpStatus, HttpStatusException;
+    try {
+        return SysTime.fromISOExtString(timestampStr, UTC());
+    } catch (TimeException e) {
+        throw new HttpStatusException(HttpStatus.BAD_REQUEST, "Invalid timestamp format. Expected ISO-8601 datetime.");
+    }
+}

finnow-api/source/util/validation/draft.d

@@ -0,0 +1,59 @@
+module util.validation.draft;
+
+import handy_http_primitives;
+import std.datetime;
+
+import transaction.data;
+import transaction.dto;
+import util.validation.common;
+import account.data;
+
+void validateDraftPayload(
+    TransactionVendorRepository vendorRepo,
+    TransactionCategoryRepository categoryRepo,
+    AccountRepository accountRepo,
+    in TransactionDraftPayload payload
+) {
+    if (payload.amount && !payload.currencyCode) {
+        throw new HttpStatusException(HttpStatus.BAD_REQUEST, "Currency is required when saving an amount.");
+    }
+    if (payload.amount && payload.amount.value == 0) {
+        throw new HttpStatusException(HttpStatus.BAD_REQUEST, "Amount should be greater than 0.");
+    }
+    if (payload.timestamp) {
+        validateTimestampFormat(payload.timestamp.value);
+    }
+    if (payload.vendorId && !vendorRepo.existsById(payload.vendorId.value)) {
+        throw new HttpStatusException(HttpStatus.BAD_REQUEST, "Vendor doesn't exist.");
+    }
+    if (payload.categoryId && !categoryRepo.existsById(payload.categoryId.value)) {
+        throw new HttpStatusException(HttpStatus.BAD_REQUEST, "Category doesn't exist.");
+    }
+    if (payload.creditedAccountId && !accountRepo.existsById(payload.creditedAccountId.value)) {
+        throw new HttpStatusException(HttpStatus.BAD_REQUEST, "Credited account doesn't exist.");
+    }
+    if (payload.debitedAccountId && !accountRepo.existsById(payload.debitedAccountId.value)) {
+        throw new HttpStatusException(HttpStatus.BAD_REQUEST, "Debited account doesn't exist.");
+    }
+    validateTags(payload.tags);
+    if (payload.lineItems.length > 0) {
+        long lineItemsTotal = 0;
+        foreach (lineItem; payload.lineItems) {
+            if (lineItem.categoryId && !categoryRepo.existsById(lineItem.categoryId.value)) {
+                throw new HttpStatusException(HttpStatus.BAD_REQUEST, "Line item's category doesn't exist.");
+            }
+            if (lineItem.quantity == 0) {
+                throw new HttpStatusException(HttpStatus.BAD_REQUEST, "Line item's quantity should greater than zero.");
+            }
+            for (ulong i = 0; i < lineItem.quantity; i++) {
+                lineItemsTotal += lineItem.valuePerItem;
+            }
+        }
+        if (payload.amount && lineItemsTotal != payload.amount.value) {
+            throw new HttpStatusException(
+                HttpStatus.BAD_REQUEST,
+                "Total of all line items doesn't equal the transaction's total."
+            );
+        }
+    }
+}

finnow-api/source/util/validation/transaction.d

@@ -0,0 +1,78 @@
+module util.validation.transaction;
+
+import handy_http_primitives;
+import std.array;
+import std.datetime;
+
+import util.validation.common;
+import transaction.dto;
+import transaction.data;
+import account.data;
+
+// class AtLeastOneLinkedAccountRule : ValidationRule!AddTransactionPayload {
+//     Optional!ValidationError validate(in AddTransactionPayload payload) {
+//         if (!payload.creditedAccountId && !payload.debitedAccountId) {
+//             return ValidationError("creditedAccountId", "At least one account must be linked.").toOptional();
+//         }
+//         return Optional!ValidationError.empty();
+//     }
+// }
+
+void validateTransactionPayload(
+    TransactionVendorRepository vendorRepo,
+    TransactionCategoryRepository categoryRepo,
+    AccountRepository accountRepo,
+    in AddTransactionPayload payload
+) {
+    if (!payload.creditedAccountId && !payload.debitedAccountId) {
+        throw new HttpStatusException(HttpStatus.BAD_REQUEST, "At least one account must be linked.");
+    }
+    if (
+        payload.creditedAccountId &&
+        payload.debitedAccountId &&
+        payload.creditedAccountId.value == payload.debitedAccountId.value
+    ) {
+        throw new HttpStatusException(HttpStatus.BAD_REQUEST, "Cannot link the same account as both credit and debit.");
+    }
+    if (payload.amount == 0) {
+        throw new HttpStatusException(HttpStatus.BAD_REQUEST, "Amount should be greater than 0.");
+    }
+    SysTime now = Clock.currTime(UTC());
+    SysTime timestamp = validateTimestampFormat(payload.timestamp);
+    if (timestamp > now) {
+        throw new HttpStatusException(HttpStatus.BAD_REQUEST, "Cannot create transaction in the future.");
+    }
+    if (payload.vendorId && !vendorRepo.existsById(payload.vendorId.value)) {
+        throw new HttpStatusException(HttpStatus.BAD_REQUEST, "Vendor doesn't exist.");
+    }
+    if (payload.categoryId && !categoryRepo.existsById(payload.categoryId.value)) {
+        throw new HttpStatusException(HttpStatus.BAD_REQUEST, "Category doesn't exist.");
+    }
+    if (payload.creditedAccountId && !accountRepo.existsById(payload.creditedAccountId.value)) {
+        throw new HttpStatusException(HttpStatus.BAD_REQUEST, "Credited account doesn't exist.");
+    }
+    if (payload.debitedAccountId && !accountRepo.existsById(payload.debitedAccountId.value)) {
+        throw new HttpStatusException(HttpStatus.BAD_REQUEST, "Debited account doesn't exist.");
+    }
+    validateTags(payload.tags);
+    if (payload.lineItems.length > 0) {
+        long lineItemsTotal = 0;
+        foreach (lineItem; payload.lineItems) {
+            if (lineItem.categoryId && !categoryRepo.existsById(lineItem.categoryId.value)) {
+                throw new HttpStatusException(HttpStatus.BAD_REQUEST, "Line item's category doesn't exist.");
+            }
+            if (lineItem.quantity == 0) {
+                throw new HttpStatusException(HttpStatus.BAD_REQUEST, "Line item's quantity should greater than zero.");
+            }
+            for (ulong i = 0; i < lineItem.quantity; i++) {
+                lineItemsTotal += lineItem.valuePerItem;
+            }
+        }
+        if (lineItemsTotal != payload.amount) {
+            throw new HttpStatusException(
+                HttpStatus.BAD_REQUEST,
+                "Total of all line items doesn't equal the transaction's total."
+            );
+        }
+    }
+}

web-app/src/components/TransactionDraftCard.vue

@@ -26,7 +26,15 @@ function goToDraft() {
       <!-- Top row contains timestamp and amount. -->
       <div style="display: flex; justify-content: space-between">
         <div>
-          <div class="font-mono font-size-xsmall text-normal">Draft #{{ draft.id }}</div>
+          <div class="font-mono font-size-xsmall text-normal">
+            Draft #{{ draft.id }}
+            <AppBadge
+              v-if="draft.templateName"
+              size="sm"
+            >
+              Template: {{ draft.templateName }}
+            </AppBadge>
+          </div>
           <div
             class="text-muted font-mono font-size-xsmall"
             v-if="draft.timestamp"

web-app/src/pages/TransactionDraftPage.vue

@@ -173,7 +173,7 @@ async function onVendorClicked() {
       <AppButton
         icon="wrench"
         @click="
-          router.push(`/profiles/${getSelectedProfile(route)}/transaction-drafts/${draft.id}/edit`)
+          router.push(`/profiles/${getSelectedProfile(route)}/transaction-drafts/${draft?.id}/edit`)
         "
       >
         Edit

web-app/src/pages/transaction-editor/EditTransactionPage.vue

@@ -102,6 +102,7 @@ onMounted(async () => {
             v-model="formData.templateName"
             :disabled="loading"
             style="max-width: 200px"
+            maxlength="32"
           />
         </FormControl>
       </FormGroup>

web-app/src/pages/transaction-editor/util.ts

@@ -84,10 +84,7 @@ export interface TransactionEditorContextBase {
 export class NewTransactionEditorContext implements TransactionEditorContextBase {
   isFormDataValid(formData: TransactionEditorFormFields): boolean {
     return (
-      formData.amount !== null &&
-      formData.amount > 0 &&
-      formData.timestamp !== null &&
-      formData.currency !== null
+      isFormDataValidForDraftSave(formData) || isFormDataValidForTransactionSubmission(formData)
     )
   }
 
@@ -126,7 +123,9 @@ export class NewTransactionEditorContext implements TransactionEditorContextBase
     return [
       {
         name: 'Save',
-        disabled: !(this.areChangesPresent(formData) && this.isFormDataValid(formData)),
+        disabled: !(
+          this.areChangesPresent(formData) && isFormDataValidForTransactionSubmission(formData)
+        ),
         callback: async (formData, route, router) => {
           const api = new TransactionApiClient(getSelectedProfile(route))
           // Assume that form data is valid!
@@ -137,7 +136,7 @@ export class NewTransactionEditorContext implements TransactionEditorContextBase
       },
       {
         name: 'Save Draft',
-        disabled: !this.areChangesPresent(formData),
+        disabled: !this.areChangesPresent(formData) || !isFormDataValidForDraftSave(formData),
         callback: async (formData, route, router) => {
           const api = new TransactionApiClient(getSelectedProfile(route))
           const data = toDraftPayload(formData)
@@ -169,17 +168,7 @@ export class TransactionEditorContext implements TransactionEditorContextBase {
   }
 
   isFormDataValid(formData: TransactionEditorFormFields): boolean {
-    return (
-      formData.timestamp !== null &&
-      formData.timestamp.length > 0 &&
-      formData.amount !== null &&
-      formData.amount > 0 &&
-      formData.currency !== null &&
-      formData.description !== null &&
-      formData.description.length > 0 &&
-      (formData.creditedAccountId !== null || formData.debitedAccountId !== null) &&
-      formData.creditedAccountId !== formData.debitedAccountId
-    )
+    return isFormDataValidForTransactionSubmission(formData)
   }
 
   areChangesPresent(formData: TransactionEditorFormFields): boolean {
@@ -278,9 +267,16 @@ export class DraftEditorContext implements TransactionEditorContextBase {
     this.existingDraft = existingDraft
   }
 
-  isFormDataValid(): boolean {
-    // TODO: What validation is needed client-side for draft data?
-    return true
+  isFormDataValid(formData: TransactionEditorFormFields): boolean {
+    const result =
+      (formData.amount === null || formData.currency !== null) &&
+      (formData.amount === null || formData.amount > 0) &&
+      (formData.creditedAccountId === null ||
+        formData.debitedAccountId === null ||
+        formData.creditedAccountId !== formData.debitedAccountId) &&
+      (formData.templateName === null || formData.templateName.length <= 32)
+    console.log('Checking draft editor valid:', formData, result)
+    return result
   }
 
   areChangesPresent(): boolean {
@@ -325,11 +321,11 @@ export class DraftEditorContext implements TransactionEditorContextBase {
     return fields
   }
 
-  getActions(): TransactionEditorAction[] {
+  getActions(formData: TransactionEditorFormFields): TransactionEditorAction[] {
     return [
       {
-        name: 'Save',
-        disabled: !this.areChangesPresent() || !this.isFormDataValid(),
+        name: 'Save Draft',
+        disabled: !this.areChangesPresent() || !this.isFormDataValid(formData),
         callback: async (formData, route, router) => {
           const api = new TransactionApiClient(getSelectedProfile(route))
           const data = toDraftPayload(formData)
@@ -343,6 +339,21 @@ export class DraftEditorContext implements TransactionEditorContextBase {
           )
         },
       },
+      {
+        name: 'Submit Transaction',
+        disabled: !this.areChangesPresent() || !isFormDataValidForTransactionSubmission(formData),
+        callback: async (formData, route, router) => {
+          const api = new TransactionApiClient(getSelectedProfile(route))
+          // First call the normal "Save" callback from the NewTransactionEditorContext.
+          const tec = new NewTransactionEditorContext()
+          const saveTxnAction = tec.getActions(formData).find((a) => a.name === 'Save')!
+          await saveTxnAction.callback(formData, route, router)
+          // Then if this is not a template draft, delete it.
+          if (formData.templateName === null || formData.templateName.length === 0) {
+            await api.deleteDraft(this.existingDraft.id)
+          }
+        },
+      },
       {
         name: 'Cancel',
         disabled: false,
@@ -354,6 +365,8 @@ export class DraftEditorContext implements TransactionEditorContextBase {
   }
 }
 
+// Helper functions below here!
+
 /**
  * Obtains an editor context by determining what the user is doing based on the
  * route they've navigated to.
@@ -455,3 +468,26 @@ async function goBackOrHome(router: Router, route: RouteLocation) {
     await router.replace(`/profiles/${getSelectedProfile(route)}`)
   }
 }
+
+function isFormDataValidForTransactionSubmission(formData: TransactionEditorFormFields): boolean {
+  return (
+    formData.timestamp !== null &&
+    formData.timestamp.length > 0 &&
+    formData.amount !== null &&
+    formData.amount > 0 &&
+    formData.currency !== null &&
+    formData.description !== null &&
+    formData.description.length > 0 &&
+    (formData.creditedAccountId !== null || formData.debitedAccountId !== null) &&
+    formData.creditedAccountId !== formData.debitedAccountId
+  )
+}
+
+function isFormDataValidForDraftSave(formData: TransactionEditorFormFields): boolean {
+  return (
+    formData.amount !== null &&
+    formData.amount > 0 &&
+    formData.timestamp !== null &&
+    formData.currency !== null
+  )
+}