Merge pull request #2690 from carpentries/feature/2687-cleanup-featur…

…e-flags

[Emails] Cleanup feature flags

amy/dashboard/tests/test_instructor_recruitment_views.py

@@ -22,7 +22,7 @@
 
 
 class TestUpcomingTeachingOpportunitiesList(TestCase):
-    @override_settings(INSTRUCTOR_RECRUITMENT_ENABLED=True)
+    @override_settings(FLAGS={"INSTRUCTOR_RECRUITMENT": [("boolean", True)]})
     def test_view_enabled__no_community_role(self):
         # Arrange
         request = RequestFactory().get("/")
@@ -32,9 +32,9 @@ def test_view_enabled__no_community_role(self):
         # Act
         view = UpcomingTeachingOpportunitiesList(request=request)
         # Assert
-        self.assertEqual(view.get_view_enabled(), False)
+        self.assertEqual(view.get_view_enabled(request), False)
 
-    @override_settings(INSTRUCTOR_RECRUITMENT_ENABLED=True)
+    @override_settings(FLAGS={"INSTRUCTOR_RECRUITMENT": [("boolean", True)]})
     def test_view_enabled__community_role_inactive(self):
         # Arrange
         request = RequestFactory().get("/")
@@ -59,9 +59,9 @@ def test_view_enabled__community_role_inactive(self):
         view = UpcomingTeachingOpportunitiesList(request=request)
         # Assert
         self.assertEqual(role.is_active(), False)
-        self.assertEqual(view.get_view_enabled(), False)
+        self.assertEqual(view.get_view_enabled(request), False)
 
-    @override_settings(INSTRUCTOR_RECRUITMENT_ENABLED=True)
+    @override_settings(FLAGS={"INSTRUCTOR_RECRUITMENT": [("boolean", True)]})
     def test_view_enabled__community_role_active(self):
         # Arrange
         request = RequestFactory().get("/")
@@ -84,7 +84,7 @@ def test_view_enabled__community_role_active(self):
         view = UpcomingTeachingOpportunitiesList(request=request)
         # Assert
         self.assertEqual(role.is_active(), True)
-        self.assertEqual(view.get_view_enabled(), True)
+        self.assertEqual(view.get_view_enabled(request), True)
 
     def test_get_queryset(self):
         # Arrange
@@ -201,7 +201,7 @@ def test_get_context_data(self):
 
 
 class TestSignupForRecruitment(TestCase):
-    @override_settings(INSTRUCTOR_RECRUITMENT_ENABLED=True)
+    @override_settings(FLAGS={"INSTRUCTOR_RECRUITMENT": [("boolean", True)]})
     def test_view_enabled__no_community_role(self):
         # Arrange
         request = RequestFactory().get("/")
@@ -211,9 +211,9 @@ def test_view_enabled__no_community_role(self):
         # Act
         view = SignupForRecruitment(request=request)
         # Assert
-        self.assertEqual(view.get_view_enabled(), False)
+        self.assertEqual(view.get_view_enabled(request), False)
 
-    @override_settings(INSTRUCTOR_RECRUITMENT_ENABLED=True)
+    @override_settings(FLAGS={"INSTRUCTOR_RECRUITMENT": [("boolean", True)]})
     def test_view_enabled__community_role_inactive(self):
         # Arrange
         request = RequestFactory().get("/")
@@ -238,9 +238,9 @@ def test_view_enabled__community_role_inactive(self):
         view = SignupForRecruitment(request=request)
         # Assert
         self.assertEqual(role.is_active(), False)
-        self.assertEqual(view.get_view_enabled(), False)
+        self.assertEqual(view.get_view_enabled(request), False)
 
-    @override_settings(INSTRUCTOR_RECRUITMENT_ENABLED=True)
+    @override_settings(FLAGS={"INSTRUCTOR_RECRUITMENT": [("boolean", True)]})
     def test_view_enabled__community_role_active(self):
         # Arrange
         request = RequestFactory().get("/")
@@ -263,7 +263,7 @@ def test_view_enabled__community_role_active(self):
         view = SignupForRecruitment(request=request)
         # Assert
         self.assertEqual(role.is_active(), True)
