Improve Project-User Management and Accessibility #275

mihow · 2023-10-11T06:26:19Z

This pull request enhances how projects and project members (users) are managed and accessed. It introduces a many-to-many relationship between projects and members, ensuring that users are automatically assigned to a project when they create it. Django admin panel is updated to allow manual assignment of members to a project. A filtering by user_id is added to the /projects to filter projects that the current user is involved in.My Projects tab is added at the top of the home page to make it easier for users to find and access their projects.

netlify · 2023-10-11T06:26:48Z

✅ Deploy Preview for ami-dev canceled.

Name	Link
🔨 Latest commit	`ab6fddf`
🔍 Latest deploy log	https://app.netlify.com/sites/ami-dev/deploys/67928b793df3ca0008645f8a

netlify · 2023-10-11T06:28:22Z

✅ Deploy Preview for ami-storybook canceled.

Name	Link
🔨 Latest commit	`972243a`
🔍 Latest deploy log	https://app.netlify.com/sites/ami-storybook/deploys/6667858663a7490008b286fa

mihow · 2023-11-02T02:49:44Z

Every view query and nested serializer query should have a filter to enforce that the current user is owner of the associated project
Consider prefixing all api endpoints by /project//

mihow · 2023-11-03T08:56:50Z

All queries should include objects from public projects as well.

mohamedelabbas1996 · 2025-01-20T02:59:16Z

Changes in This PR

Add many-to-many relationship between projects & users - make a new migration & commit file
Auto-assign a user to a project when they create it
Expose the projects a user belongs to in the user profile API endpoint (/me)
Update Django admin for projects to manually add users to a project until public UI is ready
Add tests:
- Ensure the current user is added to a project when it is created via the API
- Ensure the current user gets only the projects they're involved in as (owner or user) through /users/me/ endpoint.
See possible UI for "My projects" section on top of the home page
- Make it easier to find your project Make it easier to find your project #518

mohamedelabbas1996 · 2025-01-20T03:00:39Z

Closes #402

…eld in project list view

annavik · 2025-01-20T16:07:05Z

Me and @mohamedelabbas1996 had a chat about this today, regarding some frontend perspectives:

We talked about extending the endpoint /projects with a filter, to make it possible to use the same endpoint to fetch "all projects" and "my projects". This might be a bit more practical than including project data with the /me response? For example, we can skip extra requests to get the full project details and we would also get the paginated response. That would mean we can streamline the frontend code a bit better.
We also talked about how to present this in UI. @mohamedelabbas1996 had a great idea about just adding tabs to the projects list. We can follow a similar UI pattern like we do for the project overview tabs.

We decided to discuss more in the team on the meeting tomorrow about next steps! :)

mihow · 2025-01-20T18:55:35Z

Thanks for collaborating on this PR @mohamedelabbas1996 and @annavik! I look forward to reviewing your commits and suggestions.

…rojects endpoint

annavik · 2025-01-22T16:48:53Z

Frontend updates pushed. Me and @mohamedelabbas1996 solved it by adding tabs to the header:

annavik · 2025-01-22T16:49:57Z

ui/src/pages/projects/projects.tsx

  const { pagination, setPage } = usePagination()
+  const filters =
+    user.loggedIn && selectedTab === TABS.USER_PROJECTS
+      ? [{ field: 'public', value: 'false' }]


@mohamedelabbas1996 here is where the params we pass to the backend can be tweaked if needed

annavik · 2025-01-22T16:54:13Z

For the filter param name public, I think it's slightly misleading since user projects can also be public. I think this filtering is more related to membership rather than visibility. But I'm also fine keeping things as is, just a minor! :)

If you change the filter name, I added a comment (to the PR) so you can see where in the FE code to tweak.

@mihow @mohamedelabbas1996

mohamedelabbas1996 · 2025-01-22T17:15:33Z

Thanks @annavik for adding the changes on the frontend. For the filter name, we can rename it to current_user. I think this makes more sense. @mihow what are your suggestions?

mihow · 2025-01-22T19:56:45Z

@mohamedelabbas1996 there is a chance for some inconstancy when retrieving "my projects" since there is both a project owner and project members. Some ideas to address this:

When filtering projects by user_id, search both members and the project owners
Add project owner as a member in the Project save method, if they are not already a member

My recommendation is to add a new reusable method to to Project QuerySet class that can be used everywhere. Project.objects.can_view(user) and Project.objects.can_edit(user) or can_access() or filter_by_user()

It takes a little more code to setup a QuerySet for a model the first time, but it's a pattern that I would like to continue using because it keeps the logic in one place. Here is one example for the SourceImage model. You define a QuerySet, a Manager and then assign the manager to the Model.

antenna/ami/main/models.py

Lines 1170 to 1171 in 14e9ec0

    
           class SourceImageQuerySet(models.QuerySet): 
        
               def with_occurrences_count(self, classification_threshold: float = 0):

antenna/ami/main/models.py

Lines 1194 to 1196 in 14e9ec0

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

antenna/ami/main/models.py

Line 1233 in 14e9ec0

