EL-1452: Remove feature flag & redundant code

- update css - remove old templates - update templates - removing redundant config - update tests - remove environment variables - remove `behave` step in cicleci - current tests provide same/better coverage
ministryofjustice · Jul 8, 2024 · 9e8d254 · 9e8d254
1 parent 644c246
commit 9e8d254
Show file tree

Hide file tree

Showing 24 changed files with 21 additions and 785 deletions.
diff --git a/.circleci/config.yml b/.circleci/config.yml
@@ -1,7 +1,6 @@
 version: 2.1
 orbs:
   slack: circleci/slack@2.5.0
-  cla-end-to-end-tests: ministryofjustice/cla-end-to-end-tests@volatile
   aws-cli: circleci/aws-cli@4.1 # use v4 of this orb
   aws-ecr: circleci/aws-ecr@9 # this orb doesn't support OIDC v2, so we use aws-cli to authenticate
 
@@ -215,20 +214,6 @@ jobs:
             bin/deploy.sh << parameters.environment >>
       - slack/status
 
-  behave:
-    executor: aws-ecr/default
-    steps:
-      - checkout:
-          path: fala
-      - run: |
-          cd fala
-          source .circleci/define_build_environment_variables
-          echo "export FALA_IMAGE=$ECR_DEPLOY_IMAGE" >> $BASH_ENV
-          echo "export A11Y_ENABLED=true" >> $BASH_ENV
-          echo "export FALA_TESTS_ONLY=true" >> $BASH_ENV
-          echo "Setting FALA image $ECR_DEPLOY_IMAGE"
-      - cla-end-to-end-tests/behave
-
   playwright:
     docker:
       - image: cimg/python:3.12
@@ -279,10 +264,6 @@ workflows:
          requires:
            - build
          context: laa-fala
-      - behave:
-          requires:
-            - build
-          context: laa-fala
 
       - deploy_with_helm:
           name: UAT Ephemeral deploy

diff --git a/.env.example b/.env.example
@@ -4,8 +4,6 @@ DEBUG = True
 ALLOWED_HOSTS = ["localhost", '127.0.0.1']
 LAALAA_API_HOST = "https://laa-legal-adviser-api-staging.apps.live-1.cloud-platform.service.justice.gov.uk"
 ENVIRONMENT = "dev"
-FEATURE_FLAG_NO_MAP = False
-FEATURE_FLAG_SURVEY_MONKEY = False
 
 # When using virtual.env, change this to "127.0.0.1". When using docker use "db".
 DB_HOST = "db"

diff --git a/fala/apps/adviser/forms.py b/fala/apps/adviser/forms.py
@@ -92,16 +92,9 @@ def clean(self):
         if not postcode and not data.get("name"):
             raise forms.ValidationError(_("Enter a postcode or an organisation name"))
         else:
-            if settings.FEATURE_FLAG_NO_MAP:
-                if postcode and self.region == Region.ENGLAND_OR_WALES and not self._valid_postcode(postcode):
-                    self.add_error("postcode", (_("Enter a valid postcode")))
-            else:
-                region = self.region
-                if region != Region.ENGLAND_OR_WALES:
-                    region_error = self.REGION_NAMES[region]
-                    msg1 = "This service does not cover "
-                    msg2 = "Try a postcode, town or city in England or Wales."
-                    self.add_error("postcode", "%s %s" % (_(" ".join((msg1, region_error))), _(msg2)))
+            if postcode and self.region == Region.ENGLAND_OR_WALES and not self._valid_postcode(postcode):
+                self.add_error("postcode", (_("Enter a valid postcode")))
+
         return data
 
     @property

diff --git a/fala/apps/adviser/tests/test_crown_dependencies_view.py b/fala/apps/adviser/tests/test_crown_dependencies_view.py
@@ -1,9 +1,8 @@
-from django.test import SimpleTestCase, Client, override_settings
+from django.test import SimpleTestCase, Client
 from django.urls import reverse
 import bs4
 
 
