Skip to content

Commit

Permalink
Improve Project-User Management and Accessibility (#275)
Browse files Browse the repository at this point in the history
* Initial support for project owners

* Added projects<->users many-to-many relationship

* Auto-assign user as project owner when the user creates a project

* Exposed current user's projects via users/me/ endpoint

* Allowed add users to a project through the admin page

* Added tests for project owner auto-assignment & user's project

* fix: made the select user field optional & hide the project' users field in project list view

* fix: show projects that a user is added to

* Added public query parameter to filter current user's projects for /projects endpoint

* feat: update project overview to handle user projects

* style: move create button to header

* Renamed users field to members and made it optional

* Use user_id param for project filtering

* Update the frontend to filter projects by user_id

* Add project creator to members if they are not already a member

* Added project model manager

* Removed user projects from /me endpoint

* Updated /projects user_id filter to just check if the filtering is for the current logged in user

* Moved the logic for adding  project owner to  members  to Project.save

* feat: include selected view as query param

* fix : add project owner to members when the project is created from admin page

* Squashed migrations

* Deleted old migration files

* feat: function for ensuring the owner is a member

---------

Co-authored-by: mohamedelabbas1996 <[email protected]> (primary author)
Co-authored-by: Anna Viklund <[email protected]>
  • Loading branch information
mihow and annavik authored Jan 24, 2025
1 parent 2d11938 commit 650a305
Show file tree
Hide file tree
Showing 10 changed files with 187 additions and 16 deletions.
20 changes: 19 additions & 1 deletion ami/main/admin.py
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,25 @@ class BlogPostAdmin(admin.ModelAdmin[BlogPost]):
class ProjectAdmin(admin.ModelAdmin[Project]):
"""Admin panel example for ``Project`` model."""

list_display = ("name", "priority", "active", "created_at", "updated_at")
def save_related(self, request, form, formsets, change):
super().save_related(request, form, formsets, change)
form.instance.ensure_owner_membership()

list_display = ("name", "owner", "priority", "active", "created_at", "updated_at")
list_filter = ("active", "owner")
search_fields = ("name", "owner__username", "members__username")
filter_horizontal = ("members",)

fieldsets = (
(None, {"fields": ("name", "description", "priority", "active")}),
(
"Ownership & Access",
{
"fields": ("owner", "members"),
"classes": ("wide",),
},
),
)

@admin.action(description="Remove duplicate classifications from all detections")
def _remove_duplicate_classifications(self, request: HttpRequest, queryset: QuerySet[Project]) -> None:
Expand Down
2 changes: 2 additions & 0 deletions ami/main/api/serializers.py
Original file line number Diff line number Diff line change
Expand Up @@ -269,12 +269,14 @@ class Meta:

class ProjectSerializer(DefaultSerializer):
deployments = DeploymentNestedSerializerWithLocationAndCounts(many=True, read_only=True)
owner = UserNestedSerializer(read_only=True)

class Meta:
model = Project
fields = ProjectListSerializer.Meta.fields + [
"deployments",
"summary_data", # @TODO move to a 2nd request, it's too slow
"owner",
]


Expand Down
42 changes: 40 additions & 2 deletions ami/main/api/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,11 +10,12 @@
from django.forms import BooleanField, CharField, IntegerField
from django.utils import timezone
from django_filters.rest_framework import DjangoFilterBackend
from drf_spectacular.utils import extend_schema
from drf_spectacular.types import OpenApiTypes
from drf_spectacular.utils import OpenApiParameter, extend_schema
from rest_framework import exceptions as api_exceptions
from rest_framework import filters, serializers, status, viewsets
from rest_framework.decorators import action
from rest_framework.exceptions import NotFound
from rest_framework.exceptions import NotFound, PermissionDenied
from rest_framework.filters import SearchFilter
from rest_framework.generics import GenericAPIView
from rest_framework.request import Request
Expand Down Expand Up @@ -44,6 +45,7 @@
SourceImageCollection,
SourceImageUpload,
Taxon,
User,
update_detection_counts,
)
from .serializers import (
Expand Down Expand Up @@ -121,6 +123,18 @@ class ProjectViewSet(DefaultViewSet):
serializer_class = ProjectSerializer
pagination_class = ProjectPagination

def get_queryset(self):
qs: QuerySet = super().get_queryset()
# Filter projects by `user_id`
user_id = self.request.query_params.get("user_id")
if user_id:
user = User.objects.filter(pk=user_id).first()
if not user == self.request.user:
raise PermissionDenied("You can only view your projects")
if user:
qs = qs.filter_by_user(user)
return qs

def get_serializer_class(self):
"""
Return different serializers for list and detail views.
Expand All @@ -130,6 +144,30 @@ def get_serializer_class(self):
else:
return ProjectSerializer

def perform_create(self, serializer):
# Check if user is authenticated
if not self.request.user or not self.request.user.is_authenticated:
raise PermissionDenied("You must be authenticated to create a project.")

# Add current user as project owner
serializer.save(owner=self.request.user)

@extend_schema(
parameters=[
OpenApiParameter(
name="user_id",
description=(
"Filters projects to show only those associated with the specified user ID. "
"If omitted, no user-specific filter is applied."
),
required=False,
type=OpenApiTypes.INT,
),
]
)
def list(self, request, *args, **kwargs):
return super().list(request, *args, **kwargs)


class DeploymentViewSet(DefaultViewSet):
"""
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
# Generated by Django 4.2.10 on 2025-01-23 10:37

from django.conf import settings
from django.db import migrations, models
import django.db.models.deletion


class Migration(migrations.Migration):
replaces = [
("main", "0039_project_users"),
("main", "0007_project_owner"),
("main", "0040_merge_0007_project_owner_0039_project_users"),
("main", "0041_alter_project_users"),
("main", "0042_alter_project_users"),
("main", "0043_rename_users_project_members"),
]

dependencies = [
migrations.swappable_dependency(settings.AUTH_USER_MODEL),
("main", "0038_alter_detection_path_alter_sourceimage_event_and_more"),
]

operations = [
migrations.AddField(
model_name="project",
name="owner",
field=models.ForeignKey(
null=True,
on_delete=django.db.models.deletion.SET_NULL,
related_name="projects",
to=settings.AUTH_USER_MODEL,
),
),
migrations.AddField(
model_name="project",
name="members",
field=models.ManyToManyField(blank=True, related_name="user_projects", to=settings.AUTH_USER_MODEL),
),
]
24 changes: 23 additions & 1 deletion ami/main/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -95,14 +95,28 @@ def create_default_research_site(project: "Project") -> "Site":
return site


class ProjectQuerySet(models.QuerySet):
def filter_by_user(self, user):
"""
Filters projects to include only those where the given user is a member.
"""
return self.filter(members=user)


class ProjectManager(models.Manager):
def get_queryset(self) -> ProjectQuerySet:
return ProjectQuerySet(self.model, using=self._db)


@final
class Project(BaseModel):
""" """

name = models.CharField(max_length=_POST_TITLE_MAX_LENGTH)
description = models.TextField()
image = models.ImageField(upload_to="projects", blank=True, null=True)

owner = models.ForeignKey(User, on_delete=models.SET_NULL, null=True, related_name="projects")
members = models.ManyToManyField(User, related_name="user_projects", blank=True)
# Backreferences for type hinting
deployments: models.QuerySet["Deployment"]
events: models.QuerySet["Event"]
Expand All @@ -116,6 +130,12 @@ class Project(BaseModel):
devices: models.QuerySet["Device"]
sites: models.QuerySet["Site"]
jobs: models.QuerySet["Job"]
objects = ProjectManager()

def ensure_owner_membership(self):
"""Add owner to members if they are not already a member"""
if self.owner and not self.members.filter(id=self.owner.pk).exists():
self.members.add(self.owner)

def deployments_count(self) -> int:
return self.deployments.count()
Expand Down Expand Up @@ -150,6 +170,8 @@ def create_related_defaults(self):
def save(self, *args, **kwargs):
new_project = bool(self._state.adding)
super().save(*args, **kwargs)
# Add owner to members
self.ensure_owner_membership()
if new_project:
logger.info(f"Created new project {self}")
self.create_related_defaults()
Expand Down
22 changes: 22 additions & 0 deletions ami/main/tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -882,3 +882,25 @@ def test_project_devices(self):
exepcted_device_ids = {device.id for device in Device.objects.filter(project=project)}
response_device_ids = {device.get("id") for device in response_data["results"]}
self.assertEqual(response_device_ids, exepcted_device_ids)


class TestProjectOwnerAutoAssignment(APITestCase):
def setUp(self) -> None:
self.user_1 = User.objects.create_user(
email="[email protected]",
is_staff=True,
)
self.user_2 = User.objects.create_user(
email="[email protected]",
is_staff=True,
)
self.factory = APIRequestFactory()
self.client.force_authenticate(user=self.user_1)
return super().setUp()

def test_can_auto_assign_project_owner(self):
project_endpoint = "/api/v2/projects/"
request = {"name": "Test Project1234", "description": "Test Description"}
self.client.post(project_endpoint, request)
project = Project.objects.filter(name=request["name"]).first()
self.assertEqual(self.user_1.id, project.owner.id)
1 change: 0 additions & 1 deletion ui/src/components/gallery/gallery.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,6 @@ export const Gallery = ({
/>
))
)}

