Skip to content

Respect 'using' in signals #200

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

Merged
merged 16 commits into from
Nov 18, 2022
Merged
Show file tree
Hide file tree
Changes from 8 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
16 changes: 11 additions & 5 deletions admin_interface/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,19 +17,23 @@ class Theme(models.Model):
@staticmethod
def post_migrate_handler(**kwargs):
del_cached_active_theme()
Theme.get_active_theme()
Theme.get_active_theme(**kwargs)

@staticmethod
def post_delete_handler(**kwargs):
del_cached_active_theme()
Theme.get_active_theme()
Theme.get_active_theme(**kwargs)

@staticmethod
def post_save_handler(instance, **kwargs):

del_cached_active_theme()
if instance.active:
Theme.objects.exclude(pk=instance.pk).update(active=False)
Theme.get_active_theme()
objs_manager = Theme.objects
if "using" in kwargs:
objs_manager = objs_manager.using(kwargs["using"])
Copy link
Owner

Choose a reason for hiding this comment

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

This condition could be avoided by just doing:

objs_database = kwargs.get("using", "default")
objs_manager = Theme.objects.using(objs_database)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Since I'll be replacing *args, **kwargs could be replaced by database="default", that can replace the kwargs.get`. Will still be removing the if check.

objs_manager.exclude(pk=instance.pk).update(active=False)
Theme.get_active_theme(**kwargs)

@staticmethod
def pre_save_handler(instance, **kwargs):
Expand All @@ -42,8 +46,10 @@ def pre_save_handler(instance, **kwargs):
pass

@staticmethod
def get_active_theme():
def get_active_theme(*args, **kwargs):
Copy link
Owner

Choose a reason for hiding this comment

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

We don't need *args here, and **kwargs could be replaced by database="default".

Copy link
Contributor Author

Choose a reason for hiding this comment

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

okay. I'll update all the references in the model to use database

objs_manager = Theme.objects
if "using" in kwargs:
objs_manager = objs_manager.using(kwargs["using"])
objs_active_qs = objs_manager.filter(active=True)
objs_active_ls = list(objs_active_qs)
objs_active_count = len(objs_active_ls)
Expand Down
11 changes: 11 additions & 0 deletions tests/settings.py
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,14 @@
"HOST": "",
"PORT": "",
},
"postgres_2": {
"ENGINE": "django.db.backends.postgresql_psycopg2",
"NAME": "admin_interface_2",
"USER": "postgres",
"PASSWORD": "postgres",
"HOST": "",
"PORT": "",
},
}

github_workflow = os.environ.get("GITHUB_WORKFLOW")
Expand All @@ -98,8 +106,11 @@
database_config["postgres"]["HOST"] = "127.0.0.1"
database_config["postgres"]["PORT"] = "5432"

replica_engine = "postgres_2" if database_engine == "postgres" else database_engine

DATABASES = {
"default": database_config.get(database_engine),
"replica": database_config.get(replica_engine),
}

USE_I18N = True
Expand Down
34 changes: 33 additions & 1 deletion tests/test_models.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,11 @@

import random
import shutil
from unittest import expectedFailure
from unittest.mock import Mock, patch

from django.conf import settings
from django.test import TestCase
from django.test import TestCase, TransactionTestCase

from admin_interface.models import Theme

Expand Down Expand Up @@ -88,3 +90,33 @@ def test_last_theme_activated_on_multiple_themes_activated(self):
def test_repr(self):
theme = Theme.get_active_theme()
self.assertEqual("{0}".format(theme), "Django")


class AdminInterfaceModelsMultiDBTestCase(TransactionTestCase):
databases = ["default", "replica"]

def setUp(self):
pass

def tearDown(self):
shutil.rmtree(settings.MEDIA_ROOT, ignore_errors=True)

def test_get_theme_from_default_db(self):
Theme.get_active_theme()

def test_get_theme_from_replica_db(self):
kwargs = {"using": "replica"}
replica_theme = Theme.get_active_theme(**kwargs)
assert "Default" not in replica_theme.name

def test_db_are_isolated(self):
Theme.objects.using("replica").create(name="Replica Active", active=True)
default_theme = Theme.get_active_theme()
kwargs = {"using": "replica"}
replica_theme = Theme.get_active_theme(**kwargs)
assert default_theme.name != replica_theme.name

@expectedFailure
def test_fail_for_wrong_db_defined_in_kwargs(self):
kwargs = {"using": "other"}
Theme.get_active_theme(**kwargs)