Added owner model to admin page and added test cases

InvenTree/stock/migrations/0057_stock_location_item_owner.py

@@ -20,6 +20,6 @@ class Migration(migrations.Migration):
         migrations.AddField(
             model_name='stocklocation',
             name='owner',
-            field=models.ForeignKey(blank=True, help_text='Select Owner', null=True, on_delete=django.db.models.deletion.CASCADE, related_name='stock_locations', to='users.Owner'),
+            field=models.ForeignKey(blank=True, help_text='Select Owner', null=True, on_delete=django.db.models.deletion.SET_NULL, related_name='stock_locations', to='users.Owner'),
         ),
     ]

InvenTree/stock/models.py

@@ -49,7 +49,7 @@ class StockLocation(InvenTreeTree):
     Stock locations can be heirarchical as required
     """
 
-    owner = models.ForeignKey(Owner, on_delete=models.CASCADE, blank=True, null=True,
+    owner = models.ForeignKey(Owner, on_delete=models.SET_NULL, blank=True, null=True,
                               help_text='Select Owner',
                               related_name='stock_locations')
 

InvenTree/stock/templates/stock/item_base.html

@@ -21,7 +21,7 @@ InvenTree | {% trans "Stock Item" %} - {{ item }}
 
     {% if not user in owners and not user.is_superuser %}
     <div class='alert alert-block alert-info'>
-        {% trans "You are not the owner of this item. This stock item cannot be edited." %}<br>
+        {% trans "You are not in the list of owners of this item. This stock item cannot be edited." %}<br>
     </div>
     {% endif %}
 {% endif %}

InvenTree/stock/views.py

@@ -11,7 +11,8 @@ from django.views.generic import DetailView, ListView, UpdateView
 from django.forms.models import model_to_dict
 from django.forms import HiddenInput
 from django.urls import reverse
-from django.contrib.auth.models import Group, User
+from django.contrib.auth import get_user_model
+from django.contrib.auth.models import Group
 
 from django.utils.translation import ugettext as _
 
@@ -145,28 +146,53 @@ class StockLocationEdit(AjaxUpdateView):
 
         # Is ownership control enabled?
         stock_ownership_control = InvenTreeSetting.get_setting('STOCK_OWNERSHIP_CONTROL')
-        owner = location.owner
 
         if not stock_ownership_control:
+            # Hide owner field
             form.fields['owner'].widget = HiddenInput()
         else:
-            form.fields['owner'].initial = owner
-            if location.parent:
-                form.fields['owner'].initial = location.parent.owner
-            if not self.request.user.is_superuser:
-                form.fields['owner'].disabled = True
+            # Get location's owner
+            location_owner = location.owner
+
+            if location_owner:
+                if location.parent:
+                    try:
+                        # If location has parent and owner: automatically select parent's owner
+                        parent_owner = location.parent.owner
+                        form.fields['owner'].initial = parent_owner
+                    except AttributeError:
+                        pass
+                else:
+                    # If current owner exists: automatically select it
+                    form.fields['owner'].initial = location_owner
+
+                # Update queryset or disable field (only if not admin)
+                if not self.request.user.is_superuser:
+                    if type(location_owner.owner) is Group:
+                        user_as_owner = Owner.get_owner(self.request.user)
+                        queryset = location_owner.get_related_owners(include_group=True)
+
+                        if user_as_owner not in queryset:
+                            # Only owners or admin can change current owner
+                            form.fields['owner'].disabled = True
+                        else:
+                            form.fields['owner'].queryset = queryset
 
         return form
 
     def save(self, object, form, **kwargs):
         """ If location has children and ownership control is enabled:
