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

#2647: Small waffle flag refactor - [BACKUP] #2834

Merged
merged 2 commits into from
Sep 23, 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
16 changes: 4 additions & 12 deletions src/registrar/models/user.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
from django.contrib.auth.models import AbstractUser
from django.db import models
from django.db.models import Q
from django.http import HttpRequest

from registrar.models import DomainInformation, UserDomainRole
from registrar.models.utility.portfolio_helper import UserPortfolioPermissionChoices
Expand All @@ -14,6 +13,7 @@
from .verified_by_staff import VerifiedByStaff
from .domain import Domain
from .domain_request import DomainRequest
from registrar.utility.waffle import flag_is_active_for_user
from waffle.decorators import flag_is_active

from phonenumber_field.modelfields import PhoneNumberField # type: ignore
Expand Down Expand Up @@ -204,14 +204,10 @@ def has_any_domains_portfolio_permission(self, portfolio):
) or self._has_portfolio_permission(portfolio, UserPortfolioPermissionChoices.VIEW_MANAGED_DOMAINS)

def has_organization_requests_flag(self):
request = HttpRequest()
request.user = self
return flag_is_active(request, "organization_requests")
return flag_is_active_for_user(self, "organization_requests")

def has_organization_members_flag(self):
request = HttpRequest()
request.user = self
return flag_is_active(request, "organization_members")
return flag_is_active_for_user(self, "organization_members")

def has_view_members_portfolio_permission(self, portfolio):
# BEGIN
Expand Down Expand Up @@ -422,12 +418,8 @@ def check_portfolio_invitations_on_login(self):
for invitation in PortfolioInvitation.objects.filter(
email__iexact=self.email, status=PortfolioInvitation.PortfolioInvitationStatus.INVITED
):
# need to create a bogus request and assign user to it, in order to pass request
# to flag_is_active
request = HttpRequest()
request.user = self
only_single_portfolio = (
not flag_is_active(request, "multiple_portfolios") and self.get_first_portfolio() is None
not flag_is_active_for_user(self, "multiple_portfolios") and self.get_first_portfolio() is None
)
if only_single_portfolio or flag_is_active(None, "multiple_portfolios"):
try:
Expand Down
8 changes: 2 additions & 6 deletions src/registrar/models/user_portfolio_permission.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
from django.db import models
from django.forms import ValidationError
from django.http import HttpRequest
from waffle import flag_is_active
from registrar.utility.waffle import flag_is_active_for_user
from registrar.models.utility.portfolio_helper import UserPortfolioPermissionChoices, UserPortfolioRoleChoices
from .utility.time_stamped_model import TimeStampedModel
from django.contrib.postgres.fields import ArrayField
Expand Down Expand Up @@ -101,11 +100,8 @@ def clean(self):
# Check if a user is set without accessing the related object.
has_user = bool(self.user_id)
if self.pk is None and has_user:
# Have to create a bogus request to set the user and pass to flag_is_active
request = HttpRequest()
request.user = self.user
existing_permissions = UserPortfolioPermission.objects.filter(user=self.user)
if not flag_is_active(request, "multiple_portfolios") and existing_permissions.exists():
if not flag_is_active_for_user(self.user, "multiple_portfolios") and existing_permissions.exists():
raise ValidationError(
"Only one portfolio permission is allowed per user when multiple portfolios are disabled."
)
Expand Down
23 changes: 23 additions & 0 deletions src/registrar/tests/test_utilities.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
from django.test import TestCase
from registrar.models import User
from waffle.testutils import override_flag
from registrar.utility.waffle import flag_is_active_for_user


class FlagIsActiveForUserTest(TestCase):

def setUp(self):
# Set up a test user
self.user = User.objects.create_user(username="testuser")

@override_flag("test_flag", active=True)
def test_flag_active_for_user(self):
# Test that the flag is active for the user
is_active = flag_is_active_for_user(self.user, "test_flag")
self.assertTrue(is_active)

@override_flag("test_flag", active=False)
def test_flag_inactive_for_user(self):
# Test that the flag is inactive for the user
is_active = flag_is_active_for_user(self.user, "test_flag")
self.assertFalse(is_active)
2 changes: 2 additions & 0 deletions src/registrar/utility/email.py
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,8 @@ def send_templated_email(
def _can_send_email(to_address, bcc_address):
"""Raises an EmailSendingError if we cannot send an email. Does nothing otherwise."""

# testing below a global waffle flag which will not be associated with a user
# or with http request, so pass None to flag_is_active
if flag_is_active(None, "disable_email_sending"): # type: ignore
message = "Could not send email. Email sending is disabled due to flag 'disable_email_sending'."
raise EmailSendingError(message)
Expand Down
12 changes: 12 additions & 0 deletions src/registrar/utility/waffle.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
from django.http import HttpRequest
from waffle.decorators import flag_is_active


def flag_is_active_for_user(user, flag_name):
"""flag_is_active_for_user can be used when a waffle_flag may be
activated for a user, but the context of where the flag needs to
be tested does not have a request object available.
When the request is available, flag_is_active should be used."""
request = HttpRequest()
request.user = user
return flag_is_active(request, flag_name)
Loading