-
Notifications
You must be signed in to change notification settings - Fork 2.6k
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
FIX #18154 - Cannot change default table preferences for anonymous users #18544
FIX #18154 - Cannot change default table preferences for anonymous users #18544
Conversation
netbox/netbox/tables/tables.py
Outdated
@@ -64,6 +66,11 @@ def __init__(self, *args, user=None, **kwargs): | |||
selected_columns = None | |||
if user is not None and not isinstance(user, AnonymousUser): | |||
selected_columns = user.config.get(f"tables.{self.name}.columns") | |||
elif isinstance(user, AnonymousUser) and hasattr(settings, 'DEFAULT_USER_PREFERENCES'): | |||
default_user_preferences = settings.DEFAULT_USER_PREFERENCES | |||
default_table = default_user_preferences.get('tables', {}).get(self.name, {}).get('columns', {}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could probably simplify this to
selected_columns = settings.DEFAULT_USER_PREFERENCES.get('tables', {}).get(self.name, {}).get('columns')
Right? Or is there some logical case I'm missing?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We still need to check is settings have DEFAULT_USER_PREFERENCES
attribute, my first implementation I've missed that test and a lot of unit testa fails because the DEFAULT_USER_PREFERENCES
aren't a required configuration.
But the next parts is definitely simpler, gonna change to that format.
netbox/netbox/tables/tables.py
Outdated
@@ -13,6 +14,7 @@ | |||
from django.utils.translation import gettext_lazy as _ | |||
from django_tables2.data import TableQuerysetData | |||
|
|||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This extra line should be removed
Fixes: #18154