-            - update all children's owners with location's owner
+            - update owner of all children location of this location
+            - update owner for all stock items at this location
         """
 
         self.object = form.save()
         
+        # Is ownership control enabled?
         stock_ownership_control = InvenTreeSetting.get_setting('STOCK_OWNERSHIP_CONTROL')
+
         if stock_ownership_control:
+            # Get authorized users
             authorized_owners = self.object.owner.get_related_owners()
 
             # Update children locations
@@ -1414,6 +1440,7 @@ class StockItemEdit(AjaxUpdateView):
 
         # Is ownership control enabled?
         stock_ownership_control = InvenTreeSetting.get_setting('STOCK_OWNERSHIP_CONTROL')
+
         if not stock_ownership_control:
             form.fields['owner'].widget = HiddenInput()
         else:
@@ -1426,14 +1453,18 @@ class StockItemEdit(AjaxUpdateView):
             if location_owner:
                 form.fields['owner'].initial = location_owner
 
-                # Check location owner type and filter
+                # Check location's owner type and filter potential owners
                 if type(location_owner.owner) is Group:
                     user_as_owner = Owner.get_owner(self.request.user)
                     queryset = location_owner.get_related_owners(include_group=True)
+
                     if user_as_owner in queryset:
                         form.fields['owner'].initial = user_as_owner
+
                     form.fields['owner'].queryset = queryset
-                elif type(location_owner.owner) is User:
+
+                elif type(location_owner.owner) is get_user_model():
+                    # If location's owner is a user: automatically set owner field and disable it
                     form.fields['owner'].disabled = True
                     form.fields['owner'].initial = location_owner
 
@@ -1446,14 +1477,18 @@ class StockItemEdit(AjaxUpdateView):
             if item_owner:
                 form.fields['owner'].initial = item_owner
 
-                # Check location owner type and filter
+                # Check item's owner type and filter potential owners
                 if type(item_owner.owner) is Group:
                     user_as_owner = Owner.get_owner(self.request.user)
                     queryset = item_owner.get_related_owners(include_group=True)
+
                     if user_as_owner in queryset:
                         form.fields['owner'].initial = user_as_owner
+
                     form.fields['owner'].queryset = queryset
-                elif type(item_owner.owner) is User:
+
+                elif type(item_owner.owner) is get_user_model():
+                    # If item's owner is a user: automatically set owner field and disable it
                     form.fields['owner'].disabled = True
                     form.fields['owner'].initial = item_owner
 
@@ -1533,10 +1568,12 @@ class StockLocationCreate(AjaxCreateView):
 
         # Is ownership control enabled?
         stock_ownership_control = InvenTreeSetting.get_setting('STOCK_OWNERSHIP_CONTROL')
+
         if not stock_ownership_control:
+            # Hide owner field
             form.fields['owner'].widget = HiddenInput()
         else:
-            # If user did not selected owner, automatically match to parent's owner
+            # If user did not selected owner: automatically match to parent's owner
             if not form['owner'].data:
                 try:
                     parent_id = form['parent'].value()
@@ -1806,15 +1843,18 @@ class StockItemCreate(AjaxCreateView):
                 location_owner = None
 
             if location_owner:
-                # Check location owner type and filter
+                # Check location's owner type and filter potential owners
                 if type(location_owner.owner) is Group:
                     user_as_owner = Owner.get_owner(self.request.user)
                     queryset = location_owner.get_related_owners()
                     
                     if user_as_owner in queryset:
                         form.fields['owner'].initial = user_as_owner
+
                     form.fields['owner'].queryset = queryset
-                elif type(location_owner.owner) is User:
+
+                elif type(location_owner.owner) is get_user_model():
+                    # If location's owner is a user: automatically set owner field and disable it
                     form.fields['owner'].disabled = True
                     form.fields['owner'].initial = location_owner
                 

InvenTree/users/admin.py

@@ -11,7 +11,7 @@ from django.contrib.auth.models import Group
 from django.contrib.auth.admin import UserAdmin
 from django.utils.safestring import mark_safe
 
-from users.models import RuleSet
+from users.models import RuleSet, Owner
 
 User = get_user_model()
 
@@ -206,8 +206,17 @@ class InvenTreeUserAdmin(UserAdmin):
     )
 
 
+class OwnerAdmin(admin.ModelAdmin):
+    """
+    Custom admin interface for the Owner model
+    """
+    pass
+
+
 admin.site.unregister(Group)
 admin.site.register(Group, RoleGroupAdmin)
 
 admin.site.unregister(User)
 admin.site.register(User, InvenTreeUserAdmin)
+
+admin.site.register(Owner, OwnerAdmin)

InvenTree/users/apps.py

@@ -39,6 +39,14 @@ class UsersConfig(AppConfig):
 
     def update_owners(self):
 
-        from users.models import create_owner
+        from django.contrib.auth import get_user_model
+        from django.contrib.auth.models import Group
+        from users.models import Owner
 
-        create_owner(full_update=True)
+        # Create group owners
+        for group in Group.objects.all():
+            Owner.create(group)
+
+        # Create user owners
+        for user in get_user_model().objects.all():
+            Owner.create(user)

InvenTree/users/models.py

@@ -1,6 +1,7 @@
 # -*- coding: utf-8 -*-
 
-from django.contrib.auth.models import User, Group, Permission
+from django.contrib.auth import get_user_model
+from django.contrib.auth.models import Group, Permission
 from django.contrib.contenttypes.fields import GenericForeignKey
 from django.contrib.contenttypes.models import ContentType
 from django.db.models import UniqueConstraint, Q
@@ -9,7 +10,7 @@ from django.db import models
 from django.utils.translation import gettext_lazy as _
 
 from django.dispatch import receiver
-from django.db.models.signals import post_save
+from django.db.models.signals import post_save, post_delete
 
 
 class RuleSet(models.Model):
@@ -393,32 +394,42 @@ def check_user_role(user, role, permission):
 
 class Owner(models.Model):
     """
