Skip to content

Commit

Permalink
Remove SAFE_APPS_TAGS_FEATURE_ENABLED flag (#524)
Browse files Browse the repository at this point in the history
Removes SAFE_APPS_TAGS_FEATURE_ENABLED as this feature is now being used by the clients in production
  • Loading branch information
fmrsabino authored May 6, 2022
1 parent 99a541b commit c995ca8
Show file tree
Hide file tree
Showing 5 changed files with 29 additions and 40 deletions.
3 changes: 0 additions & 3 deletions .env.example
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,6 @@
# uncommented option that means it's either mandatory to set it or it's being
# overwritten in development to make your life easier.

# Enables the tags feature for Safe Apps
SAFE_APPS_TAGS_FEATURE_ENABLED=false

# In development avoid writing out bytecode to __pycache__ directories.
PYTHONDONTWRITEBYTECODE=true

Expand Down
4 changes: 0 additions & 4 deletions src/config/settings.py
Original file line number Diff line number Diff line change
Expand Up @@ -225,7 +225,3 @@
allowed_csrf_origins.strip()
for allowed_csrf_origins in allowed_csrf_origins.split(",")
]

SAFE_APPS_TAGS_FEATURE_ENABLED = bool(
strtobool(os.getenv("SAFE_APPS_TAGS_FEATURE_ENABLED", "false"))
)
22 changes: 8 additions & 14 deletions src/safe_apps/admin.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
from typing import Any

from django.conf import settings
from django.contrib import admin
from django.db.models import Model, QuerySet

Expand Down Expand Up @@ -36,12 +35,9 @@ class SafeAppAdmin(admin.ModelAdmin[SafeApp]):
list_filter = (ChainIdFilter,)
search_fields = ("name", "url")
ordering = ("name",)
inlines = []

if settings.SAFE_APPS_TAGS_FEATURE_ENABLED:
inlines.append(
TagInline,
)
inlines = [
TagInline,
]


@admin.register(Provider)
Expand All @@ -58,10 +54,8 @@ class ClientAdmin(admin.ModelAdmin[Client]):
ordering = ("url",)


if settings.SAFE_APPS_TAGS_FEATURE_ENABLED:

@admin.register(Tag)
class TagAdmin(admin.ModelAdmin[Tag]):
list_display = ("name",)
search_fields = ("name",)
ordering = ("name",)
@admin.register(Tag)
class TagAdmin(admin.ModelAdmin[Tag]):
list_display = ("name",)
search_fields = ("name",)
ordering = ("name",)
10 changes: 0 additions & 10 deletions src/safe_apps/serializers.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,3 @@
from typing import Any

from django.conf import settings
from drf_yasg.utils import swagger_serializer_method
from rest_framework import serializers
from rest_framework.utils.serializer_helpers import ReturnDict
Expand Down Expand Up @@ -71,10 +68,3 @@ def get_access_control(self, instance: SafeApp) -> ReturnDict:
def get_tags(self, instance) -> ReturnDict: # type: ignore[no-untyped-def]
queryset = instance.tag_set.all().order_by("name")
return TagSerializer(queryset, many=True).data

def to_representation(self, instance: SafeApp) -> Any:
result = super().to_representation(instance)
if not settings.SAFE_APPS_TAGS_FEATURE_ENABLED:
result.pop("tags", None)

return result
30 changes: 21 additions & 9 deletions src/safe_apps/tests/test_views.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
from typing import Any, Dict, List

from django.test import override_settings
from django.urls import reverse
from rest_framework.test import APITestCase

Expand All @@ -17,7 +16,6 @@ def test_empty_set(self) -> None:
self.assertEqual(response.json(), [])


@override_settings(SAFE_APPS_TAGS_FEATURE_ENABLED=False)
class JsonPayloadFormatViewTests(APITestCase):
def test_json_payload_format(self) -> None:
safe_app = SafeAppFactory.create()
Expand All @@ -34,6 +32,7 @@ def test_json_payload_format(self) -> None:
"accessControl": {
"type": "NO_RESTRICTIONS",
},
"tags": [],
}
]
url = reverse("v1:safe-apps:list")
Expand All @@ -43,7 +42,6 @@ def test_json_payload_format(self) -> None:
self.assertEqual(response.status_code, 200)
self.assertCountEqual(response.json(), json_response)

