Remove invalid assertions and returning() calls

Knex's returning() API is intended for INSERT, UPDATE and DELETE calls, not SELECTs: https://knexjs.org/guide/query-builder.html#returning I've updated the .where() queries to replace the returning() calls, i.e. by making them select all columns. I then also removed the type assertions, so as to not indicate that rows will always be returned even in cases where they might not be available. The type assertion also pretended that the `monthly_monitor_report_free` column could only be `boolean | undefined`, even though it can be `null` too (see `addUnsubcribeTokenForSubscriber`, which sets it to `null`).
mozilla · Jan 28, 2025 · d12cea0 · d12cea0
1 parent 1a10216
commit d12cea0
Show file tree

Hide file tree

Showing 4 changed files with 36 additions and 56 deletions.
diff --git a/...(proper_react)/(redesign)/(authenticated)/user/(dashboard)/settings/SettingsPage.test.tsx b/...(proper_react)/(redesign)/(authenticated)/user/(dashboard)/settings/SettingsPage.test.tsx
@@ -226,25 +226,25 @@ const mockedSubscriptionBillingAmount = {
   yearly: 13.37,
   monthly: 42.42,
 };
-const mockedPlusSubscriberEmailPreferences: SubscriberEmailPreferencesOutput = {
+const mockedPlusSubscriberEmailPreferences = {
   id: 1337,
   primary_email: "[email protected]",
   unsubscribe_token: "495398jfjvjfdj",
   monthly_monitor_report_free: false,
   monthly_monitor_report_free_at: new Date("1337-04-02T04:02:42.000Z"),
   monthly_monitor_report: true,
   monthly_monitor_report_at: new Date("1337-04-02T04:02:42.000Z"),
-};
+} as SubscriberEmailPreferencesOutput;
 