-        self.assertEqual(view.get_view_enabled(), True)
+        self.assertEqual(view.get_view_enabled(request), True)
 
     def test_get_context_data(self):
         # Arrange
@@ -505,7 +505,7 @@ def test_form_valid__creates_a_new_signup(self, mock_contrib_messages_views):
 
 
 class TestResignFromRecruitment(TestCase):
-    @override_settings(INSTRUCTOR_RECRUITMENT_ENABLED=True)
+    @override_settings(FLAGS={"INSTRUCTOR_RECRUITMENT": [("boolean", True)]})
     def test_view_enabled__no_community_role(self):
         # Arrange
         request = RequestFactory().post("/")
@@ -515,9 +515,9 @@ def test_view_enabled__no_community_role(self):
         # Act
         view = ResignFromRecruitment(request=request)
         # Assert
-        self.assertEqual(view.get_view_enabled(), False)
+        self.assertEqual(view.get_view_enabled(request), False)
 
-    @override_settings(INSTRUCTOR_RECRUITMENT_ENABLED=True)
+    @override_settings(FLAGS={"INSTRUCTOR_RECRUITMENT": [("boolean", True)]})
     def test_view_enabled__community_role_inactive(self):
         # Arrange
         request = RequestFactory().post("/")
@@ -542,9 +542,9 @@ def test_view_enabled__community_role_inactive(self):
         view = ResignFromRecruitment(request=request)
         # Assert
         self.assertEqual(role.is_active(), False)
-        self.assertEqual(view.get_view_enabled(), False)
+        self.assertEqual(view.get_view_enabled(request), False)
 
-    @override_settings(INSTRUCTOR_RECRUITMENT_ENABLED=True)
+    @override_settings(FLAGS={"INSTRUCTOR_RECRUITMENT": [("boolean", True)]})
     def test_view_enabled__community_role_active(self):
         # Arrange
         request = RequestFactory().post("/")
@@ -567,7 +567,7 @@ def test_view_enabled__community_role_active(self):
         view = ResignFromRecruitment(request=request)
         # Assert
         self.assertEqual(role.is_active(), True)
-        self.assertEqual(view.get_view_enabled(), True)
+        self.assertEqual(view.get_view_enabled(request), True)
 
     def test_get_queryset(self):
         # Arrange

amy/dashboard/views.py

@@ -2,6 +2,7 @@
 import re
 from urllib.parse import unquote
 
