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

Limits for go-live form step 2 #1984

Open
wants to merge 21 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
02fd2d4
Limits for go-live form step 2
amazingphilippe Oct 31, 2024
5b4c3bc
format
amazingphilippe Oct 31, 2024
f19859d
Merge branch 'main' into feat/golive-form-updates
amazingphilippe Oct 31, 2024
16c37ab
simplify h2s
amazingphilippe Nov 4, 2024
1ccb87b
format
amazingphilippe Nov 4, 2024
206f52f
validation and tests
amazingphilippe Nov 4, 2024
0480c2b
Merge branch 'main' into feat/golive-form-updates
amazingphilippe Nov 4, 2024
903e809
Add tests and fix bugs
amazingphilippe Nov 5, 2024
ff66f8f
Merge remote-tracking branch 'refs/remotes/origin/feat/golive-form-up…
amazingphilippe Nov 5, 2024
0fdb551
validator config
amazingphilippe Nov 5, 2024
3cd7691
Merge branch 'main' into feat/golive-form-updates
amazingphilippe Nov 5, 2024
53b9e48
Merge branch 'main' into feat/golive-form-updates
amazingphilippe Nov 5, 2024
3f69a95
Merge branch 'main' into feat/golive-form-updates
amazingphilippe Nov 18, 2024
8e5fc8f
de duplicate limit forms, got limits for annual limits
amazingphilippe Nov 20, 2024
fc3ce4f
Merge remote-tracking branch 'refs/remotes/origin/feat/golive-form-up…
amazingphilippe Nov 20, 2024
dda5083
format
amazingphilippe Nov 20, 2024
89d1eb6
Merge branch 'main' into feat/golive-form-updates
amazingphilippe Nov 21, 2024
da088c2
make tests more solid. Fix bug with residual custom limit
amazingphilippe Nov 22, 2024
e40f905
Merge branch 'main' into feat/golive-form-updates
amazingphilippe Nov 22, 2024
8a5f20d
reformat options for consistency
amazingphilippe Nov 22, 2024
4846698
Merge remote-tracking branch 'refs/remotes/origin/feat/golive-form-up…
amazingphilippe Nov 22, 2024
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
8 changes: 8 additions & 0 deletions app/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -432,6 +432,14 @@ def format_thousands(value):
return value


def format_thousands_localized(value):
if isinstance(value, Number):
return "{:,}".format(int(value)).replace(",", "\u2009" if get_current_locale(current_app) == "fr" else ",")
if value is None:
return ""
return value


def valid_phone_number(phone_number):
try:
validate_phone_number(phone_number)
Expand Down
152 changes: 118 additions & 34 deletions app/main/forms.py
amazingphilippe marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
from itertools import chain

import pytz
from flask import request
from flask import current_app, request
from flask_babel import lazy_gettext as _l
from flask_wtf import FlaskForm as Form
from flask_wtf.file import FileAllowed
Expand Down Expand Up @@ -39,10 +39,11 @@
Length,
Optional,
Regexp,
StopValidation,
)
from wtforms.widgets import CheckboxInput, ListWidget

from app import format_thousands
from app import current_service, format_thousands, format_thousands_localized
from app.main.validators import (
Blocklist,
CsvFileValidator,
Expand Down Expand Up @@ -1807,47 +1808,130 @@
)