-const mockedFreeSubscriberEmailPreferences: SubscriberEmailPreferencesOutput = {
+const mockedFreeSubscriberEmailPreferences = {
   id: 1337,
   primary_email: "[email protected]",
   unsubscribe_token: "495398jfjvjfdj",
   monthly_monitor_report_free: true,
   monthly_monitor_report_free_at: new Date("1337-04-02T04:02:42.000Z"),
   monthly_monitor_report: false,
   monthly_monitor_report_at: new Date("1337-04-02T04:02:42.000Z"),
-};
+} as SubscriberEmailPreferencesOutput;
 
 const mockedSession = {
   expires: new Date().toISOString(),

diff --git a/...redesign)/(authenticated)/user/(dashboard)/settings/panels/SettingsPanelNotifications.tsx b/...redesign)/(authenticated)/user/(dashboard)/settings/panels/SettingsPanelNotifications.tsx
@@ -52,9 +52,10 @@ export const NotificationsSettings = (props: NotificationSettingsProps) => {
   const breachAlertsEmailsAllowed = props.subscriber.all_emails_to_primary;
 
   // Extract monthly report preference from the right column
-  const monitorReportAllowed = hasPremium(props.user)
-    ? props.data?.monthly_monitor_report
-    : props.data?.monthly_monitor_report_free;
+  const monitorReportAllowed =
+    (hasPremium(props.user)
+      ? props.data?.monthly_monitor_report
+      : props.data?.monthly_monitor_report_free) ?? true;
 
   const defaultActivateAlertEmail =
     typeof breachAlertsEmailsAllowed === "boolean";

diff --git a/src/app/functions/cronjobs/unsubscribeLinks.ts b/src/app/functions/cronjobs/unsubscribeLinks.ts
@@ -17,7 +17,6 @@ export async function getMonthlyActivityFreeUnsubscribeLink(
 ) {
   try {
     const newUnsubToken = randomBytes(64).toString("hex");
-    let sub;
     const getRes = await getEmailPreferenceForSubscriber(subscriber.id);
     if (getRes.unsubscribe_token) {
       // if record has been created and the token exists, return the token
@@ -27,18 +26,15 @@ export async function getMonthlyActivityFreeUnsubscribeLink(
       !getRes.unsubscribe_token
     ) {
       // if record in the new table has not been created
-      sub = await addUnsubscribeTokenForSubscriber(
-        subscriber.id,
-        newUnsubToken,
-      );
+      await addUnsubscribeTokenForSubscriber(subscriber.id, newUnsubToken);
     } else {
       // if record already exists, but token doesn't exist, add the token
-      sub = await updateEmailPreferenceForSubscriber(subscriber.id, true, {
+      await updateEmailPreferenceForSubscriber(subscriber.id, true, {
         unsubscribe_token: newUnsubToken,
       });
     }
 
-    return `${process.env.SERVER_URL}/unsubscribe-email/monthly-report-free?token=${sub.unsubscribe_token}`;
+    return `${process.env.SERVER_URL}/unsubscribe-email/monthly-report-free?token=${newUnsubToken}`;
   } catch (e) {
     logger.error("generate_unsubscribe_link", {
       exception: e,

diff --git a/src/db/tables/subscriber_email_preferences.ts b/src/db/tables/subscriber_email_preferences.ts
@@ -5,6 +5,10 @@
 import createDbConnection from "../connect";
 import { logger } from "../../app/functions/server/logging";
 import { captureException } from "@sentry/node";
+import {
+  SubscriberEmailPreferencesRow,
+  SubscriberRow,
+} from "knex/types/tables";
 
 const knex = createDbConnection();
 
@@ -21,22 +25,15 @@ interface SubscriberPlusEmailPreferencesInput {
   monthly_monitor_report_at?: Date;
 }
 
-export interface SubscriberEmailPreferencesOutput {
-  id?: number;
-  primary_email?: string;
-  unsubscribe_token?: string;
-  monthly_monitor_report_free?: boolean;
-  monthly_monitor_report_free_at?: Date;
-  monthly_monitor_report?: boolean;
-  monthly_monitor_report_at?: Date;
-}
+export type SubscriberEmailPreferencesOutput = SubscriberRow &
+  Partial<SubscriberEmailPreferencesRow>;
 
 // TODO: modify after MNTOR-3557 - pref currently lives in two tables
 // this function only adds email prefs for free reports
 async function addEmailPreferenceForSubscriber(
   subscriberId: number,
   preference: SubscriberFreeEmailPreferencesInput,
-) {
+): Promise<SubscriberEmailPreferencesRow> {
   logger.info("add_email_preference_for_subscriber", {
     subscriberId,
     preference,
@@ -73,7 +70,7 @@ async function addEmailPreferenceForSubscriber(
 async function addUnsubscribeTokenForSubscriber(
   subscriberId: number,
   token: string,
-) {
+): Promise<SubscriberEmailPreferencesRow> {
   logger.info("add_unsubscribe_token_for_subscriber", {
     subscriberId,
   });
@@ -125,15 +122,15 @@ async function updateEmailPreferenceForSubscriber(
         );
       } else {
         res = (
-          (await knex("subscriber_email_preferences")
+          await knex("subscriber_email_preferences")
             .where("subscriber_id", subscriberId)
             .update({
               ...(preference as SubscriberFreeEmailPreferencesInput),
               // @ts-ignore knex.fn.now() results in it being set to a date,
               // even if it's not typed as a JS date object:
               monthly_monitor_report_free_at: knex.fn.now(),
             })
-            .returning("*")) as SubscriberEmailPreferencesOutput[]
+            .returning("*")
         )?.[0];
       }
       if (!res) {
@@ -144,15 +141,15 @@ async function updateEmailPreferenceForSubscriber(
     } else {
       // TODO: modify after MNTOR-3557 - pref currently lives in two tables
       res = (
-        (await knex("subscribers")
+        await knex("subscribers")
           .where("id", subscriberId)
           .update({
             ...(preference as SubscriberPlusEmailPreferencesInput),
             // @ts-ignore knex.fn.now() results in it being set to a date,
             // even if it's not typed as a JS date object:
             updated_at: knex.fn.now(),
           })
-          .returning("*")) as SubscriberEmailPreferencesOutput[]
+          .returning("*")
       )?.[0];
 
       if (!res) {
@@ -216,7 +213,9 @@ async function getEmailPreferenceForSubscriber(subscriberId: number) {
   return res?.[0];
 }
 
-async function getEmailPreferenceForUnsubscribeToken(unsubscribeToken: string) {
+async function getEmailPreferenceForUnsubscribeToken(
+  unsubscribeToken: string,
+): Promise<SubscriberEmailPreferencesRow | undefined> {
   logger.info("get_email_preference_for_unsubscribe_token", {
     token: unsubscribeToken,
   });
@@ -225,14 +224,8 @@ async function getEmailPreferenceForUnsubscribeToken(unsubscribeToken: string) {
   // TODO: modify after MNTOR-3557 - pref currently lives in two tables, we have to join the tables
   try {
     res = await knex("subscriber_email_preferences")
-      .select(
-        "subscriber_id AS id",
-        "monthly_monitor_report_free",
-        "monthly_monitor_report_free_at",
-        "unsubscribe_token",
-      )
-      .where("unsubscribe_token", unsubscribeToken)
-      .returning(["*"]);
+      .select("*")
+      .where("unsubscribe_token", unsubscribeToken);
 
     logger.debug(
       `get_email_preference_for_unsubscriber_token: ${JSON.stringify(res)}`,
@@ -249,17 +242,16 @@ async function getEmailPreferenceForUnsubscribeToken(unsubscribeToken: string) {
 
     throw e;
   }
-  return res?.[0] as SubscriberEmailPreferencesOutput;
+  return res?.[0];
 }
 
 // Not covered by tests; mostly side-effects. See test-coverage.md#mock-heavy
 /* c8 ignore start */
 async function unsubscribeMonthlyMonitorReportForUnsubscribeToken(
   unsubscribeToken: string,
 ) {
-  let sub;
   try {
-    sub = await getEmailPreferenceForUnsubscribeToken(unsubscribeToken);
+    const sub = await getEmailPreferenceForUnsubscribeToken(unsubscribeToken);
 
     if (
       typeof sub?.id === "number" &&
@@ -291,7 +283,9 @@ async function unsubscribeMonthlyMonitorReportForUnsubscribeToken(
 }
 /* c8 ignore stop */
 
-async function getEmailPreferenceForPrimaryEmail(email: string) {
+async function getEmailPreferenceForPrimaryEmail(
+  email: string,
+): Promise<SubscriberEmailPreferencesOutput> {
   logger.info("get_email_preference_for_primary_email", {
     email,
   });
@@ -300,25 +294,14 @@ async function getEmailPreferenceForPrimaryEmail(email: string) {
   // TODO: modify after MNTOR-3557 - pref currently lives in two tables, we have to join the tables
   try {
     res = await knex
-      .select(
-        "subscribers.primary_email",
-        "subscribers.id",
-        "subscribers.all_emails_to_primary",
-        "subscribers.monthly_monitor_report",
-        "subscribers.monthly_monitor_report_at",
-        "subscribers.first_broker_removal_email_sent",
-        "subscriber_email_preferences.monthly_monitor_report_free",
-        "subscriber_email_preferences.monthly_monitor_report_free_at",
-        "subscriber_email_preferences.unsubscribe_token",
-      )
+      .select("*")
       .from("subscribers")
       .where("subscribers.primary_email", email)
       .leftJoin(
         "subscriber_email_preferences",
         "subscribers.id",
         "subscriber_email_preferences.subscriber_id",
-      )
-      .returning(["*"]);
+      );
 
     logger.debug("get_email_preference_for_primary_email_success");
     logger.debug(
@@ -332,7 +315,7 @@ async function getEmailPreferenceForPrimaryEmail(email: string) {
 
     throw e;
   }
-  return res?.[0] as SubscriberEmailPreferencesOutput;
+  return res?.[0];
 }
 
 export {