chore: review server actions (#4868)

* chore: review server actions and make sure to delimit public facing functions from internal ones

* delete unused server actions

* remove privilege checks duplcation

* fix issue with error message translation for reset password component

* fix merge conflicts

app/(gcforms)/[locale]/(app administration)/admin/(no nav)/view-templates/actions.ts

@@ -1,8 +1,11 @@
 "use server";
+
 import { getAllTemplates } from "@lib/templates";
 import { AuthenticatedAction } from "@lib/actions";
 import { FormRecord } from "@lib/types";
 
+// Public facing functions - they can be used by anyone who finds the associated server action identifer
+
 export const getTemplates = AuthenticatedAction(async () => {
   const templates = await getAllTemplates();
   return filterTemplateProperties(templates);
@@ -17,6 +20,8 @@ export const getLatestPublishedTemplates = AuthenticatedAction(async () => {
   return filterTemplateProperties(templates);
 });
 
+// Internal and private functions - won't be converted into server actions
+
 const filterTemplateProperties = (templates: FormRecord[]) => {
   return templates.map((template) => {
     const {

app/(gcforms)/[locale]/(app administration)/admin/(with nav)/accounts/[id]/manage-forms/actions.ts

@@ -1,18 +1,11 @@
 "use server";
-import { cache } from "react";
 
 import { deleteTemplate } from "@lib/templates";
 import { TemplateHasUnprocessedSubmissions } from "@lib/templates";
-import { getAppSetting } from "@lib/appSettings";
 import { revalidatePath } from "next/cache";
 import { AuthenticatedAction } from "@lib/actions";
 
-export const overdueSettings = cache(async () => {
-  const promptPhaseDays = await getAppSetting("nagwarePhasePrompted");
-  const warnPhaseDays = await getAppSetting("nagwarePhaseWarned");
-  const responseDownloadLimit = await getAppSetting("responseDownloadLimit");
-  return { promptPhaseDays, warnPhaseDays, responseDownloadLimit };
-});
+// Public facing functions - they can be used by anyone who finds the associated server action identifer
 
 export const deleteForm = AuthenticatedAction(async (id: string) => {
   try {

app/(gcforms)/[locale]/(app administration)/admin/(with nav)/accounts/[id]/manage-permissions/actions.ts

@@ -1,25 +1,26 @@
 "use server";
+
 import { updatePrivilegesForUser } from "@lib/privileges";
-import { checkPrivilegesAsBoolean } from "@lib/privileges";
 import { revalidatePath } from "next/cache";
 import { authCheckAndThrow } from "@lib/actions";
 
+// Public facing functions - they can be used by anyone who finds the associated server action identifer
+
 export const updatePrivileges = async (
   userID: string,
   privilegeID: string,
   action: "add" | "remove"
 ) => {
   const { ability } = await authCheckAndThrow();
-  if (checkPrivilegesAsBoolean(ability, [{ action: "update", subject: "User" }])) {
-    try {
-      const result = await updatePrivilegesForUser(ability, userID, [{ id: privilegeID, action }]);
-      revalidatePath(
-        "(gcforms)/[locale]/(app administration)/admin/(with nav)/accounts/[id]/manage-permissions",
-        "page"
-      );
-      return { data: result };
-    } catch (e) {
-      return { error: "Failed to update permissions." };
-    }
+
+  try {
+    const result = await updatePrivilegesForUser(ability, userID, [{ id: privilegeID, action }]);
+    revalidatePath(
+      "(gcforms)/[locale]/(app administration)/admin/(with nav)/accounts/[id]/manage-permissions",
+      "page"
+    );
+    return { data: result };
+  } catch (e) {
+    return { error: "Failed to update permissions." };
   }
 };

app/(gcforms)/[locale]/(app administration)/admin/(with nav)/accounts/actions.ts

@@ -1,29 +1,26 @@
 "use server";
 
-import { checkPrivilegesAsBoolean, updatePrivilegesForUser, getPrivilege } from "@lib/privileges";
+import { updatePrivilegesForUser, getPrivilege } from "@lib/privileges";
 import { revalidatePath } from "next/cache";
 import { getUsers, updateActiveStatus } from "@lib/users";
 import { authCheckAndThrow } from "@lib/actions";
 
+// Public facing functions - they can be used by anyone who finds the associated server action identifer
+
 export const updatePublishing = async (
   userID: string,
   publishFormsId: string,
   action: "add" | "remove"
 ) => {
   const { ability } = await authCheckAndThrow();
-  if (checkPrivilegesAsBoolean(ability, [{ action: "update", subject: "User" }])) {
-    await updatePrivilegesForUser(ability, userID, [{ id: publishFormsId, action }]);
-    revalidatePath("(gcforms)/[locale]/(app administration)/admin/(with nav)/accounts", "page");
-  }
+  await updatePrivilegesForUser(ability, userID, [{ id: publishFormsId, action }]);
+  revalidatePath("(gcforms)/[locale]/(app administration)/admin/(with nav)/accounts", "page");
 };
 
 export const updateActive = async (userID: string, active: boolean) => {
   const { ability } = await authCheckAndThrow();
-
-  if (checkPrivilegesAsBoolean(ability, [{ action: "update", subject: "User" }])) {
-    await updateActiveStatus(ability, userID, active);
-    revalidatePath("(gcforms)/[locale]/(app administration)/admin/(with nav)/accounts", "page");
-  }
+  await updateActiveStatus(ability, userID, active);
+  revalidatePath("(gcforms)/[locale]/(app administration)/admin/(with nav)/accounts", "page");
 };
 
 export const getAllUsers = async (active?: boolean) => {

app/(gcforms)/[locale]/(app administration)/admin/(with nav)/flags/actions.ts

@@ -1,8 +1,11 @@
 "use server";
-import { enableFlag, disableFlag, checkAll, checkOne } from "@lib/cache/flags";
+
+import { enableFlag, disableFlag } from "@lib/cache/flags";
 import { revalidatePath } from "next/cache";
 import { authCheckAndThrow } from "@lib/actions";
 
+// Public facing functions - they can be used by anyone who finds the associated server action identifer
+
 // Note: any thrown errors will be caught in the Error boundary/component
 
 export async function modifyFlag(id: string, value: boolean) {
@@ -15,15 +18,3 @@ export async function modifyFlag(id: string, value: boolean) {
   }
   revalidatePath("(gcforms)/[locale]/(app administration)/admin/(with nav)/flags", "page");
 }
-
-export async function getAllFlags() {
-  const { ability } = await authCheckAndThrow();
-
-  return checkAll(ability);
-}
-
-export async function checkFlag(id: string) {
-  await authCheckAndThrow();
-
-  return checkOne(id);
-}

app/(gcforms)/[locale]/(app administration)/admin/(with nav)/settings/actions.ts

@@ -7,19 +7,13 @@ import {
   updateAppSetting,
 } from "@lib/appSettings";
 import { revalidatePath } from "next/cache";
-import { logMessage } from "@lib/logger";
 import { authCheckAndThrow } from "@lib/actions";
 import { redirect } from "next/navigation";
 
-function nullCheck(formData: FormData, key: string) {
-  const result = formData.get(key);
-  if (!result) throw new Error(`No value found for ${key}`);
-  return result as string;
-}
+// Public facing functions - they can be used by anyone who finds the associated server action identifer
 
 export async function getSetting(internalId: string) {
   const { ability } = await authCheckAndThrow();
-  logMessage.error("Getting setting with internalId: " + internalId);
   return getFullAppSetting(ability, internalId);
 }
 
@@ -74,3 +68,11 @@ export async function deleteSetting(internalId: string) {
   });
   revalidatePath("(gcforms)/[locale]/(app administration)/admin/(with nav)/settings", "page");
 }
+
+// Internal and private functions - won't be converted into server actions
+
+function nullCheck(formData: FormData, key: string) {
+  const result = formData.get(key);
+  if (!result) throw new Error(`No value found for ${key}`);
+  return result as string;
+}

app/(gcforms)/[locale]/(app administration)/admin/(with nav)/settings/components/server/ManageSettingForm.tsx

@@ -2,7 +2,6 @@ import { serverTranslation } from "@i18n";
 import { createSetting, getSetting, updateSetting } from "../../actions";
 import { LinkButton } from "@serverComponents/globals/Buttons/LinkButton";
 import { SaveButton } from "../client/SaveButton";
-
 import { Danger } from "@clientComponents/globals/Alert/Alert";
 import { Label } from "@clientComponents/forms";
 

app/(gcforms)/[locale]/(form administration)/form-builder/[id]/responses/[[...statusFilter]]/actions.ts

@@ -1,4 +1,5 @@
 "use server";
+
 import { Language, FormServerErrorCodes, ServerActionError } from "@lib/types/form-builder-types";
 import { getAppSetting } from "@lib/appSettings";
 import { logEvent } from "@lib/auditLogs";
@@ -53,6 +54,8 @@ import {
 } from "@clientComponents/forms/AddressComplete/utils";
 import { serverTranslation } from "@i18n";
 
+// Public facing functions - they can be used by anyone who finds the associated server action identifer
+
 export const fetchSubmissions = AuthenticatedAction(
   async ({
     formId,
@@ -106,63 +109,6 @@ export const fetchSubmissions = AuthenticatedAction(
   }
 );
 
-const sortByLayout = ({ layout, elements }: { layout: number[]; elements: Answer[] }) => {
-  return elements.sort((a, b) => layout.indexOf(a.questionId) - layout.indexOf(b.questionId));
-};
-
-const sortByGroups = ({ form, elements }: { form: FormProperties; elements: Answer[] }) => {
-  const groups = orderGroups(form.groups, form.groupsLayout) ?? {};
-  const layout = getLayoutFromGroups(form, groups);
-  return sortByLayout({ layout, elements });
-};
-
-const getAnswerAsString = (question: FormElement | undefined, answer: unknown): string => {
-  if (question && question.type === "checkbox") {
-    return Array(answer).join(", ");
-  }
-
-  if (question && question.type === "formattedDate") {
-    // Could be empty if the date was not required
-    if (!answer) {
-      return "";
-    }
-    const dateFormat = (question.properties.dateFormat || "YYYY-MM-DD") as DateFormat;
-    const dateObject = JSON.parse(answer as string) as DateObject;
-
-    return getFormattedDateFromObject(dateFormat, dateObject);
-  }
-
-  if (question && question.type === "addressComplete") {
-    if (!answer) {
-      return "";
-    }
-
-    if (question.properties.addressComponents?.splitAddress === true) {
-      return answer as string; //Address was split, return as is.
-    }
-
-    const addressObject = JSON.parse(answer as string) as AddressElements;
-    return getAddressAsString(addressObject);
-  }
-
-  return answer as string;
-};
-
-const logDownload = async (
-  responseIdStatusArray: { id: string; status: string }[],
-  format: DownloadFormat
-) => {
-  const ability = await getAbility();
-  responseIdStatusArray.forEach((item) => {
-    logEvent(
-      ability.user.id,
-      { type: "Response", id: item.id },
-      "DownloadResponse",
-      `Downloaded form response in ${format} for submission ID ${item.id}`
-    );
-  });
-};
-
 export const getSubmissionsByFormat = AuthenticatedAction(
   async ({
     formID,
@@ -444,3 +390,62 @@ export const getSubmissionRemovalDate = AuthenticatedAction(
     }
   }
 );
+
+// Internal and private functions - won't be converted into server actions
+
+const sortByLayout = ({ layout, elements }: { layout: number[]; elements: Answer[] }) => {
+  return elements.sort((a, b) => layout.indexOf(a.questionId) - layout.indexOf(b.questionId));
+};
+
+const sortByGroups = ({ form, elements }: { form: FormProperties; elements: Answer[] }) => {
+  const groups = orderGroups(form.groups, form.groupsLayout) ?? {};
+  const layout = getLayoutFromGroups(form, groups);
+  return sortByLayout({ layout, elements });
+};
+
+const getAnswerAsString = (question: FormElement | undefined, answer: unknown): string => {
+  if (question && question.type === "checkbox") {
+    return Array(answer).join(", ");
+  }
+
+  if (question && question.type === "formattedDate") {
+    // Could be empty if the date was not required
+    if (!answer) {
+      return "";
+    }
+    const dateFormat = (question.properties.dateFormat || "YYYY-MM-DD") as DateFormat;
+    const dateObject = JSON.parse(answer as string) as DateObject;
+
+    return getFormattedDateFromObject(dateFormat, dateObject);
+  }
+
+  if (question && question.type === "addressComplete") {
+    if (!answer) {
+      return "";
+    }
+
+    if (question.properties.addressComponents?.splitAddress === true) {
+      return answer as string; //Address was split, return as is.
+    }
+
+    const addressObject = JSON.parse(answer as string) as AddressElements;
+    return getAddressAsString(addressObject);
+  }
+
+  return answer as string;
+};
+
+const logDownload = async (
+  responseIdStatusArray: { id: string; status: string }[],
+  format: DownloadFormat
+) => {
+  const ability = await getAbility();
+  responseIdStatusArray.forEach((item) => {
+    logEvent(
+      ability.user.id,
+      { type: "Response", id: item.id },
+      "DownloadResponse",
+      `Downloaded form response in ${format} for submission ID ${item.id}`
+    );
+  });
+};

app/(gcforms)/[locale]/(form administration)/form-builder/[id]/responses/[[...statusFilter]]/components/ManageFormAccessDialog/actions.ts

@@ -16,6 +16,8 @@ import { logMessage } from "@lib/logger";
 import { inviteUserByEmail } from "@lib/invitations/inviteUserByEmail";
 import { cancelInvitation as cancelInvitationAction } from "@lib/invitations/cancelInvitation";
 
+// Public facing functions - they can be used by anyone who finds the associated server action identifer
+
 export const sendInvitation = async (emails: string[], templateId: string, message: string) => {
   const { ability } = await authCheckAndThrow();
   const { t } = await serverTranslation("manage-form-access");

app/(gcforms)/[locale]/(form administration)/form-builder/[id]/settings/actions.ts

@@ -1,11 +1,11 @@
 "use server";
+
 import { createKey, deleteKey, refreshKey } from "@lib/serviceAccount";
 import { revalidatePath } from "next/cache";
-
 import { promises as fs } from "fs";
 import path from "path";
 
-// Privilege Checks are done at the lib/serviceAccount.ts level
+// Public facing functions - they can be used by anyone who finds the associated server action identifer
 
 export const getReadmeContent = async () => {
   try {
@@ -17,6 +17,8 @@ export const getReadmeContent = async () => {
   }
 };
 
+// Privilege Checks are done at the lib/serviceAccount.ts level for the next server actions
+
 export const createServiceAccountKey = async (templateId: string) => {
   revalidatePath(
     "/app/(gcforms)/[locale]/(form administration)/form-builder/[id]/settings/api",

app/(gcforms)/[locale]/(form administration)/form-builder/[id]/settings/manage/actions.ts

@@ -2,6 +2,8 @@
 
 import { headers } from "next/headers";
 
+// Public facing functions - they can be used by anyone who finds the associated server action identifer
+
 // Can throw because it is not called by Client Components
 // @todo Should these types of functions be moved to a different file?
 export const getNonce = async () => {