Skip to content

baseapp-profiles: change profile filter to add name and urlpath query - BA-1737 #178

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Nov 22, 2024
Merged

baseapp-profiles: change profile filter to add name and urlpath query - BA-1737 #178

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 24 additions & 0 deletions baseapp-profiles/baseapp_profiles/graphql/filters.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
import django_filters
import swapper
from django.db.models import Q

Profile = swapper.load_model("baseapp_profiles", "Profile")


class ProfileFilter(django_filters.FilterSet):
q = django_filters.CharFilter(method="filter_q")

order_by = django_filters.OrderingFilter(
fields=(
("created", "created"),
("followers_count__total", "followers_count_total"),
("following_count__total", "following_count_total"),
)
)

class Meta:
model = Profile
fields = ["q", "order_by"]

def filter_q(self, queryset, name, value):
return queryset.filter(Q(name__icontains=value) | Q(url_paths__path__icontains=value))
9 changes: 2 additions & 7 deletions baseapp-profiles/baseapp_profiles/graphql/object_types.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import django_filters
import graphene
import swapper
from baseapp_auth.graphql import PermissionsInterface
Expand All @@ -13,6 +12,8 @@
from graphene import relay
from graphene_django.filter import DjangoFilterConnectionField

from .filters import ProfileFilter

Profile = swapper.load_model("baseapp_profiles", "Profile")
ProfileUserRole = swapper.load_model("baseapp_profiles", "ProfileUserRole")

Expand Down Expand Up @@ -41,12 +42,6 @@ class ProfileInterface(relay.Node):
profile = graphene.Field(get_object_type_for_model(Profile))


class ProfileFilter(django_filters.FilterSet):
class Meta:
model = Profile
fields = ["name"]


class ProfileMetadata(AbstractMetadataObjectType):
@property
def meta_title(self):
Expand Down
40 changes: 40 additions & 0 deletions baseapp-profiles/baseapp_profiles/tests/test_get_queries.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
import pytest
import swapper
from baseapp_pages.tests.factories import URLPathFactory
from django.contrib.contenttypes.models import ContentType

from .factories import ProfileFactory