+from django.conf import settings
 from django.contrib import messages
 from django.contrib.auth.mixins import LoginRequiredMixin
 from django.db.models import (
@@ -24,6 +25,7 @@
 from django.views.generic.detail import SingleObjectMixin
 from django_comments.models import Comment
 from flags.sources import get_flags
+from flags.views import FlaggedViewMixin
 
 from communityroles.models import CommunityRole
 from consents.forms import TermBySlugsForm
@@ -41,7 +43,6 @@
 from extrequests.base_views import AMYCreateAndFetchObjectView
 from fiscal.models import MembershipTask
 from recruitment.models import InstructorRecruitment, InstructorRecruitmentSignup
-from recruitment.views import RecruitmentEnabledMixin
 from workshops.base_views import (
     AMYCreateView,
     AMYDeleteView,
@@ -335,8 +336,9 @@ def get_queryset(self) -> QuerySet[TrainingProgress]:
 
 
 class UpcomingTeachingOpportunitiesList(
-    LoginRequiredMixin, RecruitmentEnabledMixin, ConditionallyEnabledMixin, AMYListView
+    LoginRequiredMixin, FlaggedViewMixin, ConditionallyEnabledMixin, AMYListView
 ):
+    flag_name = "INSTRUCTOR_RECRUITMENT"
     permission_required = "recruitment.view_instructorrecruitment"
     title = "Upcoming Teaching Opportunities"
     template_name = "dashboard/upcoming_teaching_opportunities.html"
@@ -373,12 +375,12 @@ def get_queryset(self):
         )
         return super().get_queryset()
 
-    def get_view_enabled(self) -> bool:
+    def get_view_enabled(self, request) -> bool:
         try:
             role = CommunityRole.objects.get(
                 person=self.request.user, config__name="instructor"
             )
-            return role.is_active() and super().get_view_enabled()
+            return role.is_active()
         except CommunityRole.DoesNotExist:
             return False
 
@@ -411,10 +413,11 @@ def get_context_data(self, **kwargs):
 
 class SignupForRecruitment(
     LoginRequiredMixin,
-    RecruitmentEnabledMixin,
+    FlaggedViewMixin,
     ConditionallyEnabledMixin,
     AMYCreateAndFetchObjectView,
 ):
+    flag_name = "INSTRUCTOR_RECRUITMENT"
     permission_required = [
         "recruitment.view_instructorrecruitment",
         "recruitment.add_instructorrecruitmentsignup",
@@ -430,12 +433,12 @@ class SignupForRecruitment(
     form_class = SignupForRecruitmentForm
     template_name = "dashboard/signup_for_recruitment.html"
 
-    def get_view_enabled(self) -> bool:
+    def get_view_enabled(self, request) -> bool:
         try:
             role = CommunityRole.objects.get(
                 person=self.request.user, config__name="instructor"
             )
-            return role.is_active() and super().get_view_enabled()
+            return role.is_active()
         except CommunityRole.DoesNotExist:
             return False
 
@@ -527,11 +530,12 @@ def form_valid(self, form):
 
 class ResignFromRecruitment(
     LoginRequiredMixin,
-    RecruitmentEnabledMixin,
+    FlaggedViewMixin,
     ConditionallyEnabledMixin,
     SingleObjectMixin,
     View,
 ):
+    flag_name = "INSTRUCTOR_RECRUITMENT"
     permission_required = [
         "recruitment.view_instructorrecruitmentsignup",
         "recruitment.delete_instructorrecruitmentsignup",
@@ -554,12 +558,12 @@ def post(self, request, *args, **kwargs):
         redirect_url = self.get_redirect_url()
         return redirect(redirect_url)
 
-    def get_view_enabled(self) -> bool:
+    def get_view_enabled(self, request) -> bool:
         try:
             role = CommunityRole.objects.get(
                 person=self.request.user, config__name="instructor"
             )
-            return role.is_active() and super().get_view_enabled()
+            return role.is_active()
         except CommunityRole.DoesNotExist:
             return False
 
@@ -744,9 +748,14 @@ def search(request):
 # ------------------------------------------------------------
 
 
-class AllFeatureFlags(LoginRequiredMixin, TemplateView):
+class AllFeatureFlags(ConditionallyEnabledMixin, LoginRequiredMixin, TemplateView):
     template_name = "dashboard/all_feature_flags.html"
 
+    def get_view_enabled(self, request) -> bool:
+        return bool(
+            settings.PROD_ENVIRONMENT and request.user.is_superuser
+        ) or not bool(settings.PROD_ENVIRONMENT)
+
     def get(self, request: HttpRequest, *args, **kwargs):
         self.request = request
         return super().get(request, *args, **kwargs)

amy/emails/tests/actions/test_admin_signs_instructor_up_for_workshop.py

@@ -222,8 +222,12 @@ def test_missing_template(self, mock_messages_missing_template: MagicMock) -> No
 
 
 class TestAdminSignsInstructorUpForWorkshopReceiverIntegration(TestBase):
-    @override_settings(INSTRUCTOR_RECRUITMENT_ENABLED=True)
-    @override_settings(FLAGS={"EMAIL_MODULE": [("boolean", True)]})
+    @override_settings(
+        FLAGS={
+            "INSTRUCTOR_RECRUITMENT": [("boolean", True)],
+            "EMAIL_MODULE": [("boolean", True)],
+        }
+    )
     @patch("django.contrib.messages.views.messages")
     @patch("emails.actions.base_action.messages_action_scheduled")
     def test_integration(

amy/emails/tests/actions/test_instructor_confirmed_for_workshop_receiver.py

@@ -224,8 +224,12 @@ def test_missing_template(self, mock_messages_missing_template: MagicMock) -> No
 
 
 class TestInstructorConfirmedForWorkshopReceiverIntegration(TestBase):
-    @override_settings(INSTRUCTOR_RECRUITMENT_ENABLED=True)
-    @override_settings(FLAGS={"EMAIL_MODULE": [("boolean", True)]})
+    @override_settings(
+        FLAGS={
+            "INSTRUCTOR_RECRUITMENT": [("boolean", True)],
+            "EMAIL_MODULE": [("boolean", True)],
+        }
+    )
     @patch("django.contrib.messages.views.messages")
     @patch("emails.actions.base_action.messages_action_scheduled")
     def test_integration(

amy/emails/tests/actions/test_instructor_declined_from_workshop.py

@@ -222,8 +222,12 @@ def test_missing_template(self, mock_messages_missing_template: MagicMock) -> No
 
 
 class TestInstructorDeclinedFromWorkshopReceiverIntegration(TestBase):
-    @override_settings(INSTRUCTOR_RECRUITMENT_ENABLED=True)
-    @override_settings(FLAGS={"EMAIL_MODULE": [("boolean", True)]})
+    @override_settings(
+        FLAGS={
+            "INSTRUCTOR_RECRUITMENT": [("boolean", True)],
+            "EMAIL_MODULE": [("boolean", True)],
+        }
+    )
     @patch("django.contrib.messages.views.messages")
     @patch("emails.actions.base_action.messages_action_scheduled")
     def test_integration(

amy/emails/tests/actions/test_instructor_signs_up_for_workshop.py

@@ -222,8 +222,12 @@ def test_missing_template(self, mock_messages_missing_template: MagicMock) -> No
 
 
 class TestInstructorSignsUpForWorkshopReceiverIntegration(TestBase):
-    @override_settings(INSTRUCTOR_RECRUITMENT_ENABLED=True)
-    @override_settings(FLAGS={"EMAIL_MODULE": [("boolean", True)]})
+    @override_settings(
+        FLAGS={
+            "INSTRUCTOR_RECRUITMENT": [("boolean", True)],
+            "EMAIL_MODULE": [("boolean", True)],
+        }
+    )
     @patch("django.contrib.messages.views.messages")
     @patch("emails.actions.base_action.messages_action_scheduled")
     def test_integration(

amy/recruitment/templatetags/instructorrecruitment.py

@@ -2,7 +2,6 @@
 from typing import Sequence, Union
 
 from django import template
-from django.conf import settings
 
 from recruitment.models import (
     InstructorRecruitment,
@@ -14,14 +13,6 @@
 register = template.Library()
 
 
-@register.simple_tag
-def is_instructor_recruitment_enabled() -> bool:
-    try:
-        return bool(settings.INSTRUCTOR_RECRUITMENT_ENABLED)
-    except AttributeError:
-        return False
-
-
 @register.simple_tag
 def get_event_conflicts(events_to_check: Sequence[Event], event: Event) -> list[Event]:
     conflicts: list[Event] = []

amy/recruitment/tests/test_template_tags.py

@@ -1,7 +1,6 @@
 from datetime import date
 
-from django.conf import settings
-from django.test import TestCase, override_settings
+from django.test import TestCase
 
 from recruitment.models import (
     InstructorRecruitment,
@@ -12,25 +11,12 @@
     get_event_conflicts,
     get_events_nearby,
     get_signup_conflicts,
-    is_instructor_recruitment_enabled,
     priority_label,
 )
 from workshops.models import Event
 
 
 class TestInstructorRecruitmentTemplateTags(TestCase):
-    def test_feature_flag_enabled(self) -> None:
-        with self.settings(INSTRUCTOR_RECRUITMENT_ENABLED=False):
-            self.assertEqual(is_instructor_recruitment_enabled(), False)
-        with self.settings(INSTRUCTOR_RECRUITMENT_ENABLED=True):
-            self.assertEqual(is_instructor_recruitment_enabled(), True)
-
-    @override_settings()
-    def test_feature_flag_removed(self) -> None:
-        del settings.INSTRUCTOR_RECRUITMENT_ENABLED
-
-        self.assertEqual(is_instructor_recruitment_enabled(), False)
-
     def test_get_event_conflicts(self) -> None:
         # Arrange
         event1 = Event(