-@override_settings(FEATURE_FLAG_NO_MAP=True)
 class PostcodeValidationTest(SimpleTestCase):
     client = Client()
     url = reverse("search")
@@ -45,7 +44,6 @@ def test_other_region_form_and_change_search_button_visible(self):
         self.assertEqual(change_search_button.text.strip(), "Change search")
 
 
-@override_settings(FEATURE_FLAG_NO_MAP=True)
 class InvalidEnglishPostcodeTest(SimpleTestCase):
     client = Client()
     url = reverse("search")

diff --git a/fala/apps/adviser/tests/test_search_view_function.py b/fala/apps/adviser/tests/test_search_view_function.py
@@ -1,9 +1,8 @@
 import bs4
-from django.test import SimpleTestCase, Client, override_settings
+from django.test import SimpleTestCase, Client
 from django.urls import reverse
 
 
-@override_settings(FEATURE_FLAG_NO_MAP=True)
 class SearchViewFunctionTest(SimpleTestCase):
     client = Client()
 
@@ -14,7 +13,6 @@ def test_invalid_postcode_generates_error(self):
         self.assertEqual({"postcode": ["Enter a valid postcode"]}, response.context_data["form"].errors)
 
 
-@override_settings(FEATURE_FLAG_NO_MAP=True)
 class ResultsPageWithBothOrgAndPostcodeTest(SimpleTestCase):
     client = Client()
     url = reverse("search")
@@ -65,7 +63,6 @@ def test_category_search_has_no_previous_button(self):
         self.assertNotContains(response, '<div class="govuk-pagination__previous">')
 
 
-@override_settings(FEATURE_FLAG_NO_MAP=True)
 class ResultsPageWithJustPostcodeTest(SimpleTestCase):
     client = Client()
     url = reverse("search")
@@ -105,7 +102,6 @@ def test_search_parameters_box_contains_only_postcode_and_categories(self):
         self.assertEqual(content, "Postcode: PE30Categories: Debt Change search")
 
 
-@override_settings(FEATURE_FLAG_NO_MAP=True)
 class ResearchBannerTest(SimpleTestCase):
     client = Client()
     url = reverse("search")
@@ -114,18 +110,11 @@ class ResearchBannerTest(SimpleTestCase):
 
     research_message = "Help improve this legal adviser search"
 
-    @override_settings(FEATURE_FLAG_SURVEY_MONKEY=True)
-    def test_research_banner_present_when_flag_set(self):
+    def test_research_banner_present(self):
         response = self.client.get(self.url, self.data)
         self.assertContains(response, self.research_message)
 
-    @override_settings(FEATURE_FLAG_SURVEY_MONKEY=False)
-    def test_research_banner_absent_when_flag_unset(self):
-        response = self.client.get(self.url, self.data)
-        self.assertNotContains(response, self.research_message)
-
 
-@override_settings(FEATURE_FLAG_NO_MAP=True)
 class ResultsPageWithJustOrgTest(SimpleTestCase):
     client = Client()
     url = reverse("search")
@@ -141,7 +130,6 @@ def test_search_parameters_box_contains_only_organisation_and_categories(self):
         self.assertEqual(content, "Organisation: fooCategories: Debt, Education Change search")
 
 
-@override_settings(FEATURE_FLAG_NO_MAP=True)
 class NewSearchViewTemplate(SimpleTestCase):
     client = Client()
 
@@ -157,7 +145,6 @@ def test_template_link_and_css(self):
         self.assertContains(response, "laa-fala__grey-box")
 
 
-@override_settings(FEATURE_FLAG_NO_MAP=True)
 class NoResultsTest(SimpleTestCase):
     client = Client()
 
@@ -174,13 +161,6 @@ class AccessibilityViewTest(SimpleTestCase):
 
     url = reverse("accessibility_statement")
 
