Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[feature] Added multitenant capablities to django-filters #346 #347

Merged
merged 12 commits into from
Apr 20, 2023

Conversation

Aryamanz29
Copy link
Member

@Aryamanz29 Aryamanz29 commented Mar 23, 2023

Todo

  • Add tests.
  • Add docs.

Closes #346

@Aryamanz29 Aryamanz29 force-pushed the issue-346/add-multi-tenant-capablities-dj-filters branch from b1a2441 to cbe401a Compare March 23, 2023 15:07
@coveralls
Copy link

coveralls commented Mar 23, 2023

Coverage Status

Coverage: 97.811% (-0.06%) from 97.872% when pulling e173d71 on issue-346/add-multi-tenant-capablities-dj-filters into 452ad6e on master.

@Aryamanz29 Aryamanz29 force-pushed the issue-346/add-multi-tenant-capablities-dj-filters branch 2 times, most recently from e5f3fdb to 55d0afb Compare March 25, 2023 20:21
@Aryamanz29 Aryamanz29 force-pushed the issue-346/add-multi-tenant-capablities-dj-filters branch from 55d0afb to 097c839 Compare March 25, 2023 20:24
@Aryamanz29 Aryamanz29 force-pushed the issue-346/add-multi-tenant-capablities-dj-filters branch from 8aa1c43 to d965671 Compare March 25, 2023 21:14
@Aryamanz29 Aryamanz29 self-assigned this Mar 25, 2023
@Aryamanz29 Aryamanz29 marked this pull request as ready for review March 25, 2023 21:22
@Aryamanz29 Aryamanz29 requested a review from nemesifier March 25, 2023 21:22
Copy link
Member

@nemesifier nemesifier left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great progress @Aryamanz29! 👏

I did some testing and it's working well.
There's some room for improvement, since this is going to become a feature we will use in all our modules.

I believe we should add something like this to this library to avoid repeating the same over and over in all modules:

class BaseOrganizationFilter(FilterDjangoByOrgManaged):
   organization_slug = filters.CharFilter(field_name='organization__slug')
   class Meta:
       fields = ['organization', 'organization_slug']

Usage example in Network Topology module:

class OrganizationFilter(BaseOrganizationFilter):
    class Meta(BaseOrganizationFilter.Meta):
        model = Node

See also my comments below.

README.rst Outdated Show resolved Hide resolved
README.rst Outdated Show resolved Hide resolved
README.rst Outdated Show resolved Hide resolved
openwisp_users/api/mixins.py Show resolved Hide resolved
openwisp_users/api/mixins.py Outdated Show resolved Hide resolved
README.rst Show resolved Hide resolved
openwisp_users/api/mixins.py Outdated Show resolved Hide resolved
Added the django filter class that can be used across various
OpenWISP API views to filter options based on the organization managed by the user
@Aryamanz29 Aryamanz29 force-pushed the issue-346/add-multi-tenant-capablities-dj-filters branch from 989b0be to a0542ae Compare April 3, 2023 12:37
@Aryamanz29 Aryamanz29 requested a review from nemesifier April 3, 2023 12:41
We should construct the opts dictionary after checking
if the field is an instance of ForeignKey or ManyToManyField.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[bug] Multi-tenant capabilities missing for django-filters
3 participants