Repository: openwisp/openwisp-utils (language: python)

Issue:
[feature/change] Generalize UUIDAdmin
`UUIDAdmin` is a cool concept, but we should generalize it to allow using it for any field.

This issue came up while working on https://github.com/openwisp/openwisp-radius/issues/448 to make the radius token field readonly and easy to copy.

The problem is that the JS which we use now is quite hardcoded towards the ID field:
https://github.com/openwisp/openwisp-utils/blob/master/openwisp_utils/static/openwisp-utils/js/uuid.js
Therefore we need to devise a more generic solution which could allow us to flag a field as readonly and copiable.

We could call this `CopyableFieldsAdmin` or something similar and we could allow to specify which fields have to work like the `uuid` field. Eg:

```python
DeviceAdmin(CopyableFieldsAdmin):
    copiable_fields = ['id']
```

We have to do this change in a backward compatible fashion, that is, other modules shoulnd't break because of this.
A new documentation section shall be added before `openwisp_utils.admin.UUIDAdmin` and we should remove the documentation of `openwisp_utils.admin.UUIDAdmin`, while also flagging this class as deprecated in the code (with comments).

Once this is introduced, we have to update the modules which are using the old `UUIDAdmin` class:

- users
- controller
- notifications
- network-topology

We should then update  also the radius module to change the code introduced here: https://github.com/openwisp/openwisp-radius/pull/451 to use this new solution.


Discussion / hints:

Fix patch:
diff --git a/README.rst b/README.rst
index b4b63d58..bf473f8d 100644
--- a/README.rst
+++ b/README.rst
@@ -937,11 +937,21 @@ is created even if the default values are unchanged.
 Without this, when creating new objects, inline items won't be saved
 unless users change the default values.
 
+``openwisp_utils.admin.CopyableFieldsAdmin``
+^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+An admin class that allows to set admin fields to be
+read-only and makes it easy to copy the fields contents.
+
+Useful for auto-generated fields such as UUIDs, secret keys, tokens, etc.
+
 ``openwisp_utils.admin.UUIDAdmin``
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
-An admin class that provides the UUID of the object as a read-only input field
-(to make it easy and quick to copy/paste).
+This class is a subclass of ``CopyableFieldsAdmin`` which
+sets ``uuid`` as the only copyable field. This class is kept
+for backward compatibility and convenience, since different models
+of various OpenWISP modules show ``uuid`` as the only copyable field.
 
 ``openwisp_utils.admin.ReceiveUrlAdmin``
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
diff --git a/openwisp_utils/admin.py b/openwisp_utils/admin.py
index 144c6efd..192aac7e 100644
--- a/openwisp_utils/admin.py
+++ b/openwisp_utils/admin.py
@@ -1,4 +1,5 @@
 from django.contrib.admin import ModelAdmin, StackedInline
+from django.core.exceptions import FieldError
 from django.urls import reverse
 from django.utils.translation import gettext_lazy as _
 
@@ -69,33 +70,86 @@ def has_changed(self):
         return super().has_changed()
 
 
-class UUIDAdmin(ModelAdmin):
+class CopyableFieldError(FieldError):
+    pass
+
+
+class CopyableFieldsAdmin(ModelAdmin):
     """
-    Defines a field name uuid whose value is that
-    of the id of the object
+    An admin class that allows to set admin
+    fields to be read-only and makes it easy
+    to copy the fields contents.
+    Useful for auto-generated fields such as
+    UUIDs, secret keys, tokens, etc
     """
 
-    def uuid(self, obj):
-        return obj.pk
-
-    def _process_fields(self, fields, request, obj):
-        fields = list(fields)
-        if 'uuid' in fields and not obj:
-            fields.remove('uuid')
-        if 'uuid' not in fields and obj:
-            fields.insert(0, 'uuid')
-        return tuple(fields)
+    copyable_fields = ()
+    change_form_template = 'admin/change_form.html'
+
+    def _check_copyable_subset_fields(self, copyable_fields, fields):
+        if not set(copyable_fields).issubset(fields):
+            class_name = self.__class__.__name__
+            raise CopyableFieldError(
+                (
+                    f'{copyable_fields} not in {class_name}.fields {fields}, '
+                    f'Check copyable_fields attribute of class {class_name}.'
+                )
+            )
 
     def get_fields(self, request, obj=None):
-        fields = super().get_fields(request, obj)
-        return self._process_fields(fields, request, obj)
+        fields = super(ModelAdmin, self).get_fields(request, obj)
+        self._check_copyable_subset_fields(self.copyable_fields, fields)
+        # We should exclude `copyable_fields` fields
+        # when the object doesn't exist for example,
+        # in the case of `add_view` because `copyable_fields`
+        # can't be edited and are auto generated by the system
+        if not obj:
+            return tuple(set(fields).difference(self.copyable_fields))
+        return fields
 
     def get_readonly_fields(self, request, obj=None):
-        fields = super().get_readonly_fields(request, obj)
-        return self._process_fields(fields, request, obj)
+        readonly_fields = super(ModelAdmin, self).get_readonly_fields(request, obj)
+        if not obj:
+            return readonly_fields
+        # Make sure `copyable_fields` is included in `read_only` fields
+        return tuple([*readonly_fields, *self.copyable_fields])
+
+    def add_view(self, request, form_url='', extra_context=None):
+        extra_context = extra_context or {}
+        extra_context['copyable_fields'] = []
+        return super().add_view(
+            request,
+            form_url,
+            extra_context=extra_context,
+        )
+
+    def change_view(self, request, object_id, form_url='', extra_context=None):
+        extra_context = extra_context or {}
+        extra_context['copyable_fields'] = list(self.copyable_fields)
+        return super().change_view(
+            request,
+            object_id,
+            form_url,
+            extra_context=extra_context,
+        )
 
     class Media:
