Skip to content
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 Linting Issues #209

Merged
merged 1 commit into from
Mar 5, 2024
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
1 change: 1 addition & 0 deletions aa_project/settings/pytest/pytest.py
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@
},
}


# Eliminates warning about missing staticfiles directory
WHITENOISE_AUTOREFRESH = True

Expand Down
5 changes: 2 additions & 3 deletions apps/api/serializers.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
from django.contrib.auth import get_user_model

from django.contrib.auth.models import User
from rest_framework import serializers

from apps.blog.models import Category, Post
Expand All @@ -10,7 +9,7 @@ class UserSerializer(serializers.ModelSerializer):
username = serializers.CharField(required=False, allow_blank=True, read_only=True)

class Meta:
model = get_user_model()
model = User
fields = (
"username",
"first_name",
Expand Down
5 changes: 2 additions & 3 deletions apps/blog/models.py
Original file line number Diff line number Diff line change
@@ -1,12 +1,11 @@
import math

from django.contrib.auth import get_user_model
from django.contrib.auth.models import User
from django.core.exceptions import ObjectDoesNotExist
from django.core.validators import MinLengthValidator
from django.db import models
from django.template.defaultfilters import slugify
from django.urls import reverse

from tinymce.models import HTMLField

from apps.blog.managers import PublishedManager
Expand Down Expand Up @@ -49,7 +48,7 @@ class Post(models.Model):
help_text="For bests results, use an image that is 1,200px wide x 600px high",
)
status = models.IntegerField(db_index=True, choices=STATUS, default=0)
author = models.ForeignKey(get_user_model(), related_name="author", on_delete=models.CASCADE)
author = models.ForeignKey(User, related_name="author", on_delete=models.CASCADE)
Copy link

Choose a reason for hiding this comment

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

suggestion (llm): This change impacts the Post model's relationship with the User model. It's essential to ensure that all tests related to the Post model, especially those that involve the author relationship, are updated to reflect this change. This includes updating any fixtures or mock objects used in these tests.

