no-unnecessary-condition lint rule violations fixed

Author: AnastasiiaBalenko

eslint-suppressions.json

@@ -1,9 +1,4 @@
 {
-  "src/server/api/bannerRouter.ts": {
-    "@typescript-eslint/no-unnecessary-condition": {
-      "count": 1
-    }
-  },
   "src/server/middleware/errorHandling.ts": {
     "@typescript-eslint/no-unsafe-assignment": {
       "count": 1
@@ -14,59 +9,9 @@
       "count": 8
     }
   },
-  "src/server/selection/ab.ts": {
-    "@typescript-eslint/no-unnecessary-condition": {
-      "count": 1
-    }
-  },
-  "src/server/selection/epsilonGreedySelection.ts": {
-    "@typescript-eslint/no-unnecessary-condition": {
-      "count": 1
-    }
-  },
-  "src/server/selection/helpers.ts": {
-    "@typescript-eslint/no-unnecessary-condition": {
-      "count": 1
-    }
-  },
-  "src/server/selection/selectVariant.ts": {
-    "@typescript-eslint/no-unnecessary-condition": {
-      "count": 1
-    }
-  },
-  "src/server/tests/banners/bannerSelection.ts": {
-    "@typescript-eslint/no-unnecessary-condition": {
-      "count": 3
-    }
-  },
-  "src/server/tests/epics/epicSelection.ts": {
-    "@typescript-eslint/no-unnecessary-condition": {
-      "count": 1
-    }
-  },
-  "src/server/tests/headers/headerSelection.ts": {
-    "@typescript-eslint/no-unnecessary-condition": {
-      "count": 2
-    }
-  },
   "src/server/utils/valueReloader.ts": {
     "@typescript-eslint/no-floating-promises": {
       "count": 1
     }
-  },
-  "src/shared/lib/geolocation.ts": {
-    "@typescript-eslint/no-unnecessary-condition": {
-      "count": 1
-    }
-  },
-  "src/shared/lib/placeholders.ts": {
-    "@typescript-eslint/no-unnecessary-condition": {
-      "count": 2
-    }
-  },
-  "src/shared/types/abTests/banner.ts": {
-    "@typescript-eslint/no-unnecessary-condition": {
-      "count": 1
-    }
   }
 }

src/server/api/bannerRouter.ts