-    An owner is either a group or user.
-    Owner can be associated to any InvenTree model (part, stock, etc.)
+    The Owner class is a proxy for a Group or User instance.
+    Owner can be associated to any InvenTree model (part, stock, build, etc.)
+
+    owner_type: Model type (Group or User)
+    owner_id: Group or User instance primary key
+    owner: Returns the Group or User instance combining the owner_type and owner_id fields
     """
 
     class Meta:
+        # Ensure all owners are unique
         constraints = [
             UniqueConstraint(fields=['owner_type', 'owner_id'],
                              name='unique_owner')
         ]
 
     owner_type = models.ForeignKey(ContentType, on_delete=models.CASCADE, null=True, blank=True)
+
     owner_id = models.PositiveIntegerField(null=True, blank=True)
+
     owner = GenericForeignKey('owner_type', 'owner_id')
 
     def __str__(self):
+        """ Defines the owner string representation """
         return f'{self.owner} ({self.owner_type.name})'
 
     @classmethod
-    def create(cls, owner):
+    def create(cls, obj):
+        """ Check if owner exist then create new owner entry """
 
-        existing_owner = cls.get_owner(owner)
+        # Check for existing owner
+        existing_owner = cls.get_owner(obj)
 
         if not existing_owner:
             # Create new owner
             try:
-                return cls.objects.create(owner=owner)
+                return cls.objects.create(owner=obj)
             except IntegrityError:
                 return None
 
@@ -426,16 +437,18 @@ class Owner(models.Model):
 
     @classmethod
     def get_owner(cls, user_or_group):
+        """ Get owner instance for a group or user """
 
+        user_model = get_user_model()
         owner = None
         content_type_id = 0
         content_type_id_list = [ContentType.objects.get_for_model(Group).id,
-                                ContentType.objects.get_for_model(User).id]
+                                ContentType.objects.get_for_model(user_model).id]
 
         # If instance type is obvious: set content type
         if type(user_or_group) is Group:
             content_type_id = content_type_id_list[0]
-        elif type(user_or_group) is User:
+        elif type(user_or_group) is get_user_model():
             content_type_id = content_type_id_list[1]
 
         if content_type_id:
@@ -462,8 +475,8 @@ class Owner(models.Model):
 
             # Check whether user_or_group is a User instance
             try:
-                user = User.objects.get(pk=user_or_group.id)
-            except User.DoesNotExist:
+                user = user_model.objects.get(pk=user_or_group.id)
+            except user_model.DoesNotExist:
                 user = None
 
             if user:
@@ -478,45 +491,50 @@ class Owner(models.Model):
         return owner
 
     def get_related_owners(self, include_group=False):
+        """
+        Get all owners "related" to an owner.
+        This method is useful to retrieve all "user-type" owners linked to a "group-type" owner
+        """
 
-        owner_users = None
+        user_model = get_user_model()
+        related_owners = None
 
         if type(self.owner) is Group:
-            users = User.objects.filter(groups__name=self.owner.name)
+            users = user_model.objects.filter(groups__name=self.owner.name)
 
             if include_group:
-                query = Q(owner_id__in=users, owner_type=ContentType.objects.get_for_model(User).id) | \
+                # Include "group-type" owner in the query
+                query = Q(owner_id__in=users, owner_type=ContentType.objects.get_for_model(user_model).id) | \
                     Q(owner_id=self.owner.id, owner_type=ContentType.objects.get_for_model(Group).id)
             else:
-                query = Q(owner_id__in=users, owner_type=ContentType.objects.get_for_model(User).id)
+                query = Q(owner_id__in=users, owner_type=ContentType.objects.get_for_model(user_model).id)
             
-            owner_users = Owner.objects.filter(query)
+            related_owners = Owner.objects.filter(query)
 
-        elif type(self.owner) is User:
-            owner_users = [self]
+        elif type(self.owner) is user_model:
+            related_owners = [self]
 
-        return owner_users
+        return related_owners
 
 
-def create_owner(full_update=False, owner=None):
-    """ Create all owners """
-    
-    if full_update:
-        # Create group owners
-        for group in Group.objects.all():
-            Owner.create(owner=group)
+@receiver(post_save, sender=Group, dispatch_uid='create_owner')
+@receiver(post_save, sender=get_user_model(), dispatch_uid='create_owner')
+def create_owner(sender, instance, **kwargs):
+    """
+    Callback function to create a new owner instance
+    after either a new group or user instance is saved.
+    """
 
-        # Create user owners
-        for user in User.objects.all():
-            Owner.create(owner=user)
-    else:
-        if owner:
-            Owner.create(owner=owner)
+    Owner.create(obj=instance)
 
 
-@receiver(post_save, sender=Group, dispatch_uid='create_missing_owner')
-@receiver(post_save, sender=User, dispatch_uid='create_missing_owner')
-def create_missing_owner(sender, instance, created, **kwargs):
-    """ Create owner instance after either user or group object is saved. """
+@receiver(post_delete, sender=Group, dispatch_uid='delete_owner')
+@receiver(post_delete, sender=get_user_model(), dispatch_uid='delete_owner')
+def delete_owner(sender, instance, **kwargs):
+    """
+    Callback function to delete an owner instance
+    after either a new group or user instance is deleted.
+    """
 
-    create_owner(owner=instance)
+    owner = Owner.get_owner(instance)
+    owner.delete()

InvenTree/users/tests.py

@@ -3,9 +3,10 @@ from __future__ import unicode_literals
 
 from django.test import TestCase
 from django.apps import apps
+from django.contrib.auth import get_user_model
 from django.contrib.auth.models import Group
 
-from users.models import RuleSet
+from users.models import RuleSet, Owner
 
 
 class RuleSetModelTest(TestCase):
@@ -157,3 +158,48 @@ class RuleSetModelTest(TestCase):
 
         # There should now not be any permissions assigned to this group
         self.assertEqual(group.permissions.count(), 0)
+
+
+class OwnerModelTest(TestCase):
+    """
+    Some simplistic tests to ensure the Owner model is setup correctly.
+    """
+
+    def setUp(self):
+        """ Add users and groups """
+
+        # Create a new user
+        self.user = get_user_model().objects.create_user(
+            username='john',
+            email='john@email.com',
+            password='custom123',
+        )
+
+        # Put the user into a new group
+        self.group = Group.objects.create(name='new_group')
+        self.user.groups.add(self.group)
+
+    def test_owner(self):
+
+        # Check that owner was created for user
+        user_as_owner = Owner.get_owner(self.user)
+        self.assertEqual(type(user_as_owner), Owner)
+
+        # Check that owner was created for group
+        group_as_owner = Owner.get_owner(self.group)
+        self.assertEqual(type(group_as_owner), Owner)
+
+        # Get related owners (user + group)
+        related_owners = group_as_owner.get_related_owners(include_group=True)
+        self.assertTrue(user_as_owner in related_owners)
+        self.assertTrue(group_as_owner in related_owners)
+
+        # Delete user and verify owner was deleted too
+        self.user.delete()
+        user_as_owner = Owner.get_owner(self.user)
+        self.assertEqual(user_as_owner, None)
+        
+        # Delete group and verify owner was deleted too
+        self.group.delete()
+        group_as_owner = Owner.get_owner(self.group)
+        self.assertEqual(group_as_owner, None)