Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
9 changes: 5 additions & 4 deletions baseapp/files/tests/test_graphql_queries_object_files.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
from django.test import override_settings

"""
Query-count tests for the `FilesInterface`.

Expand All @@ -9,7 +11,6 @@

import pytest
import swapper
from constance.test import override_config
from django.contrib.contenttypes.models import ContentType

from baseapp_comments.tests.factories import CommentFactory
Expand Down Expand Up @@ -69,7 +70,7 @@ def attach_files(target, amount: int) -> None:
)


@override_config(ENABLE_PUBLIC_ID_LOGIC=True)
@override_settings(ENABLE_PUBLIC_ID_LOGIC=True)
def test_files_count_is_flat_regardless_of_file_volume(graphql_client_with_queries):
"""`filesCount` should be a flat query path: regardless of how many `File`
rows point at the target, the GraphQL query should make the same number of
Expand Down Expand Up @@ -98,7 +99,7 @@ def test_files_count_is_flat_regardless_of_file_volume(graphql_client_with_queri
assert small_count == EXPECTED_FILES_INTERFACE_QUERY_COUNT


@override_config(ENABLE_PUBLIC_ID_LOGIC=True)
@override_settings(ENABLE_PUBLIC_ID_LOGIC=True)
def test_files_count_zero_when_no_files_does_not_extra_query(graphql_client_with_queries):
"""A target with no files should resolve in the same query budget as one
with files — the FileTarget row simply doesn't exist yet and the annotation
Expand All @@ -124,7 +125,7 @@ def test_files_count_zero_when_no_files_does_not_extra_query(graphql_client_with
assert queries_empty.count == EXPECTED_FILES_INTERFACE_QUERY_COUNT


@override_config(ENABLE_PUBLIC_ID_LOGIC=True)
@override_settings(ENABLE_PUBLIC_ID_LOGIC=True)
def test_listing_comments_with_files_is_flat(graphql_client_with_queries):
"""Listing comments that each have files must not fan out one FileTarget
query per comment — the files annotations ride along in the comment list
Expand Down
6 changes: 3 additions & 3 deletions baseapp_auth/tests/graphql/test_queries_user.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import pytest
from constance.test import override_config
from django.contrib.auth.models import Permission
from django.contrib.contenttypes.models import ContentType
from django.test import override_settings

from baseapp_core.tests.factories import UserFactory

Expand Down Expand Up @@ -157,7 +157,7 @@ def test_overcomplex_queries_are_not_executed(graphql_client_with_queries) -> No
assert queries.count == 0


@override_config(ENABLE_PUBLIC_ID_LOGIC=False)
@override_settings(ENABLE_PUBLIC_ID_LOGIC=False)
def test_anon_can_query_users_list_with_optimized_query(graphql_client_with_queries) -> None:
UserFactory.create_batch(10)
ContentType.objects.clear_cache()
Expand All @@ -182,7 +182,7 @@ def test_anon_can_query_users_list_with_optimized_query(graphql_client_with_quer
# 5. Same as #3 with LIMIT 10 for the returned page slice.


@override_config(ENABLE_PUBLIC_ID_LOGIC=True)
@override_settings(ENABLE_PUBLIC_ID_LOGIC=True)
def test_anon_can_query_users_list_with_optimized_query_with_public_id(
graphql_client_with_queries,
) -> None:
Expand Down
11 changes: 6 additions & 5 deletions baseapp_blocks/tests/test_graphql_queries_object_blocks.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
from django.test import override_settings

"""
Query-count tests for `BlocksInterface`: assert the GraphQL `blockersCount` /
`blockingCount` fields resolve in a flat number of DB queries regardless of how
Expand All @@ -6,7 +8,6 @@
"""

import pytest
from constance.test import override_config
from django.contrib.contenttypes.models import ContentType

from baseapp_core.graphql.utils import capture_database_queries
Expand Down Expand Up @@ -71,7 +72,7 @@
EXPECTED_BLOCKS_INTERFACE_NESTED_LIST_QUERY_COUNT = 9


@override_config(ENABLE_PUBLIC_ID_LOGIC=True)
@override_settings(ENABLE_PUBLIC_ID_LOGIC=True)
def test_block_counts_are_flat_regardless_of_block_volume(
django_user_client, graphql_user_client
) -> None:
Expand Down Expand Up @@ -107,7 +108,7 @@ def test_block_counts_are_flat_regardless_of_block_volume(
assert small_count == EXPECTED_BLOCKS_INTERFACE_QUERY_COUNT


@override_config(ENABLE_PUBLIC_ID_LOGIC=True)
@override_settings(ENABLE_PUBLIC_ID_LOGIC=True)
def test_block_counts_zero_when_no_blocks_does_not_extra_query(
django_user_client, graphql_user_client
) -> None:
Expand Down Expand Up @@ -141,7 +142,7 @@ def test_block_counts_zero_when_no_blocks_does_not_extra_query(
assert queries_empty.count == EXPECTED_BLOCKS_INTERFACE_QUERY_COUNT


@override_config(ENABLE_PUBLIC_ID_LOGIC=True)
@override_settings(ENABLE_PUBLIC_ID_LOGIC=True)
def test_block_counts_perm_denied_path_is_flat(graphql_user_client) -> None:
"""A non-superuser sees `null` for both counts and the perm-check path
stays flat regardless of block volume."""
Expand Down Expand Up @@ -172,7 +173,7 @@ def test_block_counts_perm_denied_path_is_flat(graphql_user_client) -> None:
assert queries_small.count <= EXPECTED_BLOCKS_INTERFACE_PERM_DENIED_UPPER_BOUND


@override_config(ENABLE_PUBLIC_ID_LOGIC=True)
@override_settings(ENABLE_PUBLIC_ID_LOGIC=True)
def test_block_list_with_target_blockers_count_is_flat(
django_user_client, graphql_user_client
) -> None:
Expand Down
27 changes: 13 additions & 14 deletions baseapp_comments/tests/test_graphql_queries_object_comments.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import pytest
from constance.test import override_config
from django.contrib.contenttypes.models import ContentType
from django.test import override_settings

Expand Down Expand Up @@ -141,7 +140,7 @@
"""


@override_config(ENABLE_PUBLIC_ID_LOGIC=True)
@override_settings(ENABLE_PUBLIC_ID_LOGIC=True)
def test_anon_see_comments_and_replies(django_user_client, graphql_client_with_queries) -> None:
target = CommentFactory()
user = django_user_client.user
Expand Down Expand Up @@ -186,7 +185,7 @@ def test_anon_see_comments_and_replies(django_user_client, graphql_client_with_q
# 13) 'SELECT "users_user"."id", "users_user"."first_name", (SELECT U0."public_id" AS "public_id" FROM "baseapp_core_documentid" U0 WHERE (U0."content_type_id" = 3272 AND U0."object_id" = ("users_user"."id"))) AS "mapped_public_id" FROM "users_user" WHERE ("users_user"."id") IN ((1129))'


@override_config(ENABLE_PUBLIC_ID_LOGIC=True)
@override_settings(ENABLE_PUBLIC_ID_LOGIC=True)
def test_anon_see_comments_and_replies_with_pagination(
django_user_client, graphql_client_with_queries
) -> None:
Expand Down Expand Up @@ -214,7 +213,7 @@ def test_anon_see_comments_and_replies_with_pagination(
assert queries.count == 13


@override_config(ENABLE_PUBLIC_ID_LOGIC=True)
@override_settings(ENABLE_PUBLIC_ID_LOGIC=True)
def test_top_level_comments_pagination_on_page_target(django_user_client, graphql_client) -> None:
"""Reproduces the FE bug: when using first=5 on the top-level comments
of a Page (matching the FE CommentsList query), hasNextPage must be True
Expand All @@ -235,7 +234,7 @@ def test_top_level_comments_pagination_on_page_target(django_user_client, graphq
assert comments_connection["pageInfo"]["hasNextPage"] is True


@override_config(ENABLE_PUBLIC_ID_LOGIC=True)
@override_settings(ENABLE_PUBLIC_ID_LOGIC=True)
def test_top_level_comments_pagination_on_comment_target(
django_user_client, graphql_client
) -> None:
Expand All @@ -256,7 +255,7 @@ def test_top_level_comments_pagination_on_comment_target(
assert comments_connection["pageInfo"]["hasNextPage"] is True


@override_config(ENABLE_PUBLIC_ID_LOGIC=True)
@override_settings(ENABLE_PUBLIC_ID_LOGIC=True)
def test_logged_user_replies_to_a_page_comment_has_next_page_true_when_more_than_5_replies(
django_user_client, graphql_user_client
) -> None:
Expand Down Expand Up @@ -287,7 +286,7 @@ def test_logged_user_replies_to_a_page_comment_has_next_page_true_when_more_than
assert comment_replies["pageInfo"]["hasNextPage"] is True


@override_config(ENABLE_PUBLIC_ID_LOGIC=True)
@override_settings(ENABLE_PUBLIC_ID_LOGIC=True)
def test_get_queryset_skips_filtering_only_when_hint_set(
django_user_client, graphql_user_client
) -> None:
Expand Down Expand Up @@ -348,7 +347,7 @@ def __init__(self, request) -> None:
assert result.count() == 3 # blocked user's comments excluded despite cache


@override_config(ENABLE_PUBLIC_ID_LOGIC=True)
@override_settings(ENABLE_PUBLIC_ID_LOGIC=True)
def test_blocked_profiles_excluded_with_pagination(django_user_client, graphql_user_client) -> None:
"""Blocked/blocking profiles are excluded and pagination still works correctly."""
page = PageFactory(user=django_user_client.user)
Expand Down Expand Up @@ -382,7 +381,7 @@ def test_blocked_profiles_excluded_with_pagination(django_user_client, graphql_u
assert comments_connection["pageInfo"]["endCursor"] is not None


@override_config(ENABLE_PUBLIC_ID_LOGIC=True)
@override_settings(ENABLE_PUBLIC_ID_LOGIC=True)
def test_top_level_comments_second_page_with_cursor(django_user_client, graphql_client) -> None:
"""Fetching the second page with an `after` cursor returns the remaining comments."""
page = PageFactory(user=django_user_client.user)
Expand Down Expand Up @@ -533,7 +532,7 @@ def test_anon_cant_see_comments(graphql_client) -> None:
assert len(content["data"]["node"]["comments"]["edges"]) == 0


@override_config(ENABLE_PUBLIC_ID_LOGIC=False)
@override_settings(ENABLE_PUBLIC_ID_LOGIC=False)
def test_comments_query_is_partially_optimized(
django_user_client, graphql_client_with_queries
) -> None:
Expand Down Expand Up @@ -572,7 +571,7 @@ def test_comments_query_is_partially_optimized(
# filters replies by status; this is the "partially optimized" part the test name refers to


@override_config(ENABLE_PUBLIC_ID_LOGIC=True)
@override_settings(ENABLE_PUBLIC_ID_LOGIC=True)
def test_comments_query_is_optimized_with_nested_replies(
django_user_client, graphql_client_with_queries
) -> None:
Expand Down Expand Up @@ -616,7 +615,7 @@ def test_comments_query_is_optimized_with_nested_replies(
assert queries.count == 12


@override_config(ENABLE_PUBLIC_ID_LOGIC=True)
@override_settings(ENABLE_PUBLIC_ID_LOGIC=True)
def test_comments_query_is_partially_optimized_with_public_id(
django_user_client, graphql_client_with_queries
) -> None:
Expand Down Expand Up @@ -656,7 +655,7 @@ def test_comments_query_is_partially_optimized_with_public_id(
# 11) 'SELECT "profiles_profile"."id", "profiles_profile"."name", (SELECT U0."public_id" FROM "baseapp_core_documentid" U0 WHERE (U0."content_type_id" = 28 AND U0."object_id" = ("profiles_profile"."id"))) AS "mapped_public_id" FROM "profiles_profile" WHERE "profiles_profile"."id" IN (736)'


@override_config(ENABLE_PUBLIC_ID_LOGIC=True)
@override_settings(ENABLE_PUBLIC_ID_LOGIC=True)
def test_comments_query_is_partially_optimized_with_public_id_and_pagination(
django_user_client, graphql_client_with_queries
) -> None:
Expand Down Expand Up @@ -686,7 +685,7 @@ def test_comments_query_is_partially_optimized_with_public_id_and_pagination(
assert queries.count == 12


@override_config(ENABLE_PUBLIC_ID_LOGIC=True)
@override_settings(ENABLE_PUBLIC_ID_LOGIC=True)
def test_comments_query_from_foreigh_target_is_partially_optimized_with_public_id(
django_user_client,
graphql_client_with_queries,
Expand Down
4 changes: 2 additions & 2 deletions baseapp_core/hashids/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ The key idea behind this structure is that each folder represents a different st

#### Legacy vs Public Id

The Legacy strategy follows the default behavior of Django, DRF, and Graphene. It relies on model primary keys to handle queries and updates coming from external services. If you decide to keep using primary keys in the frontend, which is not recommended, you can disable the Public Id strategy through the constance configuration `ENABLE_PUBLIC_ID_LOGIC`. When disabled, the system will rely exclusively on the Legacy strategy.
The Legacy strategy follows the default behavior of Django, DRF, and Graphene. It relies on model primary keys to handle queries and updates coming from external services. If you decide to keep using primary keys in the frontend, which is not recommended, you can disable the Public Id strategy with the Django setting `ENABLE_PUBLIC_ID_LOGIC = False`. When disabled, the system will rely exclusively on the Legacy strategy.

The Public Id strategy uses the `public_id` field from the `DocumentId` model. This strategy is responsible for resolving and exposing public IDs and is the recommended approach.

Expand All @@ -73,7 +73,7 @@ Public facing functions here are functions that other parts of the system are ex

Currently, the heuristics for selecting the Public Id strategy over the Legacy one are:

1. The Public Id strategy must be enabled through the constance configuration `ENABLE_PUBLIC_ID_LOGIC`.
1. The Public Id strategy must be enabled with the Django setting `ENABLE_PUBLIC_ID_LOGIC` (`True` by default).
2. The model being filtered must extend `DocumentIdMixin`.
3. The model being filtered must use an auto incrementing primary key.

Expand Down
9 changes: 7 additions & 2 deletions baseapp_core/hashids/strategies/__init__.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
from typing import TYPE_CHECKING, Any, Optional, Type

from constance import config
from django.conf import settings

from baseapp_core.hashids.models import LegacyWithPkMixin
from baseapp_core.hashids.strategies.bundle import HashidsStrategyBundle
Expand All @@ -13,7 +13,12 @@


def _is_public_id_logic_enabled() -> bool:
return bool(config.ENABLE_PUBLIC_ID_LOGIC)
"""`settings.ENABLE_PUBLIC_ID_LOGIC` (default True).

A deployment-level choice that never changes while the app runs, so it is a Django setting:
it is read for every ID resolved or rendered, and a setting costs no cache or database trip.
"""
return bool(getattr(settings, "ENABLE_PUBLIC_ID_LOGIC", True))


def _is_model_public_id_compatible(model_cls: type) -> bool:
Expand Down
28 changes: 16 additions & 12 deletions baseapp_core/hashids/tests/test_hashids_strategy_pickers.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
from unittest.mock import MagicMock, patch

import pytest
from constance.test import override_config
from django.test import override_settings
from graphql_relay import to_global_id

from baseapp_core.graphql.relay import Node
Expand Down Expand Up @@ -35,13 +35,17 @@

@pytest.mark.django_db
class TestPublicIdLogicEnabled:
def test_is_public_id_logic_enabled_returns_true_when_config_enabled(self) -> None:
with override_config(ENABLE_PUBLIC_ID_LOGIC=True):
assert _is_public_id_logic_enabled() is True
def test_enabled_by_the_setting(self, settings) -> None:
settings.ENABLE_PUBLIC_ID_LOGIC = True
assert _is_public_id_logic_enabled() is True

def test_is_public_id_logic_enabled_returns_false_when_config_disabled(self) -> None:
with override_config(ENABLE_PUBLIC_ID_LOGIC=False):
assert _is_public_id_logic_enabled() is False
def test_disabled_by_the_setting(self, settings) -> None:
settings.ENABLE_PUBLIC_ID_LOGIC = False
assert _is_public_id_logic_enabled() is False

def test_enabled_by_default(self, settings) -> None:
del settings.ENABLE_PUBLIC_ID_LOGIC
assert _is_public_id_logic_enabled() is True


@pytest.mark.django_db
Expand Down Expand Up @@ -312,7 +316,7 @@ def test_returns_none_when_pk_strategy_get_node_returns_none(self) -> None:
@pytest.mark.django_db
class TestHashidsStrategyIntegrationScenarios:
def test_public_id_model_with_public_id_logic_enabled_uses_public_id_strategy(self) -> None:
with override_config(ENABLE_PUBLIC_ID_LOGIC=True):
with override_settings(ENABLE_PUBLIC_ID_LOGIC=True):
dummy_instance = DummyPublicIdModelFactory()
strategy = get_hashids_strategy_from_instance_or_cls(dummy_instance)

Expand All @@ -326,7 +330,7 @@ def test_public_id_model_with_public_id_logic_enabled_uses_public_id_strategy(se
assert resolved_instance.pk == dummy_instance.pk

def test_public_id_model_with_public_id_logic_disabled_uses_legacy_strategy(self) -> None:
with override_config(ENABLE_PUBLIC_ID_LOGIC=False):
with override_settings(ENABLE_PUBLIC_ID_LOGIC=False):
dummy_instance = DummyPublicIdModelFactory()
strategy = get_hashids_strategy_from_instance_or_cls(dummy_instance)

Expand All @@ -343,7 +347,7 @@ def test_public_id_model_with_public_id_logic_disabled_uses_legacy_strategy(self

def test_legacy_model_always_uses_legacy_strategy(self) -> None:
for config_value in [True, False, None, ""]:
with override_config(ENABLE_PUBLIC_ID_LOGIC=config_value):
with override_settings(ENABLE_PUBLIC_ID_LOGIC=config_value):
dummy_instance = DummyLegacyModelFactory()
strategy = get_hashids_strategy_from_instance_or_cls(dummy_instance)

Expand All @@ -354,7 +358,7 @@ def test_legacy_model_always_uses_legacy_strategy(self) -> None:
assert resolved_id == dummy_instance.pk

def test_uuid4_global_id_flow_with_public_id_logic_enabled(self) -> None:
with override_config(ENABLE_PUBLIC_ID_LOGIC=True):
with override_settings(ENABLE_PUBLIC_ID_LOGIC=True):
dummy_instance = DummyPublicIdModelFactory()
test_uuid = str(dummy_instance.public_id)
graphene_type_mock = MagicMock()
Expand Down Expand Up @@ -392,7 +396,7 @@ def test_mixed_strategy_usage_in_same_test(self) -> None:
public_id_instance = DummyPublicIdModelFactory()
legacy_instance = DummyLegacyModelFactory()

with override_config(ENABLE_PUBLIC_ID_LOGIC=True):
with override_settings(ENABLE_PUBLIC_ID_LOGIC=True):
# Test public ID strategy
public_id_strategy = get_hashids_strategy_from_instance_or_cls(public_id_instance)
assert isinstance(public_id_strategy.id_resolver, PublicIdResolverStrategy)
Expand Down
4 changes: 2 additions & 2 deletions baseapp_core/tests/test_ids_resolver.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
from base64 import b64encode

import pytest
from constance.test import override_config
from django.test import override_settings

from baseapp_core.tests.factories import UserFactory

Expand All @@ -17,7 +17,7 @@
"""


@override_config(ENABLE_PUBLIC_ID_LOGIC=False)
@override_settings(ENABLE_PUBLIC_ID_LOGIC=False)
def test_get_node_with_old_1586_pk_issue(graphql_client) -> None:
# The global_id is created by a Base64-encoding string in the format "TypeName:ID".
# For certain IDs (like 1586), the resulting Base64 string will mislead how Graphene resolves the global_id to _type and _id. e.g. "1586" will be resolved as _type="ן" and _id="".
Expand Down
Loading
Loading