@override_settings(SAFE_APPS_TAGS_FEATURE_ENABLED=True)
def test_tags_payload(self) -> None:
safe_app = SafeAppFactory.create()
tag = TagFactory.create(safe_apps=(safe_app,))
Expand All @@ -70,7 +68,6 @@ def test_tags_payload(self) -> None:
self.assertCountEqual(response.json(), json_response)


@override_settings(SAFE_APPS_TAGS_FEATURE_ENABLED=False)
class FilterSafeAppListViewTests(APITestCase):
def test_all_safes_returned(self) -> None:
(safe_app_1, safe_app_2, safe_app_3) = SafeAppFactory.create_batch(3)
Expand All @@ -86,6 +83,7 @@ def test_all_safes_returned(self) -> None:
"accessControl": {
"type": "NO_RESTRICTIONS",
},
"tags": [],
},
{
"id": safe_app_2.app_id,
Expand All @@ -98,6 +96,7 @@ def test_all_safes_returned(self) -> None:
"accessControl": {
"type": "NO_RESTRICTIONS",
},
"tags": [],
},
{
"id": safe_app_3.app_id,
Expand All @@ -110,6 +109,7 @@ def test_all_safes_returned(self) -> None:
"accessControl": {
"type": "NO_RESTRICTIONS",
},
"tags": [],
},
]
url = reverse("v1:safe-apps:list")
Expand All @@ -133,6 +133,7 @@ def test_all_apps_returned_on_empty_chain_id_value(self) -> None:
"accessControl": {
"type": "NO_RESTRICTIONS",
},
"tags": [],
},
{
"id": safe_app_2.app_id,
Expand All @@ -145,6 +146,7 @@ def test_all_apps_returned_on_empty_chain_id_value(self) -> None:
"accessControl": {
"type": "NO_RESTRICTIONS",
},
"tags": [],
},
{
"id": safe_app_3.app_id,
Expand All @@ -157,6 +159,7 @@ def test_all_apps_returned_on_empty_chain_id_value(self) -> None:
"accessControl": {
"type": "NO_RESTRICTIONS",
},
"tags": [],
},
]
url = reverse("v1:safe-apps:list") + f'{"?chainId="}'
Expand All @@ -182,6 +185,7 @@ def test_apps_returned_on_filtered_chain_id(self) -> None:
"accessControl": {
"type": "NO_RESTRICTIONS",
},
"tags": [],
},
{
"id": safe_app_5.app_id,
Expand All @@ -194,6 +198,7 @@ def test_apps_returned_on_filtered_chain_id(self) -> None:
"accessControl": {
"type": "NO_RESTRICTIONS",
},
"tags": [],
},
]
url = reverse("v1:safe-apps:list") + f'{"?chainId=1"}'
Expand Down Expand Up @@ -228,6 +233,7 @@ def test_apps_returned_on_same_chainid_key_pair(self) -> None:
"accessControl": {
"type": "NO_RESTRICTIONS",
},
"tags": [],
}
]
url = reverse("v1:safe-apps:list") + f'{"?chainId=2&chainId=1"}'
Expand Down Expand Up @@ -255,6 +261,7 @@ def test_apps_returned_on_non_existent_client_url(self) -> None:
"accessControl": {
"type": "NO_RESTRICTIONS",
},
"tags": [],
}
]
self.assertEqual(response.status_code, 200)
Expand All @@ -280,6 +287,7 @@ def test_apps_returned_on_empty_client_url(self) -> None:
"accessControl": {
"type": "NO_RESTRICTIONS",
},
"tags": [],
},
{
"id": safe_app_2.app_id,
Expand All @@ -293,6 +301,7 @@ def test_apps_returned_on_empty_client_url(self) -> None:
"type": "DOMAIN_ALLOWLIST",
"value": [client_1.url],
},
"tags": [],
},
]
self.assertEqual(response.status_code, 200)
Expand All @@ -316,6 +325,7 @@ def test_apps_returned_on_same_client_url_key_pair(self) -> None:
"type": "DOMAIN_ALLOWLIST",
"value": [client_1.url],
},
"tags": [],
}
]
url = (
Expand Down Expand Up @@ -353,6 +363,7 @@ def test_apps_returned_on_filtered_client_url(self) -> None:
"type": "DOMAIN_ALLOWLIST",
"value": ["safe.com"],
},
"tags": [],
},
{
"id": safe_app_3.app_id,
Expand All @@ -366,6 +377,7 @@ def test_apps_returned_on_filtered_client_url(self) -> None:
"type": "DOMAIN_ALLOWLIST",
"value": ["safe.com", "pump.com"],
},
"tags": [],
},
{
"id": safe_app_2.app_id,
Expand All @@ -378,6 +390,7 @@ def test_apps_returned_on_filtered_client_url(self) -> None:
"accessControl": {
"type": "NO_RESTRICTIONS",
},
"tags": [],
},
]
url = reverse("v1:safe-apps:list") + f'{"?clientUrl=safe.com"}'
Expand All @@ -388,7 +401,6 @@ def test_apps_returned_on_filtered_client_url(self) -> None:
self.assertCountEqual(response.json(), json_response)