-    @override_settings(FEATURE_FLAG_NO_MAP=True)
     def test_shows_new_accessibility_statement(self):
         response = self.client.get(self.url)
         self.assertContains(response, "Accessibility statement for Find a legal advisor")
-
-    @override_settings(FEATURE_FLAG_NO_MAP=False)
-    def test_shows_old_accessibility_statement(self):
-        response = self.client.get(self.url)
-        self.assertContains(response, "Accessibility statement")
-        self.assertNotContains(response, "Accessibility statement for Find a legal advisor")
diff --git a/fala/apps/adviser/views.py b/fala/apps/adviser/views.py
@@ -10,12 +10,7 @@
 
 
 class AdviserView(TemplateView):
-    def get_template_names(self) -> list[str]:
-        if settings.FEATURE_FLAG_NO_MAP:
-            template_name = "adviser/search.html"
-        else:
-            template_name = "adviser/search_old.html"
-        return [template_name]
+    template_name = "adviser/search.html"
 
     def get(self, request, *args, **kwargs):
         context = self.get_context_data(**kwargs)
@@ -28,8 +23,6 @@ def get(self, request, *args, **kwargs):
                 "form": form,
                 "data": form.search(),
                 "current_url": current_url,
-                "GOOGLE_MAPS_API_KEY": settings.GOOGLE_MAPS_API_KEY,
-                "FEATURE_FLAG_NO_MAP": settings.FEATURE_FLAG_NO_MAP,
                 "CHECK_LEGAL_AID_URL": settings.CHECK_LEGAL_AID_URL,
             }
         )
@@ -38,12 +31,7 @@ def get(self, request, *args, **kwargs):
 
 
 class AccessibilityView(TemplateView):
-    def get_template_names(self) -> list[str]:
-        if settings.FEATURE_FLAG_NO_MAP:
-            template_name = "adviser/accessibility_statement.html"
-        else:
-            template_name = "adviser/accessibility-statement_old.html"
-        return [template_name]
+    template_name = "adviser/accessibility_statement.html"
 
 
 class PrivacyView(TemplateView):
@@ -90,7 +78,6 @@ def get_context_data(self):
                 "data": self._data,
                 "pages": pages,
                 "params": params,
-                "FEATURE_FLAG_SURVEY_MONKEY": settings.FEATURE_FLAG_SURVEY_MONKEY,
                 "categories": categories,
                 "category_selection": self._display_category(),
             }
@@ -103,28 +90,6 @@ def _display_category(self):
                 return formatted_categories
             return []
 
