Skip to content

Commit

Permalink
Increase test coverage for users app (#373)
Browse files Browse the repository at this point in the history
  • Loading branch information
djperrefort authored Aug 8, 2024
1 parent 35cb04c commit 18e55ab
Show file tree
Hide file tree
Showing 23 changed files with 259 additions and 94 deletions.
32 changes: 16 additions & 16 deletions docs/api.yml
Original file line number Diff line number Diff line change
Expand Up @@ -3665,7 +3665,7 @@ paths:
/users/researchgroups/:
get:
operationId: users_researchgroups_list
description: Manage user membership in research groups
description: Manage user membership in research groups.
parameters:
- in: query
name: id
Expand Down Expand Up @@ -3761,7 +3761,7 @@ paths:
description: ''
post:
operationId: users_researchgroups_create
description: Manage user membership in research groups
description: Manage user membership in research groups.
tags:
- users
requestBody:
Expand All @@ -3788,7 +3788,7 @@ paths:
/users/researchgroups/{id}/:
get:
operationId: users_researchgroups_retrieve
description: Manage user membership in research groups
description: Manage user membership in research groups.
parameters:
- in: path
name: id
Expand All @@ -3809,7 +3809,7 @@ paths:
description: ''
put:
operationId: users_researchgroups_update
description: Manage user membership in research groups
description: Manage user membership in research groups.
parameters:
- in: path
name: id
Expand Down Expand Up @@ -3842,7 +3842,7 @@ paths:
description: ''
patch:
operationId: users_researchgroups_partial_update
description: Manage user membership in research groups
description: Manage user membership in research groups.
parameters:
- in: path
name: id
Expand Down Expand Up @@ -3874,7 +3874,7 @@ paths:
description: ''
delete:
operationId: users_researchgroups_destroy
description: Manage user membership in research groups
description: Manage user membership in research groups.
parameters:
- in: path
name: id
Expand All @@ -3892,7 +3892,7 @@ paths:
/users/users/:
get:
operationId: users_users_list
description: Read only access to user data
description: Manage user account data.
parameters:
- in: query
name: date_joined
Expand Down Expand Up @@ -4355,7 +4355,7 @@ paths:
description: ''
post:
operationId: users_users_create
description: Read only access to user data
description: Manage user account data.
tags:
- users
requestBody:
Expand All @@ -4382,7 +4382,7 @@ paths:
/users/users/{id}/:
get:
operationId: users_users_retrieve
description: Read only access to user data
description: Manage user account data.
parameters:
- in: path
name: id
Expand All @@ -4403,7 +4403,7 @@ paths:
description: ''
put:
operationId: users_users_update
description: Read only access to user data
description: Manage user account data.
parameters:
- in: path
name: id
Expand Down Expand Up @@ -4436,7 +4436,7 @@ paths:
description: ''
patch:
operationId: users_users_partial_update
description: Read only access to user data
description: Manage user account data.
parameters:
- in: path
name: id
Expand Down Expand Up @@ -4468,7 +4468,7 @@ paths:
description: ''
delete:
operationId: users_users_destroy
description: Read only access to user data
description: Manage user account data.
parameters:
- in: path
name: id
Expand Down Expand Up @@ -4887,7 +4887,7 @@ components:
type: integer
PatchedResearchGroup:
type: object
description: Object serializer for the `ResearchGroup` class
description: Object serializer for the `ResearchGroup` model.
properties:
id:
type: integer
Expand All @@ -4908,7 +4908,7 @@ components:
PatchedRestrictedUser:
type: object
description: Object serializer for the `User` class with administrative fields
marked as read only
marked as read only.
properties:
id:
type: integer
Expand Down Expand Up @@ -5053,7 +5053,7 @@ components:
- time
ResearchGroup:
type: object
description: Object serializer for the `ResearchGroup` class
description: Object serializer for the `ResearchGroup` model.
properties:
id:
type: integer
Expand All @@ -5078,7 +5078,7 @@ components:
RestrictedUser:
type: object
description: Object serializer for the `User` class with administrative fields
marked as read only
marked as read only.
properties:
id:
type: integer
Expand Down
8 changes: 4 additions & 4 deletions keystone_api/apps/allocations/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ def get_research_group(self) -> ResearchGroup:

return self.request.group

def __str__(self) -> str:
def __str__(self) -> str: # pragma: nocover
"""Return a human-readable summary of the allocation"""

return f'{self.cluster} allocation for {self.request.group}'
Expand Down Expand Up @@ -91,7 +91,7 @@ def get_research_group(self) -> ResearchGroup:

return self.group

def __str__(self) -> str:
def __str__(self) -> str: # pragma: nocover
"""Return the request title as a string"""

return truncatechars(self.title, 100)
Expand Down Expand Up @@ -120,7 +120,7 @@ def get_research_group(self) -> ResearchGroup:

return self.request.group

def __str__(self) -> str:
def __str__(self) -> str: # pragma: nocover
"""Return a human-readable identifier for the allocation request"""

return f'{self.reviewer} review for \"{self.request.title}\"'
Expand All @@ -142,7 +142,7 @@ class Cluster(models.Model):
description = models.TextField(max_length=150, null=True, blank=True)
enabled = models.BooleanField(default=True)

def __str__(self) -> str:
def __str__(self) -> str: # pragma: nocover
"""Return the cluster name as a string"""

return str(self.name)
4 changes: 2 additions & 2 deletions keystone_api/apps/research_products/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ class Grant(models.Model):

objects = GrantManager()

def __str__(self) -> str:
def __str__(self) -> str: # pragma: nocover
"""Return the grant title truncated to 50 characters"""

return truncatechars(self.title, 100)
Expand All @@ -49,7 +49,7 @@ class Publication(models.Model):

objects = PublicationManager()

def __str__(self) -> str:
def __str__(self) -> str: # pragma: nocover
"""Return the publication title truncated to 50 characters"""

return truncatechars(self.title, 100)
10 changes: 5 additions & 5 deletions keystone_api/apps/users/admin.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,17 +22,17 @@

@admin.register(User)
class UserAdmin(auth.admin.UserAdmin):
"""Admin interface for managing user accounts"""
"""Admin interface for managing user accounts."""

@admin.action
def activate_selected_users(self, request, queryset) -> None:
"""Mark selected users as active"""
"""Mark selected users as active."""

queryset.update(is_active=True)

@admin.action
def deactivate_selected_users(self, request, queryset) -> None:
"""Mark selected users as inactive"""
"""Mark selected users as inactive."""

queryset.update(is_active=False)

Expand All @@ -52,12 +52,12 @@ def deactivate_selected_users(self, request, queryset) -> None:

@admin.register(ResearchGroup)
class ResearchGroupAdmin(admin.ModelAdmin):
"""Admin interface for managing research group delegates"""
"""Admin interface for managing research group delegates."""

@staticmethod
@admin.display
def pi(obj: ResearchGroup) -> str:
"""Return the username of the research group PI"""
"""Return the username of the research group PI."""

return obj.pi.username

Expand Down
18 changes: 9 additions & 9 deletions keystone_api/apps/users/managers.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,22 +12,22 @@
from django.contrib.auth.base_user import BaseUserManager
from django.db import models

if TYPE_CHECKING:
if TYPE_CHECKING: # pragma: nocover
from apps.users.models import User

__all__ = ['ResearchGroupManager', 'UserManager']


class UserManager(BaseUserManager):
"""Object manager for the `User` database model"""
"""Object manager for the `User` database model."""

def create_user(
self,
username: str,
password: str,
**extra_fields
self,
username: str,
password: str,
**extra_fields
) -> 'User':
"""Create and a new user account
"""Create a new user account.
Args:
username: The account username
Expand All @@ -54,7 +54,7 @@ def create_superuser(
password: str,
**extra_fields
) -> 'User':
"""Create and a new user account with superuser privileges
"""Create a new user account with superuser privileges.
Args:
username: The account username
Expand All @@ -79,7 +79,7 @@ def create_superuser(


class ResearchGroupManager(models.Manager):
"""Object manager for the `ResearchGroup` database model"""
"""Object manager for the `ResearchGroup` database model."""

def groups_for_user(self, user: 'User') -> models.QuerySet:
"""Get all research groups the user is affiliated with.
Expand Down
12 changes: 6 additions & 6 deletions keystone_api/apps/users/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@


class User(auth_models.AbstractBaseUser, auth_models.PermissionsMixin):
"""Proxy model for the built-in django `User` model"""
"""Proxy model for the built-in django `User` model."""

# These values should always be defined when extending AbstractBaseUser
USERNAME_FIELD = 'username'
Expand All @@ -44,7 +44,7 @@ class User(auth_models.AbstractBaseUser, auth_models.PermissionsMixin):


class ResearchGroup(models.Model):
"""A user research group tied to a slurm account"""
"""A user research group tied to a slurm account."""

name = models.CharField(max_length=255, unique=True)
pi = models.ForeignKey(User, on_delete=models.CASCADE, related_name='research_group_pi')
Expand All @@ -54,16 +54,16 @@ class ResearchGroup(models.Model):
objects = ResearchGroupManager()

def get_all_members(self) -> tuple[User, ...]:
"""Return all research group members"""
"""Return all research group members."""

return (self.pi,) + tuple(self.admins.all()) + tuple(self.members.all())

def get_privileged_members(self) -> tuple[User, ...]:
"""Return all research group members with admin privileges"""
"""Return all research group members with admin privileges."""

return (self.pi,) + tuple(self.admins.all())

def __str__(self) -> str:
"""Return the research group's account name"""
def __str__(self) -> str: # pragma: nocover # pragma: nocover
"""Return the research group's account name."""

return str(self.name)
12 changes: 6 additions & 6 deletions keystone_api/apps/users/permissions.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,21 +16,21 @@


class IsGroupAdminOrReadOnly(permissions.BasePermission):
"""Grant read-only access is granted to all authenticated users.
"""Grant read-only access to all authenticated users.
Staff users retain all read/write permissions.
"""

def has_permission(self, request: Request, view: View) -> bool:
"""Return whether the request has permissions to access the requested resource"""
"""Return whether the request has permissions to access the requested resource."""

if request.method == 'TRACE':
return request.user.is_staff

return True

def has_object_permission(self, request: Request, view: View, obj: ResearchGroup):
"""Return whether the incoming HTTP request has permission to access a database record"""
"""Return whether the incoming HTTP request has permission to access a database record."""

# Read permissions are allowed to any request
if request.method in permissions.SAFE_METHODS:
Expand All @@ -41,10 +41,10 @@ def has_object_permission(self, request: Request, view: View, obj: ResearchGroup


class IsSelfOrReadOnly(permissions.BasePermission):
"""Gives read-only permissions to everyone but limits write access to staff users and record owners"""
"""Grant read-only permissions to everyone and limit write access to staff and record owners."""

def has_permission(self, request: Request, view: View) -> bool:
"""Return whether the request has permissions to access the requested resource"""
"""Return whether the request has permissions to access the requested resource."""

# Allow all users to read/update existing records
# Rely on object level permissions for further refinement of update permissions
Expand All @@ -55,7 +55,7 @@ def has_permission(self, request: Request, view: View) -> bool:
return request.user.is_staff

def has_object_permission(self, request: Request, view: View, obj: User) -> bool:
"""Return whether the incoming HTTP request has permission to access a database record"""
"""Return whether the incoming HTTP request has permission to access a database record."""

# Write operations are restricted to staff and user's modifying their own data
is_record_owner = obj == request.user
Expand Down
Loading

0 comments on commit 18e55ab

Please sign in to comment.