Expand All @@ -22,6 +24,18 @@
}
"""

SEARCH_PROFILES_BY_QUERY_PARAM = """
query AllProfiles($q: String!) {
allProfiles(q: $q) {
edges {
node {
id
}
}
}
}
"""


def test_profile_metadata(graphql_user_client, image_djangofile):
profile = ProfileFactory(image=image_djangofile)
Expand Down Expand Up @@ -128,3 +142,29 @@ def test_another_user_cant_view_members(graphql_user_client):
content = response.json()

assert content["data"]["profile"]["members"]
Copy link

@coderabbitai coderabbitai bot Nov 20, 2024

Choose a reason for hiding this comment

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

⚠️ Potential issue

Critical: Existing test has incorrect assertion

The test test_another_user_cant_view_members asserts that another user CAN view members (assert content["data"]["profile"]["members"]), which contradicts the test name and likely the intended behavior. This should be fixed while we're updating the test file.

-    assert content["data"]["profile"]["members"]
+    assert not content["data"]["profile"]["members"]
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
assert content["data"]["profile"]["members"]
assert not content["data"]["profile"]["members"]



def test_search_profiles(graphql_user_client):
profile1 = ProfileFactory(name="David")
profile2 = ProfileFactory(name="Daniel")
profile3 = ProfileFactory(name="Mark")
profile4 = ProfileFactory(name="John")
profile5 = ProfileFactory(name="Donald")
urlPath1 = URLPathFactory(path="danger.john", is_active=True, language=None)
profile_content_type = ContentType.objects.get_for_model(Profile)
urlPath1.target_content_type = profile_content_type
urlPath1.target_object_id = profile4.id
urlPath1.save()
urlPath1.refresh_from_db()
profile4.refresh_from_db()

response = graphql_user_client(SEARCH_PROFILES_BY_QUERY_PARAM, variables={"q": "da"})
content = response.json()
profiles = [
id for id in [edge["node"]["id"] for edge in content["data"]["allProfiles"]["edges"]]
]
assert profile1.relay_id in profiles
assert profile2.relay_id in profiles
assert profile3.relay_id not in profiles
assert profile4.relay_id in profiles
assert profile5.relay_id not in profiles
Comment on lines +147 to +170
Copy link

@coderabbitai coderabbitai bot Nov 20, 2024

Choose a reason for hiding this comment

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

⚠️ Potential issue

Critical: Test coverage and assertions need improvement

Several issues need to be addressed:

  1. The test only implicitly covers name search but doesn't explicitly test URL path search
  2. The assertion for profile4 (John) seems incorrect as it doesn't contain "da" but is expected in results
  3. Unnecessary database refreshes can be removed

Here's a suggested improvement:

 def test_search_profiles(graphql_user_client):
-    profile1 = ProfileFactory(name="David")
-    profile2 = ProfileFactory(name="Daniel")
-    profile3 = ProfileFactory(name="Mark")
-    profile4 = ProfileFactory(name="John")
-    profile5 = ProfileFactory(name="Donald")
-    urlPath1 = URLPathFactory(path="danger.john", is_active=True, language=None)
-    profile_content_type = ContentType.objects.get_for_model(Profile)
-    urlPath1.target_content_type = profile_content_type
-    urlPath1.target_object_id = profile4.id
-    urlPath1.save()
-    urlPath1.refresh_from_db()
-    profile4.refresh_from_db()
+    # Test profiles for name search
+    profile_david = ProfileFactory(name="David")
+    profile_daniel = ProfileFactory(name="Daniel")
+    profile_mark = ProfileFactory(name="Mark")
+    
+    # Test profile for URL path search
+    profile_with_path = ProfileFactory(name="John")
+    URLPathFactory(
+        path="dashboard/john",
+        is_active=True,
+        target_content_type=ContentType.objects.get_for_model(Profile),
+        target_object_id=profile_with_path.id
+    )
 
-    response = graphql_user_client(SEARCH_PROFILES_BY_QUERY_PARAM, variables={"q": "da"})
-    content = response.json()
-    profiles = [
-        id for id in [edge["node"]["id"] for edge in content["data"]["allProfiles"]["edges"]]
-    ]
-    assert profile1.relay_id in profiles
-    assert profile2.relay_id in profiles
-    assert profile3.relay_id not in profiles
-    assert profile4.relay_id in profiles
-    assert profile5.relay_id not in profiles
+    def get_profile_ids(query):
+        response = graphql_user_client(SEARCH_PROFILES_BY_QUERY_PARAM, variables={"q": query})
+        return [edge["node"]["id"] for edge in response.json()["data"]["allProfiles"]["edges"]]
+    
+    # Test name search
+    name_results = get_profile_ids("da")
+    assert profile_david.relay_id in name_results
+    assert profile_daniel.relay_id in name_results
+    assert profile_mark.relay_id not in name_results
+    assert profile_with_path.relay_id not in name_results
+    
+    # Test URL path search
+    path_results = get_profile_ids("dashboard")
+    assert profile_with_path.relay_id in path_results
+    assert profile_david.relay_id not in path_results
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
def test_search_profiles(graphql_user_client):
profile1 = ProfileFactory(name="David")
profile2 = ProfileFactory(name="Daniel")
profile3 = ProfileFactory(name="Mark")
profile4 = ProfileFactory(name="John")
profile5 = ProfileFactory(name="Donald")
urlPath1 = URLPathFactory(path="danger.john", is_active=True, language=None)
profile_content_type = ContentType.objects.get_for_model(Profile)
urlPath1.target_content_type = profile_content_type
urlPath1.target_object_id = profile4.id
urlPath1.save()
urlPath1.refresh_from_db()
profile4.refresh_from_db()
response = graphql_user_client(SEARCH_PROFILES_BY_QUERY_PARAM, variables={"q": "da"})
content = response.json()
profiles = [
id for id in [edge["node"]["id"] for edge in content["data"]["allProfiles"]["edges"]]
]
assert profile1.relay_id in profiles
assert profile2.relay_id in profiles
assert profile3.relay_id not in profiles
assert profile4.relay_id in profiles
assert profile5.relay_id not in profiles
def test_search_profiles(graphql_user_client):
# Test profiles for name search
profile_david = ProfileFactory(name="David")
profile_daniel = ProfileFactory(name="Daniel")
profile_mark = ProfileFactory(name="Mark")
# Test profile for URL path search
profile_with_path = ProfileFactory(name="John")
URLPathFactory(
path="dashboard/john",
is_active=True,
target_content_type=ContentType.objects.get_for_model(Profile),
target_object_id=profile_with_path.id
)
def get_profile_ids(query):
response = graphql_user_client(SEARCH_PROFILES_BY_QUERY_PARAM, variables={"q": query})
return [edge["node"]["id"] for edge in response.json()["data"]["allProfiles"]["edges"]]
# Test name search
name_results = get_profile_ids("da")
assert profile_david.relay_id in name_results
assert profile_daniel.relay_id in name_results
assert profile_mark.relay_id not in name_results
assert profile_with_path.relay_id not in name_results
# Test URL path search
path_results = get_profile_ids("dashboard")
assert profile_with_path.relay_id in path_results
assert profile_david.relay_id not in path_results

2 changes: 1 addition & 1 deletion baseapp-profiles/setup.cfg
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
[metadata]
name = baseapp_profiles
version = 0.3.0
version = 0.3.1
description = BaseApp Profiles
long_description = file: README.md
long_description_content_type = text/markdown
Expand Down