From 8c10b98fe87321c82ce17fbdb6777c2ef4589585 Mon Sep 17 00:00:00 2001
From: Oliver <oliver.henry.walters@gmail.com>
Date: Wed, 18 Oct 2023 00:41:05 +1100
Subject: [PATCH] [React] Update part parameters table (#5731)

* Implement simple "PartVariantTable" component

- Not yet nested
- More work needed for table nesting

* Fix issue rendering same image multiple times

- Use useId hook to generate random key

* Update PartParameter list API endpoint

- Allow part_detail extra field
- Add FilterSet class
- Allow filter to include variants

* Update PartParameterTable

- Display part column
- Allow returned parts to include templates of base part
- Hide actions for templated parameters

* Fix some code smells
---
 InvenTree/part/api.py                         |  46 ++++++-
 InvenTree/part/serializers.py                 |  71 +++++-----
 .../src/components/images/ApiImage.tsx        |   9 +-
 .../src/components/images/Thumbnail.tsx       |   2 +-
 .../tables/part/PartParameterTable.tsx        | 129 ++++++++++++------
 .../src/components/tables/part/PartTable.tsx  |  34 +----
 .../tables/part/PartVariantTable.tsx          |  18 +++
 .../tables/stock/StockItemTable.tsx           |   2 +-
 src/frontend/src/functions/tables.tsx         |   3 +-
 src/frontend/src/pages/part/PartDetail.tsx    |   6 +-
 10 files changed, 199 insertions(+), 121 deletions(-)
 create mode 100644 src/frontend/src/components/tables/part/PartVariantTable.tsx

diff --git a/InvenTree/part/api.py b/InvenTree/part/api.py
index cd3c18686f..d24494dfec 100644
--- a/InvenTree/part/api.py
+++ b/InvenTree/part/api.py
@@ -1475,14 +1475,22 @@ class PartParameterAPIMixin:
     queryset = PartParameter.objects.all()
     serializer_class = part_serializers.PartParameterSerializer
 
+    def get_queryset(self, *args, **kwargs):
+        """Override get_queryset method to prefetch related fields"""
+        queryset = super().get_queryset(*args, **kwargs)
+        queryset = queryset.prefetch_related('part', 'template')
+        return queryset
+
     def get_serializer(self, *args, **kwargs):
         """Return the serializer instance for this API endpoint.
 
         If requested, extra detail fields are annotated to the queryset:
+        - part_detail
         - template_detail
         """
 
         try:
+            kwargs['part_detail'] = str2bool(self.request.GET.get('part_detail', False))
             kwargs['template_detail'] = str2bool(self.request.GET.get('template_detail', True))
         except AttributeError:
             pass
@@ -1490,6 +1498,35 @@ class PartParameterAPIMixin:
         return self.serializer_class(*args, **kwargs)
 
 
+class PartParameterFilter(rest_filters.FilterSet):
+    """Custom filters for the PartParameterList API endpoint"""
+
+    class Meta:
+        """Metaclass options for the filterset"""
+        model = PartParameter
+        fields = [
+            'template'
+        ]
+
+    part = rest_filters.ModelChoiceFilter(queryset=Part.objects.all(), method='filter_part')
+
+    def filter_part(self, queryset, name, part):
+        """Filter against the provided part.
+
+        If 'include_variants' query parameter is provided, filter against variant parts also
+        """
+
+        try:
+            include_variants = str2bool(self.request.GET.get('include_variants', False))
+        except AttributeError:
+            include_variants = False
+
+        if include_variants:
+            return queryset.filter(part__in=part.get_descendants(include_self=True))
+        else:
+            return queryset.filter(part=part)
+
+
 class PartParameterList(PartParameterAPIMixin, ListCreateAPI):
     """API endpoint for accessing a list of PartParameter objects.
 
@@ -1497,11 +1534,15 @@ class PartParameterList(PartParameterAPIMixin, ListCreateAPI):
     - POST: Create a new PartParameter object
     """
 
+    filterset_class = PartParameterFilter
+
     filter_backends = SEARCH_ORDER_FILTER_ALIAS
 
     ordering_fields = [
         'name',
         'data',
+        'part',
+        'template',
     ]
 
     ordering_field_aliases = {
@@ -1516,11 +1557,6 @@ class PartParameterList(PartParameterAPIMixin, ListCreateAPI):
         'template__units',
     ]
 
-    filterset_fields = [
-        'part',
-        'template',
-    ]
-
 
 class PartParameterDetail(PartParameterAPIMixin, RetrieveUpdateDestroyAPI):
     """API endpoint for detail view of a single PartParameter object."""
diff --git a/InvenTree/part/serializers.py b/InvenTree/part/serializers.py
index 7b38827ad6..2e65df205f 100644
--- a/InvenTree/part/serializers.py
+++ b/InvenTree/part/serializers.py
@@ -240,37 +240,6 @@ class PartParameterTemplateSerializer(InvenTree.serializers.InvenTreeModelSerial
         ]
 
 
-class PartParameterSerializer(InvenTree.serializers.InvenTreeModelSerializer):
-    """JSON serializers for the PartParameter model."""
-
-    class Meta:
-        """Metaclass defining serializer fields"""
-        model = PartParameter
-        fields = [
-            'pk',
-            'part',
-            'template',
-            'template_detail',
-            'data',
-            'data_numeric',
-        ]
-
-    def __init__(self, *args, **kwargs):
-        """Custom initialization method for the serializer.
-
-        Allows us to optionally include or exclude particular information
-        """
-
-        template_detail = kwargs.pop('template_detail', True)
-
-        super().__init__(*args, **kwargs)
-
-        if not template_detail:
-            self.fields.pop('template_detail')
-
-    template_detail = PartParameterTemplateSerializer(source='template', many=False, read_only=True)
-
-
 class PartBriefSerializer(InvenTree.serializers.InvenTreeModelSerializer):
     """Serializer for Part (brief detail)"""
 
@@ -321,6 +290,43 @@ class PartBriefSerializer(InvenTree.serializers.InvenTreeModelSerializer):
     pricing_max = InvenTree.serializers.InvenTreeMoneySerializer(source='pricing_data.overall_max', allow_null=True, read_only=True)
 
 
+class PartParameterSerializer(InvenTree.serializers.InvenTreeModelSerializer):
+    """JSON serializers for the PartParameter model."""
+
+    class Meta:
+        """Metaclass defining serializer fields"""
+        model = PartParameter
+        fields = [
+            'pk',
+            'part',
+            'part_detail',
+            'template',
+            'template_detail',
+            'data',
+            'data_numeric',
+        ]
+
+    def __init__(self, *args, **kwargs):
+        """Custom initialization method for the serializer.
+
+        Allows us to optionally include or exclude particular information
+        """
+
+        template_detail = kwargs.pop('template_detail', True)
+        part_detail = kwargs.pop('part_detail', False)
+
+        super().__init__(*args, **kwargs)
+
+        if not part_detail:
+            self.fields.pop('part_detail')
+
+        if not template_detail:
+            self.fields.pop('template_detail')
+
+    part_detail = PartBriefSerializer(source='part', many=False, read_only=True)
+    template_detail = PartParameterTemplateSerializer(source='template', many=False, read_only=True)
+
+
 class PartSetCategorySerializer(serializers.Serializer):
     """Serializer for changing PartCategory for multiple Part objects"""
 
@@ -1472,7 +1478,8 @@ class BomImportExtractSerializer(InvenTree.serializers.DataFileExtractSerializer
             # At least one part column is required!
             raise serializers.ValidationError(_("No part column specified"))
 
-    def process_row(self, row):
+    @staticmethod
+    def process_row(row):
         """Process a single row from the loaded BOM file"""
         # Skip any rows which are at a lower "level"
         level = row.get('level', None)
diff --git a/src/frontend/src/components/images/ApiImage.tsx b/src/frontend/src/components/images/ApiImage.tsx
index c82c3f6c62..1b5ca4dd92 100644
--- a/src/frontend/src/components/images/ApiImage.tsx
+++ b/src/frontend/src/components/images/ApiImage.tsx
@@ -11,8 +11,9 @@ import {
   Overlay,
   Stack
 } from '@mantine/core';
+import { useId } from '@mantine/hooks';
 import { useQuery } from '@tanstack/react-query';
-import { useEffect, useState } from 'react';
+import { useState } from 'react';
 
 import { api } from '../../App';
 
@@ -22,8 +23,10 @@ import { api } from '../../App';
 export function ApiImage(props: ImageProps) {
   const [image, setImage] = useState<string>('');
 
+  const queryKey = useId();
+
   const imgQuery = useQuery({
-    queryKey: ['image', props.src],
+    queryKey: ['image', queryKey, props.src],
     enabled: props.src != undefined && props.src != null && props.src != '',
     queryFn: async () => {
       if (!props.src) {
@@ -47,7 +50,7 @@ export function ApiImage(props: ImageProps) {
         });
     },
     refetchOnMount: true,
-    refetchOnWindowFocus: true
+    refetchOnWindowFocus: false
   });
 
   return (
diff --git a/src/frontend/src/components/images/Thumbnail.tsx b/src/frontend/src/components/images/Thumbnail.tsx
index 9ac12fca5b..2e7524014c 100644
--- a/src/frontend/src/components/images/Thumbnail.tsx
+++ b/src/frontend/src/components/images/Thumbnail.tsx
@@ -11,7 +11,7 @@ export function Thumbnail({
   alt = t`Thumbnail`,
   size = 20
 }: {
-  src: string;
+  src?: string | undefined;
   alt?: string;
   size?: number;
 }) {
diff --git a/src/frontend/src/components/tables/part/PartParameterTable.tsx b/src/frontend/src/components/tables/part/PartParameterTable.tsx
index 541f663ae5..1a346ea6aa 100644
--- a/src/frontend/src/components/tables/part/PartParameterTable.tsx
+++ b/src/frontend/src/components/tables/part/PartParameterTable.tsx
@@ -1,5 +1,5 @@
 import { t } from '@lingui/macro';
-import { ActionIcon, Text, Tooltip } from '@mantine/core';
+import { ActionIcon, Group, Text, Tooltip } from '@mantine/core';
 import { IconTextPlus } from '@tabler/icons-react';
 import { useCallback, useMemo } from 'react';
 
@@ -10,6 +10,7 @@ import {
 } from '../../../functions/forms';
 import { useTableRefresh } from '../../../hooks/TableRefresh';
 import { ApiPaths, apiUrl } from '../../../states/ApiState';
+import { Thumbnail } from '../../images/Thumbnail';
 import { YesNoButton } from '../../items/YesNoButton';
 import { TableColumn } from '../Column';
 import { InvenTreeTable } from '../InvenTreeTable';
@@ -22,12 +23,36 @@ export function PartParameterTable({ partId }: { partId: any }) {
 
   const tableColumns: TableColumn[] = useMemo(() => {
     return [
+      {
+        accessor: 'part',
+        title: t`Part`,
+        switchable: true,
+        sortable: true,
+        render: function (record: any) {
+          let part = record?.part_detail ?? {};
+
+          return (
+            <Group spacing="xs" align="left" noWrap={true}>
+              <Thumbnail
+                src={part?.thumbnail || part?.image}
+                alt={part?.name}
+                size={24}
+              />
+              <Text>{part?.full_name}</Text>
+            </Group>
+          );
+        }
+      },
       {
         accessor: 'name',
         title: t`Parameter`,
         switchable: false,
         sortable: true,
-        render: (record) => record.template_detail?.name
+        render: (record) => {
+          let variant = String(partId) != String(record.part);
+
+          return <Text italic={variant}>{record.template_detail?.name}</Text>;
+        }
       },
       {
         accessor: 'description',
@@ -65,54 +90,62 @@ export function PartParameterTable({ partId }: { partId: any }) {
         render: (record) => record.template_detail?.units
       }
     ];
-  }, []);
+  }, [partId]);
 
   // Callback for row actions
   // TODO: Adjust based on user permissions
-  const rowActions = useCallback((record: any) => {
-    let actions = [];
+  const rowActions = useCallback(
+    (record: any) => {
+      // Actions not allowed for "variant" rows
+      if (String(partId) != String(record.part)) {
+        return [];
+      }
 
-    actions.push({
-      title: t`Edit`,
-      onClick: () => {
-        openEditApiForm({
-          name: 'edit-part-parameter',
-          url: ApiPaths.part_parameter_list,
-          pk: record.pk,
-          title: t`Edit Part Parameter`,
-          fields: {
-            part: {
-              hidden: true
+      let actions = [];
+
+      actions.push({
+        title: t`Edit`,
+        onClick: () => {
+          openEditApiForm({
+            name: 'edit-part-parameter',
+            url: ApiPaths.part_parameter_list,
+            pk: record.pk,
+            title: t`Edit Part Parameter`,
+            fields: {
+              part: {
+                hidden: true
+              },
+              template: {},
+              data: {}
             },
-            template: {},
-            data: {}
-          },
-          successMessage: t`Part parameter updated`,
-          onFormSuccess: refreshTable
-        });
-      }
-    });
+            successMessage: t`Part parameter updated`,
+            onFormSuccess: refreshTable
+          });
+        }
+      });
 
-    actions.push({
-      title: t`Delete`,
-      color: 'red',
-      onClick: () => {
-        openDeleteApiForm({
-          name: 'delete-part-parameter',
-          url: ApiPaths.part_parameter_list,
-          pk: record.pk,
-          title: t`Delete Part Parameter`,
-          successMessage: t`Part parameter deleted`,
-          onFormSuccess: refreshTable,
-          preFormContent: (
-            <Text>{t`Are you sure you want to remove this parameter?`}</Text>
-          )
-        });
-      }
-    });
+      actions.push({
+        title: t`Delete`,
+        color: 'red',
+        onClick: () => {
+          openDeleteApiForm({
+            name: 'delete-part-parameter',
+            url: ApiPaths.part_parameter_list,
+            pk: record.pk,
+            title: t`Delete Part Parameter`,
+            successMessage: t`Part parameter deleted`,
+            onFormSuccess: refreshTable,
+            preFormContent: (
+              <Text>{t`Are you sure you want to remove this parameter?`}</Text>
+            )
+          });
+        }
+      });
 
-    return actions;
-  }, []);
+      return actions;
+    },
+    [partId]
+  );
 
   const addParameter = useCallback(() => {
     if (!partId) {
@@ -160,9 +193,17 @@ export function PartParameterTable({ partId }: { partId: any }) {
       props={{
         rowActions: rowActions,
         customActionGroups: tableActions,
+        customFilters: [
+          {
+            name: 'include_variants',
+            label: t`Include Variants`,
+            type: 'boolean'
+          }
+        ],
         params: {
           part: partId,
-          template_detail: true
+          template_detail: true,
+          part_detail: true
         }
       }}
     />
diff --git a/src/frontend/src/components/tables/part/PartTable.tsx b/src/frontend/src/components/tables/part/PartTable.tsx
index e28dde4db5..14bc6ce90e 100644
--- a/src/frontend/src/components/tables/part/PartTable.tsx
+++ b/src/frontend/src/components/tables/part/PartTable.tsx
@@ -3,8 +3,6 @@ import { Group, Text } from '@mantine/core';
 import { useMemo } from 'react';
 import { useNavigate } from 'react-router-dom';
 
-import { editPart } from '../../../functions/forms/PartForms';
-import { notYetImplemented } from '../../../functions/notifications';
 import { shortenString } from '../../../functions/tables';
 import { useTableRefresh } from '../../../hooks/TableRefresh';
 import { ApiPaths, apiUrl } from '../../../states/ApiState';
@@ -12,7 +10,6 @@ import { Thumbnail } from '../../images/Thumbnail';
 import { TableColumn } from '../Column';
 import { TableFilter } from '../Filter';
 import { InvenTreeTable, InvenTreeTableProps } from '../InvenTreeTable';
-import { RowAction } from '../RowActions';
 
 /**
  * Construct a list of columns for the part table
@@ -193,33 +190,6 @@ export function PartListTable({ props }: { props: InvenTreeTableProps }) {
 
   const { tableKey, refreshTable } = useTableRefresh('part');
 
-  // Callback function for generating set of row actions
-  function partTableRowActions(record: any): RowAction[] {
-    let actions: RowAction[] = [];
-
-    actions.push({
-      title: t`Edit`,
-      onClick: () => {
-        editPart({
-          part_id: record.pk,
-          callback: () => {
-            // TODO: Reload the table, somehow?
-            notYetImplemented();
-          }
-        });
-      }
-    });
-
-    actions.push({
-      title: t`Detail`,
-      onClick: () => {
-        navigate(`/part/${record.pk}/`);
-      }
-    });
-
-    return actions;
-  }
-
   const navigate = useNavigate();
 
   return (
@@ -231,10 +201,12 @@ export function PartListTable({ props }: { props: InvenTreeTableProps }) {
         ...props,
         enableDownload: true,
         customFilters: tableFilters,
-        rowActions: partTableRowActions,
         params: {
           ...props.params,
           category_detail: true
+        },
+        onRowClick: (record, _index, _event) => {
+          navigate(`/part/${record.pk}/`);
         }
       }}
     />
diff --git a/src/frontend/src/components/tables/part/PartVariantTable.tsx b/src/frontend/src/components/tables/part/PartVariantTable.tsx
new file mode 100644
index 0000000000..169711570d
--- /dev/null
+++ b/src/frontend/src/components/tables/part/PartVariantTable.tsx
@@ -0,0 +1,18 @@
+import { PartListTable } from './PartTable';
+
+/**
+ * Display variant parts for thespecified parent part
+ */
+export function PartVariantTable({ partId }: { partId: string }) {
+  return (
+    <PartListTable
+      props={{
+        enableDownload: false,
+        customFilters: [],
+        params: {
+          ancestor: partId
+        }
+      }}
+    />
+  );
+}
diff --git a/src/frontend/src/components/tables/stock/StockItemTable.tsx b/src/frontend/src/components/tables/stock/StockItemTable.tsx
index b802dbfb90..05c8994340 100644
--- a/src/frontend/src/components/tables/stock/StockItemTable.tsx
+++ b/src/frontend/src/components/tables/stock/StockItemTable.tsx
@@ -30,7 +30,7 @@ function stockItemTableColumns(): TableColumn[] {
               alt={part?.name}
               size={24}
             />
-            <Text>{part.full_name}</Text>
+            <Text>{part?.full_name}</Text>
           </Group>
         );
       }
diff --git a/src/frontend/src/functions/tables.tsx b/src/frontend/src/functions/tables.tsx
index a7fbef22a1..73873a7bfa 100644
--- a/src/frontend/src/functions/tables.tsx
+++ b/src/frontend/src/functions/tables.tsx
@@ -7,10 +7,11 @@ export function shortenString({
   str,
   len = 100
 }: {
-  str: string;
+  str: string | undefined;
   len?: number;
 }) {
   // Ensure that the string is a string
+  str = str ?? '';
   str = str.toString();
 
   // If the string is already short enough, return it
diff --git a/src/frontend/src/pages/part/PartDetail.tsx b/src/frontend/src/pages/part/PartDetail.tsx
index 030b07260e..26fee9e6cd 100644
--- a/src/frontend/src/pages/part/PartDetail.tsx
+++ b/src/frontend/src/pages/part/PartDetail.tsx
@@ -33,6 +33,7 @@ import { PageDetail } from '../../components/nav/PageDetail';
 import { PanelGroup, PanelType } from '../../components/nav/PanelGroup';
 import { AttachmentTable } from '../../components/tables/AttachmentTable';
 import { PartParameterTable } from '../../components/tables/part/PartParameterTable';
+import { PartVariantTable } from '../../components/tables/part/PartVariantTable';
 import { RelatedPartTable } from '../../components/tables/part/RelatedPartTable';
 import { StockItemTable } from '../../components/tables/stock/StockItemTable';
 import { NotesEditor } from '../../components/widgets/MarkdownEditor';
@@ -56,8 +57,7 @@ export default function PartDetail() {
     params: {
       path_detail: true
     },
-    refetchOnMount: true,
-    refetchOnWindowFocus: true
+    refetchOnMount: true
   });
 
   // Part data panels (recalculate when part data changes)
@@ -92,7 +92,7 @@ export default function PartDetail() {
         label: t`Variants`,
         icon: <IconVersions />,
         hidden: !part.is_template,
-        content: <PlaceholderPanel />
+        content: <PartVariantTable partId={String(id)} />
       },
       {
         name: 'bom',