Updates to part revision support (#11670)

* Update revision validation

* Refactor UI display

* Fix for usePartFields

* Rearrange part settings

* Better visuals

* Update docs

* use 'full_name' field

* Update playwright tests

* Adjust unit test

* Fix playwright tests

docs/docs/part/revision.md

@@ -27,24 +27,22 @@ When creating a new revision of a part, there are some restrictions which must b
 
 * **Circular References**: A part cannot be a revision of itself. This would create a circular reference which is not allowed.
 * **Unique Revisions**: A part cannot have two revisions with the same revision number. Each revision (of a given part) must have a unique revision code.
-* **Revisions of Revisions**: A single part can have multiple revisions, but a revision cannot have its own revision. This restriction is in place to prevent overly complex part relationships.
 * **Template Revisions**: A part which is a [template part](./template.md) cannot have revisions. This is because the template part is used to create variants, and allowing revisions of templates would create disallowed relationship states in the database. However, variant parts are allowed to have revisions.
 * **Template References**: A part which is a revision of a variant part must point to the same template as the original part. This is to ensure that the revision is correctly linked to the original part.
 
 ## Revision Settings
 
-The following options are available to control the behavior of part revisions.
+The following [global settings](../settings/global.md) are available to control the behavior of part revisions:
 
-Note that these options can be changed in the InvenTree settings:
+| Name | Description | Default | Units |
+| ---- | ----------- | ------- | ----- |
+{{ globalsetting("PART_ENABLE_REVISION") }}
+{{ globalsetting("PART_REVISION_ASSEMBLY_ONLY") }}
 
-{{ image("part/part_revision_settings.png", "Part revision settings") }}
-
-* **Enable Revisions**: If this setting is enabled, parts can have revisions. If this setting is disabled, parts cannot have revisions.
-* **Assembly Revisions Only**: If this setting is enabled, only assembly parts can have revisions. This is useful if you only want to track revisions of assemblies, and not individual parts.
 
 ## Create a Revision
 
-To create a new revision for a given part, navigate to the part detail page, and click on the "Revisions" tab.
+To create a new revision for a given part, navigate to the part detail page, and click on the part actions menu (three vertical dots on the top right of the page).
 
 Select the "Duplicate Part" action, to create a new copy of the selected part. This will open the "Duplicate Part" form:
 
@@ -67,4 +65,5 @@ When multiple revisions exist for a particular part, you can navigate between re
 
 {{ image("part/part_revision_select.png", "Select part revision") }}
 
-Note that this revision selector is only visible when multiple revisions exist for the part.
+!!! info "Revision Selector Visibility"
+    Note that this revision selector is only visible when multiple revisions exist for the part.

src/backend/InvenTree/part/models.py

@@ -776,18 +776,12 @@ class Part(
                     'revision_of': _('Part cannot be a revision of itself')
                 })
 
-            # Part cannot be a revision of a part which is itself a revision
-            if self.revision_of.revision_of:
-                raise ValidationError({
-                    'revision_of': _(
-                        'Cannot make a revision of a part which is already a revision'
-                    )
-                })
-
             # If this part is a revision, it must have a revision code
             if not self.revision:
                 raise ValidationError({
-                    'revision': _('Revision code must be specified')
+                    'revision': _(
+                        'Revision code must be specified for a part marked as a revision'
+                    )
                 })
 
             if get_global_setting('PART_REVISION_ASSEMBLY_ONLY'):

src/backend/InvenTree/part/test_part.py

@@ -412,15 +412,11 @@ class PartTest(TestCase):
             name='Master Part', description='Master part (revision B)'
         )
 
-        with self.assertRaises(ValidationError) as exc:
+        # Ensure we can make a revision of a revision
-            rev_b.revision_of = rev_a
+        rev_b.revision_of = rev_a
-            rev_b.revision = 'B'
+        rev_b.variant_of = template
-            rev_b.save()
+        rev_b.revision = 'B'
-
+        rev_b.save()
-        self.assertIn(
-            'Cannot make a revision of a part which is already a revision',
-            str(exc.exception),
-        )
 
         rev_b.variant_of = template
         rev_b.revision_of = part

src/frontend/src/forms/PartForms.tsx