objects = SourceImageManager()

mohamedelabbas1996 · 2025-01-22T23:33:10Z

mohamedelabbas1996 · 2025-01-22T23:33:56Z

Now project members can be managed from the Admin page

mohamedelabbas1996 · 2025-01-22T23:36:07Z

@mohamedelabbas1996 there is a chance for some inconstancy when retrieving "my projects" since there is both a project owner and project members. Some ideas to address this:
* When filtering projects by `user_id`, search both members and the project owners 

* Add project owner as a member in the Project save method, if they are not already a member

I opted to add the owner to members if not already exists and then we can just filter by members

mohamedelabbas1996 · 2025-01-22T23:37:49Z

My recommendation is to add a new reusable method to to Project QuerySet class that can be used everywhere. Project.objects.can_view(user) and Project.objects.can_edit(user) or can_access() or filter_by_user()

It takes a little more code to setup a QuerySet for a model the first time, but it's a pattern that I would like to continue using because it keeps the logic in one place. Here is one example for the SourceImage model. You define a QuerySet, a Manager and then assign the manager to the Model.

Added the model manager. Thanks for the recommendation!

mihow · 2025-01-23T00:19:54Z

ami/main/api/views.py

+        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 or is_active_staff(self.request.user)):


Active "staff" is actually everyone who has edit permissions. Superusers are a different boolean field. But how about we simplify and just check for the matching user for now? Since superusers can see anything in the Django admin.

mihow · 2025-01-23T00:21:37Z

ami/main/api/views.py

+        project = serializer.save(owner=self.request.user)
+
+        # Add owner to project members if not already a member
+        if not project.members.filter(id=self.request.user.id).exists():


I would move this check to the model Project.save() method instead, because there are other places where the owner can be set or changed. Like if you add a project in the Django admin, the owner is not made a member.

mihow · 2025-01-23T00:27:35Z

ami/main/models.py

+        """
+        Filters projects to include only those where the given user is a member.
+        """
+        return self.filter(members=user)


Since you made this queryset method (thank you!), it's pretty easy to filter by member or owner.

self.filter(models.Q(members=user) | models.Q(owner=user)).distinct()

mihow · 2025-01-23T00:27:54Z

ami/main/models.py

 @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")


I'm glad we kept owner in this PR

mihow · 2025-01-23T00:32:18Z

ami/main/tests.py

@@ -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):


Thanks for the tests! remember that is_staff is different than is_superuser. We are using is_staff to give a user edit permissions to everything in the app. is_superuser are the only users who can delete some objects (like jobs). We should review this in the next permission PRs.

@mihow Thank you for the clarification!

mihow · 2025-01-23T00:35:03Z

ui/src/pages/projects/projects.tsx

