EL-1452: Remove feature flag & redundant code (#305)

- 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 - updates based on review comments
ministryofjustice · Jul 15, 2024 · a50985d · a50985d
1 parent 4baa8b8
commit a50985d
Show file tree

Hide file tree

Showing 24 changed files with 16 additions and 759 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: Deploy UAT branch

diff --git a/.env.example b/.env.example
@@ -4,8 +4,7 @@ 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
+FEATURE_FLAG_SURVEY_MONKEY = "enabled"
 
 # 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
@@ -3,7 +3,6 @@
 from django import forms
 from django.utils.translation import gettext_lazy as _
 from django.utils.safestring import mark_safe
-from django.conf import settings
 
 import laalaa.api as laalaa
 import re
@@ -37,15 +36,6 @@ def to_python(self, value):
 
 
 class AdviserSearchForm(forms.Form):
-    # Only used with FEATURE_FLAG_NO_MAP False
-    REGION_NAMES = {
-        Region.NI: "Northern Ireland. ",
-        Region.IOM: "the Isle of Man. ",
-        Region.JERSEY: "Jersey. ",
-        Region.GUERNSEY: "Guernsey. ",
-        Region.SCOTLAND: "Scotland. ",
-    }
-
     page = forms.IntegerField(required=False, widget=forms.HiddenInput())
 
     postcode = CapitalisedPostcodeField(
@@ -80,28 +70,16 @@ class AdviserSearchForm(forms.Form):
     def __init__(self, *args, **kwargs):
         kwargs.setdefault("label_suffix", "")
         super(AdviserSearchForm, self).__init__(*args, **kwargs)
-        if not settings.FEATURE_FLAG_NO_MAP:
-            self.fields["postcode"].label = _("Enter postcode")
-            self.fields["postcode"].help_text = _(
-                "For example, <span class='notranslate' translate='no'>SW1H 9AJ</span>"
-            )
 
     def clean(self):
         data = self.cleaned_data
         postcode = data.get("postcode")
         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
@@ -3,7 +3,6 @@
 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")
@@ -125,7 +121,6 @@ def test_research_banner_absent_when_flag_unset(self):
         self.assertNotContains(response, self.research_message)
 
 
-@override_settings(FEATURE_FLAG_NO_MAP=True)
 class ResultsPageWithJustOrgTest(SimpleTestCase):
     client = Client()
     url = reverse("search")
@@ -141,7 +136,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 +151,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 +167,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):
@@ -121,7 +109,6 @@ def get_context_data(self):
                 "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,
             }
 
@@ -166,19 +153,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
@@ -225,12 +225,8 @@ def root(*x):
 
 LAALAA_API_HOST = os.environ.get("LAALAA_API_HOST", None)
 
-FEATURE_FLAG_NO_MAP = os.environ.get("FEATURE_FLAG_NO_MAP", "").lower() == "enabled"
-
 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",
 )
 

diff --git a/fala/templates/500.html b/fala/templates/500.html
@@ -8,9 +8,7 @@
 
 {% block content %}
   <div class="govuk-grid-column-two-thirds">
-    <header class="page-header">
-      <h1 class="govuk-heading-xl">Sorry, there is a problem with the service</h1>
-    </header>
+    <h1 class="govuk-heading-xl">Sorry, there is a problem with the service</h1>
     <p class="govuk-body">Try again later.</p>
     <p class="govuk-body">You can also <a class="govuk-link" href="http://solicitors.lawsociety.org.uk/">find a solicitor</a> on the Law Society website.</p>
     <p class="govuk-body">Search using your location, then select ‘Accepts legal aid’ to show legal aid solicitors near you.</p>