class GoLiveAboutNotificationsForm(GoLiveAboutServiceForm):
notification_types = MultiCheckboxField(
_l("Specify the type of notifications you plan on sending."),
choices=[
("email", _l("Email")),
("sms", _l("Text message")),
],
class OptionalIntegerRange:
def __init__(self, trigger_field, trigger_value, min=None, max=None, message=None):
self.trigger_field = trigger_field
self.trigger_value = trigger_value
self.min = min
self.max = max
self.message = message

def __call__(self, form, field):

Check notice

Code scanning / CodeQL

Explicit returns mixed with implicit (fall through) returns Note

Mixing implicit and explicit returns may indicate an error as implicit returns always return None.
trigger_data = getattr(form, self.trigger_field).data

# If trigger radio isn't selected, Stop Validation
if trigger_data != self.trigger_value:
field.process(formdata=None) # Clear the field
field.errors = [] # Delete any errors
return StopValidation() # Stop validation chain

# Only validate if the trigger condition is met
if trigger_data == self.trigger_value:
# First check if empty
if field.data is None or field.data == "":
raise ValidationError(self.message or _l("This cannot be empty"))
# Then check range if value is provided
if self.min is not None and field.data < self.min:
raise ValidationError(_l("Number must be more than {min}").format(min=format_thousands_localized(self.min)))
if self.max is not None and field.data > self.max:
raise ValidationError(_l("Number must be less than {max}").format(max=format_thousands_localized(self.max)))
else:
return True


class BaseGoLiveAboutNotificationsForm():
def volume_choices(self, limit, notification_type):
return [
("0", _l("None")),
("within_limit", _l("1 to {}").format(format_thousands_localized(limit))),
(
"above_limit",
_l("{min} to {max}").format(
min=format_thousands_localized(limit + 1), max=format_thousands_localized(limit * 10)
),
),
(f"more_{notification_type}", _l("More than {}").format(format_thousands_localized(limit * 10))),
]

def volume_choices_restricted(self, limit):
return [
("0", _l("None")),
("within_limit", _l("1 to {}").format(format_thousands_localized(limit))),
("above_limit", _l("More than {}").format(format_thousands_localized(limit))),
]

def more_validators(self, limit, notification_type):
return [
OptionalIntegerRange(
trigger_field=f"daily_{notification_type}_volume",
trigger_value=f"more_{notification_type}",
min=limit * 10 + 1, # +1 because we want the value to be greater than (and not equal to) the previous option
)
]

daily_email_volume = RadioField(
_l("How many emails do you expect to send on a busy day?"),
validators=[DataRequired()],
)
expected_volume = RadioField(
_l("How many notifications do you plan on sending per month?"),
choices=[
("1-1k", _l("1 to 1,000 notifications")),
("1k-10k", _l("1,000 to 10,000 notifications")),
("10k-100k", _l("10,000 to 100,000 notifications")),
("100k+", _l("More than 100,000 notifications")),
],
annual_email_volume = RadioField(
_l("How many emails do you expect to send in a year?"),
validators=[DataRequired()],
)


class GoLiveAboutNotificationsFormNoOrg(GoLiveAboutServiceFormNoOrg):
notification_types = MultiCheckboxField(
_l("Specify the type of notifications you plan on sending."),
choices=[
("email", _l("Email")),
("sms", _l("Text message")),
],
daily_sms_volume = RadioField(
_l("How many text messages do you expect to send on a busy day?"),
validators=[DataRequired()],
)
expected_volume = RadioField(
_l("How many notifications do you plan on sending per month?"),
choices=[
("1-1k", _l("1 to 1,000 notifications")),
("1k-10k", _l("1,000 to 10,000 notifications")),
("10k-100k", _l("10,000 to 100,000 notifications")),
("100k+", _l("More than 100,000 notifications")),
],
annual_sms_volume = RadioField(
_l("How many text messages do you expect to send in a year?"),
validators=[DataRequired()],
)

how_many_more_email = IntegerField(
label=_l("How many?"),
default="",
)
how_many_more_sms = IntegerField(
label=_l("How many?"),
default="",
)

def __init__(self, *args, **kwargs):
super().__init__(*args, **kwargs)

# Choices for daily/annual emails
self.daily_email_volume.choices = self.volume_choices(
limit=current_app.config["DEFAULT_LIVE_SERVICE_LIMIT"], notification_type="email"
)
self.annual_email_volume.choices = self.volume_choices_restricted(limit=current_service.email_annual_limit)

# Choices for daily/annual sms
self.daily_sms_volume.choices = self.volume_choices(
limit=current_app.config["DEFAULT_LIVE_SMS_DAILY_LIMIT"], notification_type="sms"
)
self.annual_sms_volume.choices = self.volume_choices_restricted(limit=current_service.sms_annual_limit)

# Validators for daily emails/sms
self.how_many_more_email.validators = self.more_validators(
limit=current_app.config["DEFAULT_LIVE_SERVICE_LIMIT"], notification_type="email"
)
self.how_many_more_sms.validators = self.more_validators(
limit=current_app.config["DEFAULT_LIVE_SMS_DAILY_LIMIT"], notification_type="sms"
)

@property
def volume_conditionals(self):
return {"more_email": self.how_many_more_email, "more_sms": self.how_many_more_sms}


class GoLiveAboutNotificationsForm(BaseGoLiveAboutNotificationsForm, GoLiveAboutServiceForm):
def __init__(self, *args, **kwargs):
super().__init__(*args, **kwargs)


class GoLiveAboutNotificationsFormNoOrg(BaseGoLiveAboutNotificationsForm, GoLiveAboutServiceFormNoOrg):
def __init__(self, *args, **kwargs):
super().__init__(*args, **kwargs)


class BrandingGOCForm(StripWhitespaceForm):
"""
Expand Down
12 changes: 9 additions & 3 deletions app/templates/views/service-settings/use-case.html
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
{% from "components/form.html" import form_wrapper %}
{% from "components/page-header.html" import page_header %}
{% from "components/page-footer.html" import page_footer %}
{% from "components/radios.html" import radios %}
{% from "components/select-input.html" import select %}
{% from "components/textbox.html" import textbox %}
{% from "components/checkbox.html" import checkboxes %}

Expand Down Expand Up @@ -38,8 +38,14 @@
<p>
{{ _("This information helps us set sending limits and budget accordingly.") }}
</p>
{{ checkboxes(form.notification_types) }}
{{ radios(form.expected_volume) }}
<h2 class="heading-medium">{{ _("Emails") }}</h2>
{{ select(form.daily_email_volume, option_conditionals=form.volume_conditionals) }}
{{ select(form.annual_email_volume, option_conditionals=form.volume_conditionals) }}

<h2 class="heading-medium">{{ _("Text messages") }}</h2>
{{ select(form.daily_sms_volume, option_conditionals=form.volume_conditionals) }}
{{ select(form.annual_sms_volume, option_conditionals=form.volume_conditionals) }}


<!-- Coming from the previous step. If it's not included, checkboxes will be reset -->
<div class="hidden">
Expand Down
11 changes: 11 additions & 0 deletions app/translations/csv/fr.csv
Original file line number Diff line number Diff line change
Expand Up @@ -1459,6 +1459,17 @@
"1,000 to 10,000 notifications","1&nbsp;000 à 10&nbsp;000 notifications"
"10,000 to 100,000 notifications","10&nbsp;000 à 100&nbsp;000 notifications"
"More than 100,000 notifications","Plus de 100&nbsp;000 notifications"
"None","Aucun"
"1 to {}","1 à {}"
"{min} to {max}","{min} à {max}"
"More than {}","Plus de {}"
"How many?","Combien?"
"How many emails do you expect to send on a busy day?","Combien de courriels enverrez-vous lors d’une journée occupée?"
"How many emails do you expect to send in a year?","Combien de courriels enverrez-vous par année?"
"How many text messages do you expect to send on a busy day?","Combien de messages texte enverrez-vous lors d’une journée occupée?"
"How many text messages do you expect to send in a year?","Combien de messages texte enverrez-vous par année?"
"Number must be more than {min}","Le nombre doit être supérieur à {min}"
"Number must be less than {max}","Le nombre doit être inférieur à {max}"
"Specify the type of notifications you plan on sending.","Précisez le type de notification que vous prévoyez envoyer."
"How many notifications do you plan on sending per month?","Combien de notifications prévoyez-vous envoyer par mois?"
"Once you have completed all the steps, submit your request to the GC Notify team. We’ll be in touch within 2 business days.","Après avoir complété toutes les étapes, soumettez votre demande à l’équipe Notification GC. Nous communiquerons avec vous dans un délai de 2 jours ouvrables."
Expand Down
67 changes: 66 additions & 1 deletion tests/app/main/test_validators.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
import pytest
from wtforms import ValidationError

from app.main.forms import RegisterUserForm, ServiceSmsSenderForm
from app.main.forms import OptionalIntegerRange, RegisterUserForm, ServiceSmsSenderForm
from app.main.validators import NoCommasInPlaceHolders, OnlySMSCharacters, ValidGovEmail


Expand Down Expand Up @@ -178,3 +178,68 @@ def test_sms_sender_form_validation(
form.sms_sender.data = "00111222333"
form.validate()
assert "Can't start with 00" == form.errors["sms_sender"][0]


@pytest.mark.parametrize(
"trigger_data, field_data, expected_error",
[
# Case 1: Trigger not activated
("option_a", "invalid", None),
("option_a", None, None),
("option_a", "", None),
# Case 2: Trigger activated, valid values
("trigger_value", 1000, None),
("trigger_value", 5000, None),
# Case 3: Trigger actived, empty values
("trigger_value", None, "This cannot be empty"),
("trigger_value", "", "This cannot be empty"),
# Case 4: Trigger actived, values not within min and max
("trigger_value", 999, "Number must be more than 1,000"),
("trigger_value", 5001, "Number must be less than 5,000"),
],
)
def test_optional_integer_range_validation(trigger_data, field_data, expected_error, mocker):
# Mock locale
mocker.patch("app.get_current_locale", return_value="en")

# Form mock with trigger field
form = Mock()
form.trigger_field = Mock(data=trigger_data)

# Integer field mock
field = Mock(data=field_data)
field.errors = []

# Init validator
validator = OptionalIntegerRange(
trigger_field="trigger_field",
trigger_value="trigger_value",
min=1000,
max=5000,
)

if expected_error:
# Test when we expect an error
with pytest.raises(ValidationError) as error:
validator(form, field)
assert str(error.value) == expected_error
else:
# Test when we expect no error
validator(form, field)
assert field.errors == []


def test_optional_integer_range_custom_message():
form = Mock()
form.trigger_field = Mock(data="trigger_value")
field = Mock(data=None)

validator = OptionalIntegerRange(
trigger_field="trigger_field",
trigger_value="trigger_value",
message="Custom error message",
)

with pytest.raises(ValidationError) as exc:
validator(form, field)
assert str(exc.value) == "Custom error message"
24 changes: 18 additions & 6 deletions tests/app/main/views/test_service_settings.py
Original file line number Diff line number Diff line change
Expand Up @@ -1014,8 +1014,12 @@ def test_request_to_go_live_use_case_page(
"department_org_name": "Org name",
"intended_recipients": ["public"],
"purpose": "Purpose",
"notification_types": [],
"expected_volume": None,
"daily_email_volume": None,
"annual_email_volume": None,
"daily_sms_volume": None,
"annual_sms_volume": None,
"how_many_more_email": None,
"how_many_more_sms": None,
},
"step": "about-notifications",
}
Expand All @@ -1042,8 +1046,12 @@ def test_request_to_go_live_use_case_page(
_expected_status=302,
_expected_redirect=url_for("main.request_to_go_live", service_id=SERVICE_ONE_ID),
_data={
"notification_types": ["sms"],
"expected_volume": "1k-10k",
"daily_email_volume": "0",
"annual_email_volume": "within_limit",
"daily_sms_volume": "more_sms",
"annual_sms_volume": "above_limit",
"how_many_more_email": None,
"how_many_more_sms": 25000,
# Need to submit intended_recipients again because otherwise
# the form thinks we removed this value (it's checkboxes).
# On the real form, this field is hidden on the second step
Expand All @@ -1061,8 +1069,12 @@ def test_request_to_go_live_use_case_page(
"department_org_name": "Org name",
"intended_recipients": ["public"],
"purpose": "Purpose",
"notification_types": ["sms"],
"expected_volume": "1k-10k",
"daily_email_volume": "0",
"annual_email_volume": "within_limit",
"daily_sms_volume": "more_sms",
"annual_sms_volume": "above_limit",
"how_many_more_email": None,
"how_many_more_sms": 25000,
},
"step": "about-notifications",
},
Expand Down
Loading