categories = models.ManyToManyField(
Category,
related_name="posts",
Expand Down
9 changes: 1 addition & 8 deletions apps/blog/tests/test_models.py
Original file line number Diff line number Diff line change
@@ -1,24 +1,17 @@
import datetime

from django.contrib.auth import get_user_model
Copy link

Choose a reason for hiding this comment

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

suggestion (llm): The removal of get_user_model import in this test file suggests that there were changes in how the user model is referenced in the tests. Please ensure that all instances where get_user_model() was used are updated to use the User model directly, and that these changes do not affect the integrity of the tests.

import pytest
from django.db import models
from django.shortcuts import reverse
from django.utils.text import slugify

import pytest

from mixer.backend.django import mixer
from tinymce.models import HTMLField

from apps.blog.models import Category, Post


pytestmark = pytest.mark.django_db(reset_sequences=True)


user_model = get_user_model()


class TestCategory:
def test_name_is_charfield(self):
category = mixer.blend(Category, pk=1)
Expand Down
4 changes: 2 additions & 2 deletions apps/blog/views.py
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
from time import perf_counter

from django.contrib import messages
from django.contrib.auth import get_user_model
from django.contrib.auth.mixins import LoginRequiredMixin, UserPassesTestMixin
from django.contrib.auth.models import User
from django.contrib.postgres.search import SearchQuery, SearchRank, SearchVector
from django.shortcuts import get_list_or_404, get_object_or_404
from django.urls import reverse, reverse_lazy
Expand Down Expand Up @@ -93,7 +93,7 @@ class AuthorPostListView(PostView):
paginate_orphans = 3

def get_queryset(self):
user = get_object_or_404(get_user_model(), username=self.kwargs["username"])
user = get_object_or_404(User, username=self.kwargs["username"])
return super().get_queryset().filter(author=user)

def get_context_data(self, **kwargs):
Expand Down
10 changes: 4 additions & 6 deletions apps/users/forms.py
Original file line number Diff line number Diff line change
@@ -1,9 +1,7 @@
from crispy_forms.helper import FormHelper
from django import forms
from django.contrib.auth import get_user_model
from django.contrib.auth.forms import UserCreationForm
from django.utils.translation import gettext_lazy as _

from crispy_forms.helper import FormHelper
from django.contrib.auth.models import User
from two_factor.forms import TOTPDeviceForm
from two_factor.utils import totp_digits

Expand All @@ -14,7 +12,7 @@ class UserRegisterForm(UserCreationForm):
email = forms.EmailField()

class Meta:
model = get_user_model()
model = User
fields = ["username", "email", "first_name", "last_name", "password1", "password2"]

def clean_username(self):
Expand All @@ -31,7 +29,7 @@ class UserUpdateForm(forms.ModelForm):
email = forms.EmailField()

class Meta:
model = get_user_model()
model = User
fields = ["username", "email", "first_name", "last_name"]

def clean_username(self):
Expand Down
9 changes: 3 additions & 6 deletions apps/users/models.py
Original file line number Diff line number Diff line change
@@ -1,13 +1,12 @@
from datetime import timedelta

from django.conf import settings
from django.contrib.auth import get_user_model
from django.contrib.auth.models import User
from django.core.exceptions import ObjectDoesNotExist
from django.db import models
from django.template.defaultfilters import slugify
from django.urls import reverse
from django.utils import timezone

from django_otp.util import random_hex
from encrypted_model_fields.fields import EncryptedCharField

Expand All @@ -29,7 +28,7 @@ class AuthorView(models.IntegerChoices):

# Relationship Fields
user = models.OneToOneField(
get_user_model(), primary_key=True, related_name="profile", on_delete=models.CASCADE
User, primary_key=True, related_name="profile", on_delete=models.CASCADE
Copy link

Choose a reason for hiding this comment

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

suggestion (llm): The change in the ForeignKey relationship to use User directly should prompt a review of the model tests, especially those involving the profile related name. Ensure that the tests reflect this change and that any mock or factory objects used in the tests are updated accordingly.

)

def __str__(self):
Expand Down Expand Up @@ -98,9 +97,7 @@ class EmailToken(models.Model):
token_expiration_timestamp = models.DateTimeField(null=True, blank=True)

# Relationship Fields
user = models.ForeignKey(
get_user_model(), related_name="user_email_tokens", on_delete=models.CASCADE
)
user = models.ForeignKey(User, related_name="user_email_tokens", on_delete=models.CASCADE)

def __repr__(self):
return f"<EmailToken(id={self.id}, user_id={self.user_id}, user='{self.user}')>"
Expand Down
26 changes: 13 additions & 13 deletions apps/users/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,10 @@

from django.conf import settings
from django.contrib import auth, messages
from django.contrib.auth import get_user_model
from django.contrib.auth.forms import AuthenticationForm
from django.contrib.auth.hashers import check_password
from django.contrib.auth.mixins import UserPassesTestMixin
from django.contrib.auth.models import User
from django.contrib.auth.views import (
PasswordResetCompleteView,
PasswordResetConfirmView,
Expand All @@ -20,7 +20,6 @@
from django.utils.safestring import mark_safe
from django.views.generic import CreateView, DetailView, TemplateView
from django.views.generic.edit import FormView

from shapeshifter.views import MultiModelFormView
from two_factor.forms import AuthenticationTokenForm
from two_factor.views.core import LoginView, SetupView
Expand Down Expand Up @@ -51,7 +50,7 @@ class UserRegisterView(CreateView):

def user_exists(self, form) -> bool:
username = form.data["username"]
Copy link

Choose a reason for hiding this comment

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

issue (llm): Directly referencing the User model instead of using get_user_model() reduces the flexibility of the user model. If the project ever needs to customize the user model, this change will necessitate refactoring all direct references to User.

user = get_user_model().objects.filter(username=username)
user = User.objects.filter(username=username)
Copy link

Choose a reason for hiding this comment

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

suggestion (llm): I noticed the change from get_user_model() to User directly. While this change might not directly impact functionality, it's crucial to ensure that all related unit tests are updated to reflect this change. Specifically, any mock objects or fixtures that rely on the user model should be reviewed to ensure consistency.

if user.exists():
return True

Expand Down Expand Up @@ -110,7 +109,7 @@ def build_html_content(self, user, token) -> str:
},
)