{!isLoading && items.length === 0 && <EmptyState />}
</div>
)
Expand Down
5 changes: 4 additions & 1 deletion ui/src/design-system/components/tabs/tabs.module.scss
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,12 @@
}

.tabsList {
margin-bottom: 16px;
padding: 2px 0;

&:not(:last-child) {
margin-bottom: 16px;
}

:not(:last-child) {
margin-right: 4px;
}
Expand Down
44 changes: 34 additions & 10 deletions ui/src/pages/projects/projects.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,17 +2,33 @@ import { useProjects } from 'data-services/hooks/projects/useProjects'
import { PageFooter } from 'design-system/components/page-footer/page-footer'
import { PageHeader } from 'design-system/components/page-header/page-header'
import { PaginationBar } from 'design-system/components/pagination-bar/pagination-bar'
import * as Tabs from 'design-system/components/tabs/tabs'
import { NewProjectDialog } from 'pages/project-details/new-project-dialog'
import { STRING, translate } from 'utils/language'
import { usePagination } from 'utils/usePagination'
import { UserPermission } from 'utils/user/types'
import { useUser } from 'utils/user/userContext'
import { useUserInfo } from 'utils/user/userInfoContext'
import { useSelectedView } from 'utils/useSelectedView'
import { ProjectGallery } from './project-gallery'
import styles from './projects.module.scss'

export const TABS = {
MY_PROJECTS: 'my-projects',
ALL_PROJECTS: 'all-projects',
}

export const Projects = () => {
const { user } = useUser()
const { userInfo } = useUserInfo()
const { selectedView: selectedTab, setSelectedView: setSelectedTab } =
useSelectedView(user.loggedIn ? TABS.MY_PROJECTS : TABS.ALL_PROJECTS)
const { pagination, setPage } = usePagination()
const filters =
user.loggedIn && selectedTab === TABS.MY_PROJECTS
? [{ field: 'user_id', value: userInfo?.id }]
: []
const { projects, total, userPermissions, isLoading, isFetching, error } =
useProjects({ pagination })
useProjects({ pagination, filters })
const canCreate = userPermissions?.includes(UserPermission.Create)

return (
Expand All @@ -23,15 +39,23 @@ export const Projects = () => {
isLoading={isLoading}
isFetching={isFetching}
>
{canCreate && <NewProjectDialog />}
{user.loggedIn ? (
<Tabs.Root onValueChange={setSelectedTab} value={selectedTab}>
<Tabs.List>
<Tabs.Trigger
label={translate(STRING.TAB_ITEM_MY_PROJECTS)}
value={TABS.MY_PROJECTS}
/>
<Tabs.Trigger
label={translate(STRING.TAB_ITEM_ALL_PROJECTS)}
value={TABS.ALL_PROJECTS}
/>
</Tabs.List>
</Tabs.Root>
) : null}
{canCreate ? <NewProjectDialog /> : null}
</PageHeader>
<div className={styles.galleryContent}>
<ProjectGallery
error={error}
isLoading={isLoading}
projects={projects}
/>
</div>
<ProjectGallery error={error} isLoading={isLoading} projects={projects} />
<PageFooter>
{projects?.length ? (
<PaginationBar
Expand Down
4 changes: 4 additions & 0 deletions ui/src/utils/language.ts
Original file line number Diff line number Diff line change
Expand Up @@ -163,11 +163,13 @@ export enum STRING {
NAV_ITEM_TAXA,

/* TAB_ITEM */
TAB_ITEM_ALL_PROJECTS,
TAB_ITEM_COLLECTIONS,
TAB_ITEM_DEVICES,
TAB_ITEM_FIELDS,
TAB_ITEM_GALLERY,
TAB_ITEM_IDENTIFICATION,
TAB_ITEM_MY_PROJECTS,
TAB_ITEM_PIPELINES,
TAB_ITEM_SITES,
TAB_ITEM_STORAGE,
Expand Down Expand Up @@ -411,11 +413,13 @@ const ENGLISH_STRINGS: { [key in STRING]: string } = {
[STRING.NAV_ITEM_TAXA]: 'Taxa',

/* TAB_ITEM */
[STRING.TAB_ITEM_ALL_PROJECTS]: 'All projects',
[STRING.TAB_ITEM_COLLECTIONS]: 'Collections',
[STRING.TAB_ITEM_DEVICES]: 'Device types',
[STRING.TAB_ITEM_FIELDS]: 'Fields',
[STRING.TAB_ITEM_GALLERY]: 'Gallery view',
[STRING.TAB_ITEM_IDENTIFICATION]: 'Identification',
[STRING.TAB_ITEM_MY_PROJECTS]: 'My projects',
[STRING.TAB_ITEM_PIPELINES]: 'Pipelines',
[STRING.TAB_ITEM_SITES]: 'Sites',
[STRING.TAB_ITEM_STORAGE]: 'Storage',
Expand Down

0 comments on commit 650a305

Please sign in to comment.