@override_settings(SAFE_APPS_TAGS_FEATURE_ENABLED=False)
class ProviderInfoTests(APITestCase):
def test_provider_returned_in_response(self) -> None:
provider = ProviderFactory.create()
Expand All @@ -406,6 +418,7 @@ def test_provider_returned_in_response(self) -> None:
"accessControl": {
"type": "NO_RESTRICTIONS",
},
"tags": [],
}
]
url = reverse("v1:safe-apps:list")
Expand All @@ -430,6 +443,7 @@ def test_provider_not_returned_in_response(self) -> None:
"accessControl": {
"type": "NO_RESTRICTIONS",
},
"tags": [],
}
]
url = reverse("v1:safe-apps:list")
Expand All @@ -440,7 +454,6 @@ def test_provider_not_returned_in_response(self) -> None:
self.assertCountEqual(response.json(), json_response)


@override_settings(SAFE_APPS_TAGS_FEATURE_ENABLED=False)
class CacheSafeAppTests(APITestCase):
def test_should_cache_response(self) -> None:
safe_app_1 = SafeAppFactory.create()
Expand All @@ -457,6 +470,7 @@ def test_should_cache_response(self) -> None:
"accessControl": {
"type": "NO_RESTRICTIONS",
},
"tags": [],
}
]
url = reverse("v1:safe-apps:list")
Expand All @@ -471,7 +485,6 @@ def test_should_cache_response(self) -> None:
self.assertCountEqual(response.json(), json_response)


@override_settings(SAFE_APPS_TAGS_FEATURE_ENABLED=False)
class SafeAppsVisibilityTests(APITestCase):
def test_visible_safe_app_is_shown(self) -> None:
visible_safe_app = SafeAppFactory.create(visible=True)
Expand All @@ -487,6 +500,7 @@ def test_visible_safe_app_is_shown(self) -> None:
"accessControl": {
"type": "NO_RESTRICTIONS",
},
"tags": [],
}
]
url = reverse("v1:safe-apps:list")
Expand All @@ -507,7 +521,6 @@ def test_not_visible_safe_app_is_not_shown(self) -> None:
self.assertCountEqual(response.json(), json_response)


@override_settings(SAFE_APPS_TAGS_FEATURE_ENABLED=False)
class ClientTests(APITestCase):
def test_client_with_no_exclusive_apps(self) -> None:
SafeAppFactory.create()
Expand Down Expand Up @@ -539,7 +552,6 @@ def test_client_with_exclusive_apps(self) -> None:
self.assertEqual(json_response[0]["accessControl"]["value"], [client_1.url])


@override_settings(SAFE_APPS_TAGS_FEATURE_ENABLED=True)
class TagsTests(APITestCase):
def test_empty_tags(self) -> None:
SafeAppFactory.create()
Expand Down

0 comments on commit c995ca8

Please sign in to comment.