-    class OldMapState(object):
-        def __init__(self, form, current_url):
-            self.current_url = current_url
-            self.form = form
-
-        def get_queryset(self):
-            return []
-
-        @property
-        def template_name(self):
-            return "search_old.html"
-
-        def get_context_data(self):
-            return {
-                "form": self.form,
-                "data": self.form.search(),
-                "current_url": self.current_url,
-                "GOOGLE_MAPS_API_KEY": settings.GOOGLE_MAPS_API_KEY,
-                "FEATURE_FLAG_NO_MAP": settings.FEATURE_FLAG_NO_MAP,
-                "CHECK_LEGAL_AID_URL": settings.CHECK_LEGAL_AID_URL,
-            }
-
     class OtherJurisdictionState(object):
         REGION_TO_LINK = {
             Region.NI: {
@@ -166,19 +131,15 @@ def get_context_data(self):
 
     def get(self, request, *args, **kwargs):
         form = AdviserSearchForm(data=request.GET or None)
-        if settings.FEATURE_FLAG_NO_MAP:
-            if form.is_valid():
-                region = form.region
-                if region == Region.ENGLAND_OR_WALES or region == Region.SCOTLAND:
-                    self.state = self.EnglandOrWalesState(form)
-                else:
-                    self.state = self.OtherJurisdictionState(region, form.cleaned_data["postcode"])
+        if form.is_valid():
+            region = form.region
+            if region == Region.ENGLAND_OR_WALES or region == Region.SCOTLAND:
+                self.state = self.EnglandOrWalesState(form)
             else:
-                self.state = self.ErrorState(form)
+                self.state = self.OtherJurisdictionState(region, form.cleaned_data["postcode"])
         else:
-            view_name = resolve(request.path_info).url_name
-            current_url = reverse(view_name)
-            self.state = self.OldMapState(form, current_url)
+            self.state = self.ErrorState(form)
+
         return super().get(self, request, *args, **kwargs)
 
     def get_template_names(self):

diff --git a/fala/assets-src/sass/main.scss b/fala/assets-src/sass/main.scss
@@ -15,24 +15,6 @@ $laa-unknown-colour: govuk-colour("orange");
 $laa-staging-colour: govuk-colour("turquoise");
 $laa-dev-colour: govuk-colour("pink");
 
-.page-header {
-  margin: 2em 0;
-}
-
-.form-action {
-  margin-top: 1em;
-}
-
-body.govuk-template__body {
-  height: auto;
-  height: initial;
-  display: block;
-}
-
-.laa-postcode {
-  text-transform: uppercase;
-}
-
 .fala-tickbox-columns_new {
   // Default for mobile (1 column)
   column-count: 1; 
@@ -58,20 +40,6 @@ body.govuk-template__body {
   }
 }
 
-/*below override for archaic class*/
-p.govuk-body:last-child,
-p.govuk-body-m:last-child {
-  margin-bottom:20px;
-}
-
-p.govuk-body-l:last-child {
-  margin-bottom:30px;
-}
-
-.gds-header__error {
-  border-bottom: 10px solid $govuk-error-colour;
-}
-
 .laa-cookie-banner {
   background: govuk-colour("white");
   display: none;
@@ -88,14 +56,6 @@ p.govuk-body-l:last-child {
   }
 }
 
-.laa-hide-if-no-js {
-  display:none;
-}
-
-.js-enabled .laa-hide-if-no-js {
-  display:initial;
-}
-
 body:not(.fala-production) {
   .govuk-header__container {
     border-bottom-color:$laa-unknown-colour;
@@ -123,37 +83,7 @@ body.fala-dev {
   }
 }
 
-.fala-header__logotype-crest {
-  position: relative;
-  top: -7px;
-  margin-right: 5px;
-  vertical-align: top;
-}
-
-.govuk-header__logo,
-.govuk-header__content {
-  display:inline-block;
-  float:none;
-  padding-left:0;
-  width:unset;
-}
-
-.govuk-header__logo {
-  white-space: nowrap;
-  min-width: 33.33%;
-  margin-bottom: 0;
-}
-
-.govuk-footer__copyright-logo {
-  max-width: 140px;
-}
-
 .laa-fala {
-  &__search {
-  margin-top:1em;
-  clear:both;
-  }
-
   &__grey-box {
     background-color: govuk-colour("light-grey");
     padding: 1em;

diff --git a/fala/settings/base.py b/fala/settings/base.py
@@ -219,8 +219,4 @@ def root(*x):
 
 ENVIRONMENT = os.environ.get("ENVIRONMENT", "unknown")
 
-GOOGLE_MAPS_API_KEY = os.environ.get("GOOGLE_MAPS_API_KEY", "")
-
 CHECK_LEGAL_AID_URL = "https://www.gov.uk/check-legal-aid"
-
-FEATURE_FLAG_SURVEY_MONKEY = os.environ.get("FEATURE_FLAG_SURVEY_MONKEY", "").lower() == "enabled"
diff --git a/fala/settings/production.py b/fala/settings/production.py
@@ -3,7 +3,6 @@
 
 settings_required = (
     "SECRET_KEY",
-    "GOOGLE_MAPS_API_KEY",
     "LAALAA_API_HOST",
 )