Remove FF_SPIKE_SMS_DAILY_LIMIT and FF_SMS_PARTS_UI (#1623)

* Remove FF_SPIKE_SMS_DAILY_LIMIT * fix * fix test * remove ff * fix formatting * fix * fix changes * remove --------- Co-authored-by: Andrew <[email protected]>
cds-snc · Aug 1, 2023 · 8e7d1c6 · 8e7d1c6
1 parent 268de25
commit 8e7d1c6
Show file tree

Hide file tree

Showing 11 changed files with 56 additions and 249 deletions.
diff --git a/Makefile b/Makefile
@@ -59,6 +59,7 @@ run-dev:
 format:
 	isort ./app ./tests
 	black ./app ./tests
+	flake8 ./app ./tests
 	isort --check-only ./app ./tests
 	mypy ./
 	npx prettier --write app/assets/javascripts app/assets/stylesheets
diff --git a/app/config.py b/app/config.py
@@ -138,8 +138,6 @@ class Config(object):
 
     # FEATURE FLAGS
     FF_SALESFORCE_CONTACT = env.bool("FF_SALESFORCE_CONTACT", False)
-    FF_SPIKE_SMS_DAILY_LIMIT = env.bool("FF_SPIKE_SMS_DAILY_LIMIT", False)
-    FF_SMS_PARTS_UI = env.bool("FF_SMS_PARTS_UI", False)
     FF_BOUNCE_RATE_V15 = env.bool("FF_BOUNCE_RATE_V15", False)
     FF_SALESFORCE_CONTACT = env.bool("FF_SALESFORCE_CONTACT", False)
     FF_EMAIL_DAILY_LIMIT = env.bool("FF_EMAIL_DAILY_LIMIT", False)
@@ -203,8 +201,6 @@ class Test(Development):
     TESTING = True
     WTF_CSRF_ENABLED = False
     GC_ARTICLES_API = "articles.alpha.canada.ca/notification-gc-notify"
-    FF_SPIKE_SMS_DAILY_LIMIT = False
-    FF_SMS_PARTS_UI = False
     FF_SALESFORCE_CONTACT = False
     FF_EMAIL_DAILY_LIMIT = False
 

diff --git a/app/main/views/send.py b/app/main/views/send.py
@@ -785,7 +785,7 @@ def check_messages(service_id, template_id, upload_id, row_index=2):
     data["sms_parts_remaining"] = current_service.sms_daily_limit - daily_sms_fragment_count(service_id)
     data["send_exceeds_daily_limit"] = data["sms_parts_to_send"] > data["sms_parts_remaining"]
 
-    if current_app.config["FF_SPIKE_SMS_DAILY_LIMIT"] and data["send_exceeds_daily_limit"]:
+    if data["send_exceeds_daily_limit"]:
         return render_template("views/check/column-errors.html", **data)
 
     metadata_kwargs = {

diff --git a/app/main/views/service_settings.py b/app/main/views/service_settings.py
@@ -98,7 +98,6 @@ def service_settings(service_id: str):
         "free_yearly_sms": current_app.config["FREE_YEARLY_SMS_LIMIT"],
     }
     assert limits["free_yearly_email"] >= 2_000_000, "The user-interface does not support French translations of < 2M"
-
     return render_template(
         "views/service-settings.html",
         service_permissions=PLATFORM_ADMIN_SERVICE_PERMISSIONS,

diff --git a/app/templates/views/check/ok.html b/app/templates/views/check/ok.html
@@ -88,9 +88,6 @@ <h2 class="heading-medium">{{ _('Preview of one message out of {}').format(count
     } if preview_row < count_of_recipients + 1
   ) }}
   </section>
-  {% if config["FF_SMS_PARTS_UI"] and template.template_type == 'sms'%}
-    {% include 'views/check/sms-parts.html' %}
-  {% endif %}
   <div class="mb-12 clear-both contain-floats">
     <form method="post" enctype="multipart/form-data" action="{{ url_for('main.start_job', service_id=current_service.id, upload_id=upload_id, original_file_name=original_file_name) }}" class='page-footer'>
       <input type="hidden" name="csrf_token" value="{{ csrf_token() }}"/>

diff --git a/app/templates/views/dashboard/dashboard.html b/app/templates/views/dashboard/dashboard.html
@@ -24,13 +24,6 @@ <h2 class="heading-medium mt-8">{{ _("Scheduled sends") }}</h2>
       {{ ajax_block(partials, updates_url, 'upcoming', interval=5) }}
     {% endif %}
 
-    {% if config["FF_SMS_PARTS_UI"] %}
-      <h2 class="heading-medium mt-8">
-        {{ _('Usage today') }}
-      </h2>
-      {{ ajax_block(partials, updates_url, 'daily_totals', interval=5) }}
-    {% endif %}
-
     {{ ajax_block(partials, updates_url, 'weekly_totals', interval=5) }}
 
     {% if not config["FF_BOUNCE_RATE_V15"] %}

diff --git a/app/templates/views/notifications/check.html b/app/templates/views/notifications/check.html
@@ -75,9 +75,6 @@
   {% endif %}
 
   {{ template|string|translate_preview_template }}
-  {% if config["FF_SMS_PARTS_UI"] and template.template_type == 'sms'%}
-    {% include 'views/check/sms-parts.html' %}
-  {% endif %}
 
   <div class="js-stick-at-bottom-when-scrolling">
     <form method="post" enctype="multipart/form-data" action="{{url_for(

diff --git a/app/templates/views/service-settings.html b/app/templates/views/service-settings.html
@@ -272,14 +272,12 @@ <h2 class="heading-small p-0 m-0">{{ _('Your service is in trial mode') }}</h2>
           {{ text_field('')}}
         {% endcall %}
 
-        {% if config["FF_SPIKE_SMS_DAILY_LIMIT"] %}
-          {% call settings_row(if_has_permission='sms', alt_cond=current_service.live) %}
-            {% set txt = _('Daily text fragments limit') %}
-            {{ text_field(txt)}}
-            {{ text_field(current_service.sms_daily_limit | format_number) }}
-            {{ text_field('') }}
-          {% endcall %}
-        {% endif %}
+        {% call settings_row(if_has_permission='sms', alt_cond=current_service.live) %}
+          {% set txt = _('Daily text fragments limit') %}
+          {{ text_field(txt)}}
+          {{ text_field(current_service.sms_daily_limit | format_number) }}
+          {{ text_field('') }}
+        {% endcall %}
 
       {% endcall %}
 
@@ -376,18 +374,16 @@ <h2 class="heading-medium">{{ _('Platform admin settings') }}</h2>
             ) }}
           {% endcall %}
 