def email_two_factor_token(self, user: get_user_model(), token):
def email_two_factor_token(self, user: User, token):
Copy link

Choose a reason for hiding this comment

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

suggestion (llm): Given the changes in method signatures to use User directly, it's important to verify if the tests covering email_two_factor_token and related methods are still valid. Ensure that the tests accurately mock the User model and any of its interactions within these methods.

"""Sends email containing current token"""

subject = "Your One Time Token"
Expand Down Expand Up @@ -155,7 +154,8 @@ def login_user(self, user):

def handle_email_auth_user(self, user):
"""Handles the actions for processing an email authenticated user"""
if user_passes_auth := self.authenticate_user(user=user):
user_passes_auth = self.authenticate_user(user=user)
if user_passes_auth:
Comment on lines +157 to +158
Copy link

Choose a reason for hiding this comment

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

suggestion (rule): Sourcery has identified the following issue:

Suggested change
user_passes_auth = self.authenticate_user(user=user)
if user_passes_auth:
if user_passes_auth := self.authenticate_user(user=user):

self.login_user(user=user)
token = self.retrieve_token_from_db(user)
self.email_two_factor_token(user, token)
Expand All @@ -176,8 +176,8 @@ def post(self, *args, **kwargs):

# Scenario 1: The user does not exist in the DB
try:
user = get_user_model().objects.get(username=username)
except get_user_model().DoesNotExist:
user = User.objects.get(username=username)
except User.DoesNotExist:
self._add_user_does_not_exist_message()
return redirect(self.request.path_info)

Expand All @@ -202,7 +202,7 @@ def post(self, *args, **kwargs):
# enable the Django Two-Factor Auth package to handle
elif self.steps.current == "token":
user_pk = self.request.session["wizard_user_login_view"]["user_pk"]
user = get_user_model().objects.get(pk=user_pk)
user = User.objects.get(pk=user_pk)
if user.profile.is_two_factor_auth_by_token:
return super().post(*args, **kwargs)

Expand All @@ -224,7 +224,7 @@ class UserSetupEmailView(TemplateView):
template_name = "two_factor/setup_by_email.html"
success_url = reverse_lazy("blog:users:setup_email_token")

def store_token_in_db(self, user: get_user_model(), token: str):
def store_token_in_db(self, user: User, token: str):
"""Creates an email token object in the DB"""
EmailToken.objects.create(
challenge_email_address=user.email,
Expand All @@ -235,7 +235,7 @@ def store_token_in_db(self, user: get_user_model(), token: str):
user_id=user.id,
)

def build_html_content(self, user: get_user_model(), token: str) -> str:
def build_html_content(self, user: User, token: str) -> str:
""" " Specifies the email template and context variables"""
return render_to_string(
template_name="emails/token.html",
Expand All @@ -246,7 +246,7 @@ def build_html_content(self, user: get_user_model(), token: str) -> str:
},
)

def email_two_factor_token(self, user: get_user_model(), token: str):
def email_two_factor_token(self, user: User, token: str):
"""Sends email containing one-time token"""

subject = "Your One Time Token"
Expand Down Expand Up @@ -315,7 +315,7 @@ def build_html_content(self, user, token) -> str:
},
)

def email_two_factor_success(self, user: get_user_model(), token):
def email_two_factor_success(self, user: User, token):
"""Sends email containing one-time token"""

subject = "Two-Factor Authentication Successful"
Expand Down Expand Up @@ -365,7 +365,7 @@ class ProfileView(TwoFactorAuthUserMixin, DetailView):
template_name = "users/profile.html"

def get_object(self, queryset=None):
return get_object_or_404(get_user_model(), username=self.kwargs["username"])
return get_object_or_404(User, username=self.kwargs["username"])


class ProfileUpdateView(TwoFactorAuthUserMixin, UserPassesTestMixin, MultiModelFormView):
Expand Down
Loading