@@ -16,8 +16,6 @@ export function usePartFields({
   duplicatePartInstance?: any;
   create?: boolean;
 }): ApiFormFieldSet {
-  const settings = useGlobalSettingsState();
-
   const globalSettings = useGlobalSettingsState();
 
   const [virtual, setVirtual] = useState<boolean | undefined>(undefined);
@@ -38,8 +36,10 @@ export function usePartFields({
       revision: {},
       revision_of: {
         filters: {
-          is_revision: false,
+          is_template: false,
-          is_template: false
+          assembly: globalSettings.isSet('PART_REVISION_ASSEMBLY_ONLY')
+            ? true
+            : undefined
         }
       },
       variant_of: {
@@ -155,14 +155,14 @@ export function usePartFields({
             value: true
           },
           copy_bom: {
-            value: settings.isSet('PART_COPY_BOM'),
+            value: globalSettings.isSet('PART_COPY_BOM'),
             hidden: !duplicatePartInstance?.assembly
           },
           copy_notes: {
             value: true
           },
           copy_parameters: {
-            value: settings.isSet('PART_COPY_PARAMETERS')
+            value: globalSettings.isSet('PART_COPY_PARAMETERS')
           },
           copy_tests: {
             value: true,
@@ -172,18 +172,18 @@ export function usePartFields({
       };
     }
 
-    if (settings.isSet('PART_REVISION_ASSEMBLY_ONLY')) {
+    if (globalSettings.isSet('PART_REVISION_ASSEMBLY_ONLY')) {
       fields.revision_of.filters['assembly'] = true;
     }
 
     // Pop 'revision' field if PART_ENABLE_REVISION is False
-    if (!settings.isSet('PART_ENABLE_REVISION')) {
+    if (!globalSettings.isSet('PART_ENABLE_REVISION')) {
       delete fields['revision'];
       delete fields['revision_of'];
     }
 
     // Pop 'expiry' field if expiry not enabled
-    if (!settings.isSet('STOCK_ENABLE_EXPIRY')) {
+    if (!globalSettings.isSet('STOCK_ENABLE_EXPIRY')) {
       delete fields['default_expiry'];
     }
 
@@ -198,8 +198,7 @@ export function usePartFields({
     purchaseable,
     create,
     globalSettings,
-    duplicatePartInstance,
+    duplicatePartInstance
-    settings
   ]);
 }
 

src/frontend/src/pages/Index/Settings/SystemSettings.tsx

@@ -199,13 +199,13 @@ export default function SystemSettings() {
         content: (
           <GlobalSettingList
             keys={[
+              'PART_NAME_FORMAT',
               'PART_IPN_REGEX',
               'PART_ALLOW_DUPLICATE_IPN',
               'PART_ALLOW_EDIT_IPN',
               'PART_ALLOW_DELETE_FROM_ASSEMBLY',
               'PART_ENABLE_REVISION',
               'PART_REVISION_ASSEMBLY_ONLY',
-              'PART_NAME_FORMAT',
               'PART_SHOW_RELATED',
               'PART_CREATE_INITIAL',
               'PART_CREATE_SUPPLIER',

src/frontend/src/pages/part/PartDetail.tsx

@@ -8,6 +8,7 @@ import {
   HoverCard,
   Loader,
   type MantineColor,
+  Paper,
   Skeleton,
   Stack,
   Text
@@ -325,55 +326,24 @@ export default function PartDetail() {
       refetchOnMount: true
     });
 
-  // Fetch information on part revision
+  const revisionsEnabled = useMemo(
+    () => globalSettings.isSet('PART_ENABLE_REVISION'),
+    [globalSettings]
+  );
+
+  // Fetch information on parts which are revisions of *this* part
   const partRevisionQuery = useQuery({
     refetchOnMount: true,
-    queryKey: [
+    enabled: revisionsEnabled && !!part && !!part.revision_count,
-      'part_revisions',
+    queryKey: ['part_revisions', part.pk, part.revision_count],
-      part.pk,
+    queryFn: async () =>
-      part.revision_of,
+      api
-      part.revision_count
+        .get(apiUrl(ApiEndpoints.part_list), {
-    ],
-    queryFn: async () => {
-      if (!part.revision_of && !part.revision_count) {
-        return [];
-      }
-
-      const revisions = [];
-
-      // First, fetch information for the top-level part
-      if (part.revision_of) {
-        await api
-          .get(apiUrl(ApiEndpoints.part_list, part.revision_of))
-          .then((response) => {
-            revisions.push(response.data);
-          });
-      } else {
-        revisions.push(part);
-      }
-
-      const url = apiUrl(ApiEndpoints.part_list);
-
-      await api
-        .get(url, {
           params: {
-            revision_of: part.revision_of || part.pk
+            revision_of: part.pk
           }
         })
-        .then((response) => {
+        .then((response) => response.data)
-          switch (response.status) {
-            case 200:
-              response.data.forEach((r: any) => {
-                revisions.push(r);
-              });
-              break;
-            default:
-              break;
-          }
-        });
-
-      return revisions;
-    }
   });
 
   const partRevisionOptions: any[] = useMemo(() => {
@@ -393,26 +363,14 @@ export default function PartDetail() {
       };
     });
 
-    // Add this part if not already available
-    if (!options.find((o) => o.value == part.pk)) {
-      options.push({
-        value: part.pk,
-        label: part.full_name,
-        part: part
-      });
-    }
-
     return options.sort((a, b) => {
       return `${a.part.revision}`.localeCompare(b.part.revision);
     });
   }, [part, partRevisionQuery.isFetching, partRevisionQuery.data]);
 
   const enableRevisionSelection: boolean = useMemo(() => {
-    return (
+    return partRevisionOptions.length > 0 && revisionsEnabled;
-      partRevisionOptions.length > 0 &&
+  }, [partRevisionOptions, revisionsEnabled]);
-      globalSettings.isSet('PART_ENABLE_REVISION')
-    );
-  }, [partRevisionOptions, globalSettings]);
 
   const detailsPanel = useMemo(() => {
     if (instanceQuery.isFetching) {
@@ -482,6 +440,7 @@ export default function PartDetail() {
         name: 'variant_of',
         label: t`Variant of`,
         model: ModelType.part,
+        model_field: 'full_name',
         hidden: !part.variant_of
       },
       {
@@ -489,6 +448,7 @@ export default function PartDetail() {
         name: 'revision_of',
         label: t`Revision of`,
         model: ModelType.part,
+        model_field: 'full_name',
         hidden: !part.revision_of
       },
       {
@@ -763,10 +723,17 @@ export default function PartDetail() {
             </Grid.Col>
           </Grid>
           {enableRevisionSelection && (
-            <Stack gap='xs'>
+            <Paper p='sm' withBorder>
-              <Text>{t`Select Part Revision`}</Text>
+              <Stack gap='xs'>
-              <RevisionSelector part={part} options={partRevisionOptions} />
+                <Group gap='xs'>
-            </Stack>
+                  <ActionIcon variant='transparent'>
+                    <IconVersions />
+                  </ActionIcon>
+                  <Text>{t`Select Part Revision`}</Text>
+                </Group>
+                <RevisionSelector part={part} options={partRevisionOptions} />
+              </Stack>
+            </Paper>
           )}
         </Stack>
         <DetailsTable fields={tr} item={data} />

src/frontend/tests/pages/pui_part.spec.ts

@@ -745,13 +745,12 @@ test('Parts - Parameter Filtering', async ({ browser }) => {
   await clearTableFilters(page);
 
   // All parts should be available (no filters applied)
-  await page.getByText(/\/ 42\d/).waitFor();
+  await page.getByText(/\/ 43\d/).waitFor();
 
   const clearParamFilter = async (name: string) => {
     await clickOnParamFilter(page, name);
     await page.getByLabel(`clear-filter-${name}`).waitFor();
     await page.getByLabel(`clear-filter-${name}`).click();
-    // await page.getByLabel(`clear-filter-${name}`).click();
   };
 
   // Let's filter by color
@@ -764,7 +763,7 @@ test('Parts - Parameter Filtering', async ({ browser }) => {
   // Reset the filter
   await clearParamFilter('Color');
 
-  await page.getByText(/\/ 42\d/).waitFor();
+  await page.getByText(/\/ 43\d/).waitFor();
 });
 
 test('Parts - Test Results', async ({ browser }) => {
@@ -804,20 +803,26 @@ test('Parts - 404', async ({ browser }) => {
   await page.evaluate(() => console.clear());
 });
 
-test('Parts - Revision', async ({ browser }) => {
+test('Parts - Revisions', async ({ browser }) => {
-  const page = await doCachedLogin(browser, { url: 'part/906/details' });
+  const page = await doCachedLogin(browser, { url: 'part/917/details' });
 
-  await page.getByText('Revision of').waitFor();
+  await page.getByText('ENCAB | Encabulator | C').first().waitFor();
   await page.getByText('Select Part Revision').waitFor();
+
+  // Link to the "revision_of" part
+  await page.getByRole('cell', { name: 'ENCAB | Encabulator | B' }).waitFor();
+
+  // Select a revision
+  await page.getByText('ENCAB | Encabulator | CNo').click();
   await page
-    .getByText('Green Round Table (revision B) | B', { exact: true })
+    .getByRole('option', {
-    .click();
+      name: 'Thumbnail ENCAB | Encabulator | C4 No stock'
-  await page
+    })
-    .getByRole('option', { name: 'Thumbnail Green Round Table No stock' })
     .click();
 
-  await page.waitForURL('**/web/part/101/**');
+  await page.waitForURL('**/web/part/920/**');
-  await page.getByText('Select Part Revision').waitFor();
+  await page.getByText('Part: ENCAB | Encabulator | C4').first().waitFor();
+  await page.getByRole('link', { name: 'ENCAB | Encabulator | C' }).waitFor();
 });
 
 test('Parts - Bulk Edit', async ({ browser }) => {