-          {% if config["FF_SPIKE_SMS_DAILY_LIMIT"] %}
-            {% call row() %}
-              {% set txt = _('Daily text fragments limit') %}
-              {{ text_field(txt)}}
-              {{ text_field(current_service.sms_daily_limit | format_number) }}
-              {{ edit_field(
-                change_txt,
-                url_for('.set_sms_message_limit', service_id=current_service.id),
-                for=txt
-              ) }}
-            {% endcall %}
-          {% endif %}
+          {% call row() %}
+            {% set txt = _('Daily text fragments limit') %}
+            {{ text_field(txt)}}
+            {{ text_field(current_service.sms_daily_limit | format_number) }}
+            {{ edit_field(
+              change_txt,
+              url_for('.set_sms_message_limit', service_id=current_service.id),
+              for=txt
+            ) }}
+          {% endcall %}
 
           {% call row() %}
             {% set txt = _('API rate limit per minute') %}

diff --git a/tests/app/main/views/test_dashboard.py b/tests/app/main/views/test_dashboard.py
@@ -507,10 +507,6 @@ def test_correct_columns_display_on_dashboard(
         assert len(page.select(column_name)) == expected_column_count
 
 
-@pytest.mark.parametrize(
-    "feature_flag",
-    [True, False],
-)
 def test_daily_usage_section_shown(
     client_request,
     mocker,
@@ -520,23 +516,16 @@ def test_daily_usage_section_shown(
     mock_get_jobs,
     service_one,
     app_,
-    feature_flag,
 ):
-    app_.config["FF_SMS_PARTS_UI"] = feature_flag
-
     page = client_request.get(
         "main.service_dashboard",
         service_id=service_one["id"],
     )
     headings = [element.text.strip() for element in page.find_all("h2")]
     big_number_labels = [element.text.strip() for element in page.select(".big-number-label")]
 
-    if feature_flag:
-        assert "Usage today" in headings
-        assert "text messages  left today" in big_number_labels
-    else:
-        assert "Usage today" not in headings
-        assert "text messages  left today" not in big_number_labels
+    assert "Usage today" not in headings
+    assert "text messages  left today" not in big_number_labels
 
 
 @pytest.mark.parametrize(

diff --git a/tests/app/main/views/test_send.py b/tests/app/main/views/test_send.py
@@ -41,7 +41,6 @@
     mock_get_service_letter_template,
     mock_get_service_template,
     normalize_spaces,
-    set_config,
 )
 
 template_types = ["email", "sms"]
@@ -2502,7 +2501,7 @@ def test_check_messages_back_link(
     ],
     ids=["none_sent", "some_sent"],
 )
-def test_check_messages_shows_too_many_sms_messages_errors_when_FF_SPIKE_SMS_DAILY_LIMIT_true(
+def test_check_messages_shows_too_many_sms_messages_errors(
     app_,
     mocker,
     client_request,
@@ -2540,71 +2539,21 @@ def test_check_messages_shows_too_many_sms_messages_errors_when_FF_SPIKE_SMS_DAI
             }
         }
 
-    with set_config(app_, "FF_SPIKE_SMS_DAILY_LIMIT", True):
-        page = client_request.get(
-            "main.check_messages",
-            service_id=SERVICE_ONE_ID,
-            template_id=fake_uuid,
-            upload_id=fake_uuid,
-            original_file_name="valid.csv",
-            _test_page_title=False,
-        )
+    page = client_request.get(
+        "main.check_messages",
+        service_id=SERVICE_ONE_ID,
+        template_id=fake_uuid,
+        upload_id=fake_uuid,
+        original_file_name="valid.csv",
+        _test_page_title=False,
+    )
 
     # remove excess whitespace from element
     details = page.find(role="alert").findAll("h2")[0]
     details = " ".join([line.strip() for line in details.text.split("\n") if line.strip() != ""])
     assert details == expected_msg
 
 
-def test_check_messages_does_not_show_too_many_sms_messages_errors_when_FF_SPIKE_SMS_DAILY_LIMIT_false(
-    app_,
-    mocker,
-    client_request,
-    mock_get_service,  # set message_limit to 50 and sms limit to 20
-    mock_get_users_by_service,
-    mock_get_service_template,
-    mock_get_template_statistics,
-    mock_get_job_doesnt_exist,
-    mock_get_jobs,
-    mock_s3_download,
-    mock_s3_set_metadata,
-    fake_uuid,
-):
-    # csv with 100 phone numbers
-    mocker.patch(
-        "app.main.views.send.s3download",
-        return_value=",\n".join(["phone number"] + ([mock_get_users_by_service(None)[0]["mobile_number"]] * 30)),
-    )
-    mocker.patch(
-        "app.service_api_client.get_service_statistics",
-        return_value={
-            "sms": {"requested": 0, "delivered": 0, "failed": 0},
-            "email": {"requested": 0, "delivered": 0, "failed": 0},
-        },
-    )
-
-    with client_request.session_transaction() as session:
-        session["file_uploads"] = {
-            fake_uuid: {
-                "template_id": fake_uuid,
-                "notification_count": 1,
-                "valid": True,
-            }
-        }
-
-    with set_config(app_, "FF_SPIKE_SMS_DAILY_LIMIT", False):
-        page = client_request.get(
-            "main.check_messages",
-            service_id=SERVICE_ONE_ID,
-            template_id=fake_uuid,
-            upload_id=fake_uuid,
-            original_file_name="valid.csv",
-            _test_page_title=False,
-        )
-
-    assert page.find(role="alert") is None
-
-
 @pytest.mark.parametrize(
     "num_requested,expected_msg",
     [
@@ -3693,23 +3642,3 @@ class Object(object):
     multiple_choise_options = [x.text.strip() for x in options]
 
     assert multiple_choise_options == expected_filenames
-
-
-@pytest.mark.parametrize(
-    "feature_flag, expected",
-    [(True, 1), (False, 0)],
-)
-def test_sms_parts_section_is_visible_depending_on_ff(
-    feature_flag, expected, client_request, service_one, fake_uuid, mock_get_service_template, mock_get_template_statistics, app_
-):
-    app_.config["FF_SMS_PARTS_UI"] = feature_flag
-
-    with client_request.session_transaction() as session:
-        session["recipient"] = "6502532223"
-        session["placeholders"] = {}
-
-    page = client_request.get("main.check_notification", service_id=service_one["id"], template_id=fake_uuid)
-
-    assert page.h1.text.strip() == "Review before sending"
-    sms_count = page.select("#sms-count")
-    assert len(list(sms_count)) == expected