+  const { user } = useUser()
+  const { userInfo} = useUserInfo()
+
+  const [selectedTab, setSelectedTab] = useState(


@annavik can we add a URL path or param so we can link to all projects when necessary? With the new feature, if someone sends someone else the projects link, they likely won't see the same projects.

Yes good point, fixed! ✅

…r the current logged in user

mohamedelabbas1996

@mihow I 've tried to move adding project owner to members to Project.save method. It works well from the api but it doesn't auto-add the owner if we create a project from the admin page

mihow · 2025-01-23T01:47:41Z

ami/main/migrations/0007_project_owner.py

@@ -0,0 +1,25 @@
+# Generated by Django 4.2.2 on 2023-10-11 02:15


One more thing, will you squash these migrations into one or two? python manage.py squashmigrations first_one last_one. That will remove some clutter and extra steps when we merge into main (instead of adding the field, then renaming the field). I only do this for migrations that haven't modified live data yet (migrations in an branch like these).

…dmin page

…ect model

Initial support for project owners

f7e3376

Merge branch 'main' into project-owner

972243a

mihow mentioned this pull request Jun 10, 2024

Add project<->user association #402

Open

mihow changed the title ~~Initial support for project owners~~ [Draft] Initial support for project owners Jun 19, 2024

mohamedelabbas1996 added 6 commits January 17, 2025 10:56

Merge branch 'main' into project-owner

04e56e1

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

20d17be

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

434ae28

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

e34543a

Allowed add users to a project through the admin page

b627e30

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

49bf707

mohamedelabbas1996 added 3 commits January 19, 2025 22:09

cleanup: removed commented code

f6059c8

fix: made the select user field optional & hide the project' users fi…

d7c827f

…eld in project list view

fix: show projects that a user is added to

2778a59

annavik modified the milestone: Ready for labeling projects (AMBER & Insectarium) Jan 22, 2025

mohamedelabbas1996 and others added 3 commits January 22, 2025 09:52

Added public query parameter to filter current user's projects for /p…

8cd1e8c

…rojects endpoint

feat: update project overview to handle user projects

190162f

style: move create button to header

43edc99

annavik reviewed Jan 22, 2025

View reviewed changes

mohamedelabbas1996 added 2 commits January 22, 2025 14:55

Renamed users field to members and made it optional

f61679b

Use user_id param for project filtering

158b807

mohamedelabbas1996 changed the title ~~[Draft] Initial support for project owners~~ Improve Project-User Management and Accessibility Jan 22, 2025

mohamedelabbas1996 marked this pull request as ready for review January 22, 2025 21:35

mohamedelabbas1996 added 4 commits January 22, 2025 18:03

Update the frontend to fiter projects by user_id

c824370

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

aaf4bcc

Added project model manager

72bd44c

Removed user projects from /me endpoint

daffe4f

mihow commented Jan 23, 2025

View reviewed changes

mohamedelabbas1996 added 2 commits January 22, 2025 20:29

Updated /projects user_id filter to just check if the filtering is fo…

352ce7d

…r the current logged in user

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

f61d21b

mohamedelabbas1996 reviewed Jan 23, 2025

View reviewed changes

mihow commented Jan 23, 2025

View reviewed changes

annavik added 2 commits January 23, 2025 09:38

chore: format code and update tab key

2ed0054

feat: include selected view as query param

2f9e5f7

annavik force-pushed the project-owner branch from 24fcfa8 to 2f9e5f7 Compare January 23, 2025 08:39

mohamedelabbas1996 added 4 commits January 23, 2025 10:26

fix : add project owner to members when the project is created from a…

d62fea5

…dmin page

Squashed migrations

d56d112

Deleted old migration files

eb040c1

cleanup: refactored add member logic to add_member method in the proj…

ab6fddf

…ect model

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Improve Project-User Management and Accessibility #275

Improve Project-User Management and Accessibility #275

mihow commented Oct 11, 2023 •

edited by mohamedelabbas1996

Loading

netlify bot commented Oct 11, 2023 •

edited

Loading

netlify bot commented Oct 11, 2023 •

edited

Loading

mihow commented Nov 2, 2023

mihow commented Nov 3, 2023

mohamedelabbas1996 commented Jan 20, 2025 •

edited

Loading

mohamedelabbas1996 commented Jan 20, 2025

annavik commented Jan 20, 2025

mihow commented Jan 20, 2025 •

edited

Loading

annavik commented Jan 22, 2025

annavik Jan 22, 2025 •

edited

Loading

annavik commented Jan 22, 2025

mohamedelabbas1996 commented Jan 22, 2025 •

edited

Loading

mihow commented Jan 22, 2025

mohamedelabbas1996 commented Jan 22, 2025

mohamedelabbas1996 commented Jan 22, 2025

mohamedelabbas1996 commented Jan 22, 2025 •

edited

Loading

mohamedelabbas1996 commented Jan 22, 2025

mihow Jan 23, 2025

mihow Jan 23, 2025 •

edited

Loading

mihow Jan 23, 2025

mihow Jan 23, 2025

mihow Jan 23, 2025

mohamedelabbas1996 Jan 23, 2025

mihow Jan 23, 2025

annavik Jan 23, 2025

mohamedelabbas1996 left a comment

mihow Jan 23, 2025

		@@ -0,0 +1,25 @@
		# Generated by Django 4.2.2 on 2023-10-11 02:15

Improve Project-User Management and Accessibility #275

Are you sure you want to change the base?

Improve Project-User Management and Accessibility #275

Conversation

mihow commented Oct 11, 2023 • edited by mohamedelabbas1996 Loading

netlify bot commented Oct 11, 2023 • edited Loading

✅ Deploy Preview for ami-dev canceled.

netlify bot commented Oct 11, 2023 • edited Loading

✅ Deploy Preview for ami-storybook canceled.

mihow commented Nov 2, 2023

mihow commented Nov 3, 2023

mohamedelabbas1996 commented Jan 20, 2025 • edited Loading

Changes in This PR

mohamedelabbas1996 commented Jan 20, 2025

annavik commented Jan 20, 2025

mihow commented Jan 20, 2025 • edited Loading

annavik commented Jan 22, 2025

annavik Jan 22, 2025 • edited Loading

Choose a reason for hiding this comment

annavik commented Jan 22, 2025

mohamedelabbas1996 commented Jan 22, 2025 • edited Loading

mihow commented Jan 22, 2025

mohamedelabbas1996 commented Jan 22, 2025

mohamedelabbas1996 commented Jan 22, 2025

mohamedelabbas1996 commented Jan 22, 2025 • edited Loading

mohamedelabbas1996 commented Jan 22, 2025

Choose a reason for hiding this comment

mihow Jan 23, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mohamedelabbas1996 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mihow commented Oct 11, 2023 •

edited by mohamedelabbas1996

Loading

netlify bot commented Oct 11, 2023 •

edited

Loading

netlify bot commented Oct 11, 2023 •

edited

Loading

mohamedelabbas1996 commented Jan 20, 2025 •

edited

Loading

mihow commented Jan 20, 2025 •

edited

Loading

annavik Jan 22, 2025 •

edited

Loading

mohamedelabbas1996 commented Jan 22, 2025 •

edited

Loading

mohamedelabbas1996 commented Jan 22, 2025 •

edited

Loading

mihow Jan 23, 2025 •

edited

Loading