@@ -104,7 +104,7 @@ export const buildBannerRouter = (
             const design = getDesignForVariant(variant, bannerDesigns.get());
 
             const contributionAmounts = choiceCardAmounts.get();
-            const requiredCountry = targeting.countryCode ?? 'GB';
+            const requiredCountry = targeting.countryCode || 'GB';
             const requiredRegion = countryCodeToCountryGroupId(requiredCountry);
             const targetingMvtId = targeting.mvtId || 1;
             const variantAmounts = selectAmountsTestVariant(

src/server/selection/ab.ts

@@ -106,10 +106,6 @@ export const selectAmountsTestVariant = (
     const assignmentIndex = getRandomNumber(`${seed + mvtId}`) % variants.length;
     const variant = variants[assignmentIndex];
 
-    if (!variant) {
-        return undefined;
-    }
-
     // Test is running and user has been successfully assigned to a control/variant
     return {
         testName: liveTestName ?? `${testName}_AB_TEST`,

src/server/selection/epsilonGreedySelection.ts

@@ -29,10 +29,6 @@ export function selectVariantWithHighestMean<V extends Variant, T extends Test<V
             ? sortedVariants[0]
             : sortedVariants[Math.floor(Math.random() * bestVariants)]; // more than 1
 
-    if (!variant) {
-        return undefined;
-    }
-
     return test.variants.find((v) => v.name === variant.variantName);
 }
 

src/server/selection/helpers.ts

@@ -6,7 +6,7 @@ import type { BanditVariantData } from './banditData';
 
 export function selectRandomVariant<V extends Variant, T extends Test<V>>(test: T): V | undefined {
     const randomVariantIndex = Math.floor(Math.random() * test.variants.length);
-    const randomVariantData = test.variants[randomVariantIndex];
+    const randomVariantData = test.variants.length && test.variants[randomVariantIndex];
 
     if (!randomVariantData) {
         logError(`Failed to select random variant for bandit test: ${test.name}`);

src/server/selection/selectVariant.ts

@@ -67,12 +67,10 @@ export const selectVariant = <V extends Variant, T extends Test<V>>(
     } else {
         // No configured methodology, default to AB test
         const variant = selectVariantUsingMVT<V, T>(test, mvtId);
-        if (variant) {
-            return {
-                test,
-                variant,
-            };
-        }
+        return {
+            test,
+            variant,
+        };
     }
     return;
 };

src/server/tests/banners/bannerSelection.ts

@@ -179,8 +179,8 @@ const purchaseMatches = (
     }
 
     const { product, userType } = purchaseInfo;
-    const productValid = product && testPurchaseInfo?.product.includes(product);
-    const userValid = userType && testPurchaseInfo?.userType.includes(userType);
+    const productValid = testPurchaseInfo?.product.includes(product);
+    const userValid = testPurchaseInfo?.userType.includes(userType);
 
     return productValid && userValid;
 };
@@ -245,15 +245,7 @@ export const selectBannerTest = (
             purchaseMatches(test, targeting.purchaseInfo, targeting.isSignedIn) &&
             canShowBannerAgain(targeting, test, bannerDeployTimes, now, deploySchedule) &&
             correctSignedInStatus(targeting.isSignedIn, test.signedInStatus) &&
-            pageContextMatches(
-                targeting,
-                test.contextTargeting ?? {
-                    tagIds: [],
-                    sectionIds: [],
-                    excludedTagIds: [],
-                    excludedSectionIds: [],
-                },
-            ) &&
+            pageContextMatches(targeting, test.contextTargeting) &&
             consentStatusMatches(targeting.hasConsented, test.consentStatus) &&
             abandonedBasketMatches(test.bannerChannel, targeting.abandonedBasket) &&
             matchesFrontsOnlyRequirement(test, targeting)

src/server/tests/epics/epicSelection.ts

@@ -170,7 +170,7 @@ export const deviceTypeMatchesFilter = (userDeviceType: UserDeviceType): Filter
 
 type FilterResults = Record<string, boolean>;
 
-export type Debug = Record<string, FilterResults>;
+export type Debug = Record<string, FilterResults | undefined>;
 
 export interface Result {
     result?: {
@@ -216,9 +216,9 @@ export const findTestAndVariant = (
         const test = tests.find((test) =>
             filters.every((filter) => {
                 const got = filter.test(test, targeting);
-
-                if (debug[test.name]) {
-                    debug[test.name][filter.id] = got;
+                const debugResults = debug[test.name];
+                if (debugResults) {
+                    debugResults[filter.id] = got;
                 } else {
                     debug[test.name] = { [filter.id]: got };
                 }

src/server/tests/headers/headerSelection.ts

@@ -325,8 +325,8 @@ const purchaseMatches = (
     }
 
     const { product, userType } = purchaseInfo;
-    const productValid = product && testPurchaseInfo?.product.includes(product);
-    const userValid = userType && testPurchaseInfo?.userType.includes(userType);
+    const productValid = testPurchaseInfo?.product.includes(product);
+    const userValid = testPurchaseInfo?.userType.includes(userType);
 
     return productValid && userValid;
 };

src/shared/lib/geolocation.ts

@@ -634,7 +634,7 @@ export const isGWCheckoutUrl = (baseUrl: string): boolean =>
 export const addRegionIdToSupportUrl = (originalUrl: string, countryCode?: string): string => {
     if (countryCode) {
         const supportRegionId = countryCodeToSupportRegionId(countryCode);
-        if (supportRegionId && !isGWCheckoutUrl(originalUrl)) {
+        if (!isGWCheckoutUrl(originalUrl)) {
             return originalUrl.replace(
                 /(support\.theguardian\.com)\/(contribute-in-epic|contribute|subscribe|checkout)/,
                 (_, domain, path) => `${domain}/${supportRegionId.toLowerCase()}/${path}`,