implement PR Feedback

bitmovin · May 14, 2024 · f877515 · f877515
1 parent b970808
commit f877515
Show file tree

Hide file tree

Showing 4 changed files with 55 additions and 37 deletions.
diff --git a/bitmovin-analytics-datasource/src/components/FilterInput.tsx b/bitmovin-analytics-datasource/src/components/FilterInput.tsx
@@ -15,7 +15,7 @@ type Props = {
   readonly onDelete: () => void;
   readonly addFilterDisabled: boolean;
   readonly onAddFilter: () => void;
-  readonly parsingValueError?: string;
+  readonly parsingValueError: string | undefined;
 };
 
 export function FilterInput(props: Props) {

diff --git a/bitmovin-analytics-datasource/src/components/FilterRow.tsx b/bitmovin-analytics-datasource/src/components/FilterRow.tsx
@@ -9,11 +9,6 @@ import { QueryAttribute, SELECTABLE_QUERY_ATTRIBUTES } from '../types/queryAttri
 import { FilterInput } from './FilterInput';
 import { convertFilterValueToProperType } from '../utils/filterUtils';
 
-type Props = {
-  readonly isAdAnalytics: boolean;
-  readonly onChange: (newFilters: QueryFilter[]) => void;
-};
-
 type Filter = {
   selectedAttribute: SelectableValue<QueryAdAttribute | QueryAttribute>;
   selectedOperator: SelectableValue<QueryFilterOperator>;
@@ -22,27 +17,35 @@ type Filter = {
   parsingValueError: string;
 };
 
-export function FilterRow(props: Props) {
-  const [filters, setFilters] = useState<Filter[]>([]);
+const mapFilterAttributesToSelectableValue = (
+  filters: Filter[],
+  isAdAnalytics: boolean
+): Array<SelectableValue<QueryAttribute | QueryAdAttribute>> => {
+  const selectedAttributes = filters.map((filter) => filter.selectedAttribute);
+  if (isAdAnalytics) {
+    return difference(SELECTABLE_QUERY_AD_ATTRIBUTES, selectedAttributes);
+  } else {
+    return difference(SELECTABLE_QUERY_ATTRIBUTES, selectedAttributes);
+  }
+};
 
-  const mapFilterAttributesToSelectableValue = (): Array<SelectableValue<QueryAttribute | QueryAdAttribute>> => {
-    const selectedAttributes = filters.map((filter) => filter.selectedAttribute);
-    if (props.isAdAnalytics) {
-      return difference(SELECTABLE_QUERY_AD_ATTRIBUTES, selectedAttributes);
-    } else {
-      return difference(SELECTABLE_QUERY_ATTRIBUTES, selectedAttributes);
-    }
-  };
+const mapFiltersToQueryFilters = (filters: Filter[]): QueryFilter[] => {
+  return filters.map((filter) => {
+    return {
+      name: filter.selectedAttribute.value!,
+      operator: filter.selectedOperator.value!,
+      value: filter.convertedFilterValue,
+    } as QueryFilter;
+  });
+};
 
-  const mapFiltersToQueryFilters = (filters: Filter[]): QueryFilter[] => {
-    return filters.map((filter) => {
-      return {
-        name: filter.selectedAttribute.value!,
-        operator: filter.selectedOperator.value!,
-        value: filter.convertedFilterValue,
-      } as QueryFilter;
-    });
-  };
+type Props = {
+  readonly isAdAnalytics: boolean;
+  readonly onChange: (newFilters: QueryFilter[]) => void;
+};
+
+export function FilterRow(props: Props) {
+  const [filters, setFilters] = useState<Filter[]>([]);
 
   const addFilterInput = () => {
     setFilters((prevState) => [
@@ -76,14 +79,16 @@ export function FilterRow(props: Props) {
       setFilters(newFilters);
 
       props.onChange(mapFiltersToQueryFilters(newFilters));
-    } catch (e: any) {
-      const errorMessage = e.message;
-      const newFilter = { ...filter, parsingValueError: errorMessage } as Filter;
+    } catch (e: unknown) {
+      if (e instanceof Error) {
+        const errorMessage = e.message;
+        const newFilter = { ...filter, parsingValueError: errorMessage } as Filter;
 
-      const newFilters = [...filters];
-      newFilters.splice(index, 1, newFilter);
+        const newFilters = [...filters];
+        newFilters.splice(index, 1, newFilter);
 
-      setFilters(newFilters);
+        setFilters(newFilters);
+      }
     }
   };
 
@@ -138,11 +143,11 @@ export function FilterRow(props: Props) {
           </InlineLabel>
         </HorizontalGroup>
       )}
-      {filters.map((filter, index) => (
+      {filters.map((filter, index, filtersArray) => (
         <FilterInput
           key={index}
           isAdAnalytics={props.isAdAnalytics}
-          selectableFilterAttributes={mapFilterAttributesToSelectableValue()}
+          selectableFilterAttributes={mapFilterAttributesToSelectableValue(filtersArray, props.isAdAnalytics)}
           onAttributeChange={(newValue: SelectableValue<QueryAdAttribute | QueryAttribute>) =>
             onAttributesChange(index, newValue)
           }
@@ -151,7 +156,7 @@ export function FilterRow(props: Props) {
           onDelete={() => deleteFilterInput(index)}
           addFilterDisabled={isEmpty(filter.selectedAttribute) || isEmpty(filter.selectedOperator)}
           onAddFilter={() => onAddFilter(index)}
-          parsingValueError={filter.parsingValueError === '' ? undefined : filter.parsingValueError}
+          parsingValueError={isEmpty(filter.parsingValueError) ? undefined : filter.parsingValueError}
         />
       ))}
 

diff --git a/bitmovin-analytics-datasource/src/utils/filterUtils.ts b/bitmovin-analytics-datasource/src/utils/filterUtils.ts
@@ -1,8 +1,10 @@
+import { isEmpty } from 'lodash';
+
 import { QUERY_AD_ATTRIBUTES, QueryAdAttribute } from '../types/queryAdAttributes';
 import { QUERY_FILTER_OPERATORS, QueryFilterOperator, QueryFilterValue } from '../types/queryFilter';
 import { QUERY_ATTRIBUTES, QueryAttribute } from '../types/queryAttributes';
 
-export const isNullFilter = (filterAttribute: QueryAttribute | QueryAdAttribute): boolean => {
+const isNullFilter = (filterAttribute: QueryAttribute | QueryAdAttribute): boolean => {
   switch (filterAttribute) {
     case QUERY_ATTRIBUTES.CDN_PROVIDER:
     case QUERY_ATTRIBUTES.CUSTOM_DATA_1:
@@ -158,14 +160,24 @@ const convertFilter = (rawValue: string, filterAttribute: QueryAttribute, filter
   }
 };
 
+/**
+ * Transforms the string filter Value from the UI to the appropriate type for our API.
+ *
+ * @param {string} rawValue The raw string value from the Filter Input.
+ * @param {QueryAttribute | QueryAdAttribute} filterAttribute The filter attribute.
+ * @param {string} filterAttributeLabel The filter attribute label.
+ * @param {QueryFilterOperator} filterOperator The filter operator.
+ * @param {boolean} isAdAnalytics If Ad Analytics are queried.
+ * @returns {QueryFilterValue} The correctly converted Filter Value.
+ * */
 export const convertFilterValueToProperType = (
   rawValue: string,
   filterAttribute: QueryAttribute | QueryAdAttribute,
   filterAttributeLabel: string,
   filterOperator: QueryFilterOperator,
   isAdAnalytics: boolean
 ): QueryFilterValue => {
-  if (rawValue === '' && isNullFilter(filterAttribute)) {
+  if (isEmpty(rawValue) && isNullFilter(filterAttribute)) {
     return null;
   }
 

diff --git a/tsconfig.json b/tsconfig.json
@@ -4,7 +4,8 @@
     "target": "es5",
     "sourceMap": true,
     "rootDir": "src",
-    "outDir": "dist"
+    "outDir": "dist",
+    "strict": true
   },
   "include": ["src/**/*"]
 }