-        js = ('admin/js/jquery.init.js', 'openwisp-utils/js/uuid.js')
+        js = ('admin/js/jquery.init.js', 'openwisp-utils/js/copyable.js')
+
+
+class UUIDAdmin(CopyableFieldsAdmin):
+    """
+    This class is a subclass of `CopyableFieldsAdmin`
+    which sets `uuid` as the only copyable field
+    This class is kept for backward compatibility
+    and convenience, since different models of various
+    OpenWISP modules show `uuid` as the only copyable field
+    """
+
+    copyable_fields = ('uuid',)
+
+    def uuid(self, obj):
+        return obj.pk
 
     uuid.short_description = _('UUID')
 
diff --git a/openwisp_utils/admin_theme/templates/admin/change_form.html b/openwisp_utils/admin_theme/templates/admin/change_form.html
new file mode 100644
index 00000000..2956fee9
--- /dev/null
+++ b/openwisp_utils/admin_theme/templates/admin/change_form.html
@@ -0,0 +1,9 @@
+{% extends "admin/change_form.html" %}
+{% block admin_change_form_document_ready %}
+{{ block.super }}
+{% if copyable_fields %}
+    <script>
+        window.copyableFields = {{ copyable_fields | safe }} || [];
+    </script>
+{% endif %}
+{% endblock %}
diff --git a/openwisp_utils/static/openwisp-utils/js/copyable.js b/openwisp_utils/static/openwisp-utils/js/copyable.js
new file mode 100644
index 00000000..c7832b0f
--- /dev/null
+++ b/openwisp_utils/static/openwisp-utils/js/copyable.js
@@ -0,0 +1,16 @@
+'use strict';
+django.jQuery(function ($) {
+    if (window.copyableFields) {
+        window.copyableFields.forEach(copyableField => {
+            var copyableFieldContainer = $(`.field-${copyableField} .readonly`).eq(0);
+    
+            copyableFieldContainer.html(`<input readonly id="id_${copyableField}" type="text" 
+            class="vTextField readonly" value="${copyableFieldContainer.text()}">`);
+
+            var copyableFieldSelectedId = $(`#id_${copyableField}`);
+            copyableFieldSelectedId.click(function () {
+                $(this).select();
+            });
+        });
+    }
+});
diff --git a/openwisp_utils/static/openwisp-utils/js/uuid.js b/openwisp_utils/static/openwisp-utils/js/uuid.js
deleted file mode 100644
index 7cf5a3b9..00000000
--- a/openwisp_utils/static/openwisp-utils/js/uuid.js
+++ /dev/null
@@ -1,10 +0,0 @@
-'use strict';
-django.jQuery(function ($) {
-    var container = $('.field-uuid .readonly').eq(0),
-        value = container.text();
-    container.html('<input readonly id="id_id" type="text" class="vTextField readonly" value="' + value + '">');
-    var id = $('#id_id');
-    id.click(function () {
-        $(this).select();
-    });
-});


Test patch:
diff --git a/tests/test_project/tests/test_admin.py b/tests/test_project/tests/test_admin.py
index 740cefea..56885232 100644
--- a/tests/test_project/tests/test_admin.py
+++ b/tests/test_project/tests/test_admin.py
@@ -6,7 +6,7 @@
 from django.core.exceptions import ImproperlyConfigured
 from django.test import TestCase
 from django.urls import reverse
-from openwisp_utils.admin import ReadOnlyAdmin
+from openwisp_utils.admin import CopyableFieldError, CopyableFieldsAdmin, ReadOnlyAdmin
 from openwisp_utils.admin_theme import settings as admin_theme_settings
 from openwisp_utils.admin_theme.apps import OpenWispAdminThemeConfig, _staticfy
 from openwisp_utils.admin_theme.checks import admin_theme_settings_checks
@@ -204,6 +204,36 @@ def test_uuid_field_in_add(self):
         self.assertNotContains(response, 'field-uuid')
         self.assertContains(response, 'field-receive_url')
 
+    def test_copyablefields_admin(self):
+        class TestCopyableFieldAdmin(CopyableFieldsAdmin):
+            copyable_fields = ('session_id', 'username')
+
+        options = dict(username='bobby', session_id='1')
+        radius_acc = self._create_radius_accounting(**options)
+        ma = TestCopyableFieldAdmin(RadiusAccounting, AdminSite)
+        path = reverse(
+            'admin:test_project_radiusaccounting_change', args=[radius_acc.pk]
+        )
+        self.assertEqual(
+            ma.get_readonly_fields(self.client.request, radius_acc),
+            TestCopyableFieldAdmin.copyable_fields,
+        )
+        response = self.client.get(path)
+        self.assertEqual(response.status_code, 200)
+        self.assertContains(response, 'field-username')
+        self.assertContains(response, 'field-session_id')
+
+    def test_invalid_copyablefields_admin_error(self):
+        class TestCopyableFieldAdmin(CopyableFieldsAdmin):
+            pass
+
+        ma = TestCopyableFieldAdmin(Project, AdminSite)
+        ma.copyable_fields = ('invalid_field',)
+        copyable_field_err = "('invalid_field',) not in TestCopyableFieldAdmin.fields"
+        with self.assertRaises(CopyableFieldError) as err:
+            ma.get_fields(self.client.request)
+        self.assertIn(copyable_field_err, err.exception.args[0])
+
     def test_receive_url_admin(self):
         p = Project.objects.create(name='test_receive_url_admin_project')
         ma = ProjectAdmin(Project, self.site)