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 27 commits into
base: main
Choose a base branch
from
Open

Conversation

amazingphilippe
Copy link
Contributor

@amazingphilippe amazingphilippe commented Oct 31, 2024

Summary | Résumé

  • Changes the go-live form to gather more structured data. As outlined inthis shorthand form doc.

Test instructions | Instructions pour tester la modification

  1. Log into Notify Staging
  2. Create a new trial service if needed
  3. In the request to go live checklist, choose the "Tell us about how you intend to use GC Notify" step
  4. Fill in page 1 of the form.
  5. The new form elemetns are on page 2 of the form.
  6. You should be able to pick a range for the volumes of sms and emails you want to send daily/monthly.
  7. For the daily limit, if you want to send a lot more, there is a conditional field that opens when you select the "More than X" option.

Copy link

Copy link

@YedidaZalik YedidaZalik left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@amazingphilippe now that I can see how this appears in the interface, I agree that "How many" works!
I also realized that we could shorten the headings from
"About emails you want to send" to "Email" and "About text messages you want to send" to "Text messages"
I think that could reduce cognitive load. Is it too late to do that?

@amazingphilippe
Copy link
Contributor Author

"About emails you want to send" to "Email" and "About text messages you want to send" to "Text messages"

I agree! Not too late. Good catch!

@amazingphilippe amazingphilippe marked this pull request as ready for review November 4, 2024 16:08
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.
Copy link
Member

@andrewleith andrewleith left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great overall. A few comments about adding some tests due to the overall complexity of this particular form.

app/main/forms.py Outdated Show resolved Hide resolved
app/main/forms.py Outdated Show resolved Hide resolved
app/main/forms.py Outdated Show resolved Hide resolved
app/main/forms.py Show resolved Hide resolved
app/main/forms.py Outdated Show resolved Hide resolved
Copy link

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants