Repository: netbox-community/netbox
Pull request: Raises validation error if file path and root are not unique
<!--
    Thank you for your interest in contributing to NetBox! Please note that
    our contribution policy requires that a feature request or bug report be
    approved and assigned prior to opening a pull request. This helps avoid
    waste time and effort on a proposed change that we might not be able to
    accept.

    IF YOUR PULL REQUEST DOES NOT REFERENCE AN ISSUE WHICH HAS BEEN ASSIGNED
    TO YOU, IT WILL BE CLOSED AUTOMATICALLY.

    Please specify your assigned issue number on the line below.
-->
### Fixes: #14187

<!--
    Please include a summary of the proposed changes below.
-->


Problem statement:
attempting to add a script from a datasource that already exists throws a server error
### NetBox version

v3.6.4

### Python version

3.11

### Steps to Reproduce

1: create a datasource (eg local) with a script file in it
2: add the script (extras/scripts/add/)
3: attempt to add the same script again

### Expected Behavior

a validation error should show up (or the file should not be available in the dropdown list as its already been added)

### Observed Behavior

django server error is thrown

```
<class 'django.db.utils.IntegrityError'>

duplicate key value violates unique constraint "core_managedfile_unique_root_path"
DETAIL:  Key (file_root, file_path)=(scripts, ip_grabber.py) already exists.

Python version: 3.11.4
NetBox version: 3.6.4
Plugins: 
  netbox_circuitmaintenance: 0.3.0
  netbox_gateways: 0.4.3
  ``

Hints from issue discussion:

Commit message: raises validation error if file path and root are not unique #14187

Patch to review:
diff --git a/netbox/extras/models/scripts.py b/netbox/extras/models/scripts.py
index 122f56f2065..443dabdb339 100644
--- a/netbox/extras/models/scripts.py
+++ b/netbox/extras/models/scripts.py
@@ -2,6 +2,7 @@
 import logging
 from functools import cached_property
 
+from django.core.exceptions import ValidationError
 from django.db import models
 from django.urls import reverse
 from django.utils.translation import gettext_lazy as _
@@ -76,6 +77,14 @@ def _get_name(cls):
 
         return scripts
 
+    def clean(self):
+        super().clean()
+
+        # Ensure that the file root and path make a unique pair
+        if self.file_root and self.file_path:
+            if ScriptModule.objects.filter(file_root=self.file_root, file_path=self.file_path).exclude(pk=self.pk).exists():
+                raise ValidationError(f"A script module with this file path already exists ({self.file_root}/{self.file_path}).")
+
     def save(self, *args, **kwargs):
         self.file_root = ManagedFileRootPathChoices.SCRIPTS
         return super().save(*args, **kwargs)


Code review comments:
diff_hunk: @@ -76,6 +77,14 @@ def _get_name(cls):
 
         return scripts
 
+    def clean(self):
+        super().clean()
+
+        # Ensure that the file root and path make a unique pair
+        if self.file_root and self.file_path:
+            if ScriptModule.objects.filter(file_root=self.file_root, file_path=self.file_path).exclude(pk=self.pk).exists():
path: netbox/extras/models/scripts.py
text: @user1:
```suggestion
            if self._meta.model.objects.filter(file_root=self.file_root, file_path=self.file_path).exclude(pk=self.pk).exists():
```

diff_hunk: @@ -76,6 +77,14 @@ def _get_name(cls):
 
         return scripts
 
+    def clean(self):
path: netbox/extras/models/scripts.py
text: @user1:
This should probably go on the ManagedFile class. We can tweak the method to reference the inheriting class itself so that there's no overlap between scripts and reports, for instance.