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.

Sign up for GitHub

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

Jump to bottom

[Fyst-294] add notification preferences page #5027

Open
wants to merge 20 commits into
base: main
Choose a base branch
from
Open

[Fyst-294] add notification preferences page #5027

wants to merge 20 commits into from

Conversation

squanto
Copy link
Contributor

@squanto squanto commented Nov 25, 2024 •
edited
Loading

Link to pivotal/JIRA issue

https://codeforamerica.atlassian.net/browse/FYST-294

Is PM acceptance required? (delete one)

  • Yes - don't merge until JIRA issue is accepted!

Reminder: merge main into this branch and get green tests before merging to main

What was done?

Add :sms_notification_opt_in and :email_notification_opt_in

How to test?

Step through state return flows until you've verified a code, then enter either a phone number or select email

Screenshot

Screenshot 2024-11-25 at 3 19 19 PM

squanto and others added 5 commits November 25, 2024 11:49
@squanto @mrotondo
Add state file md notification prefs
84da6f2 
Co-authored-by: Mike Rotondo <[email protected]>
@squanto @mrotondo
Add ny opt in columns
b1180c9 
Co-authored-by: Mike Rotondo <[email protected]>
@squanto @mrotondo
Add state file az intake fields
03e7619 
Co-authored-by: Mike Rotondo <[email protected]>
@squanto @mrotondo
Add id and nc forms
be9f029 
Co-authored-by: Mike Rotondo <[email protected]>
@squanto @mrotondo
Fill out specs and add nj
869424b 
Co-authored-by: Mike Rotondo <[email protected]>
@github-actions GitHub Actions
Copy link

Heroku app: https://gyr-review-app-5027-815e197df5c3.herokuapp.com/
View logs: heroku logs --app gyr-review-app-5027 (optionally add --tail)

squanto and others added 3 commits November 25, 2024 13:05
@squanto @mrotondo
fix failing specs
1f18f3c 
Co-authored-by: Mike Rotondo <[email protected]>
@squanto @mrotondo
Fix data transfer spec
2535914 
Co-authored-by: Mike Rotondo <[email protected]>
@squanto @mrotondo
Normalize i18n
d59d91b 
Co-authored-by: Mike Rotondo <[email protected]>
powersurge360
powersurge360 requested changes Nov 26, 2024
View reviewed changes
Copy link
Contributor

@powersurge360 powersurge360 left a comment

Choose a reason for hiding this comment

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

I think you are missing form specs at least. The form is pretty simple so I think you can get away with a quick one with just shoulda_matchers. The controller is so basic as to more or less not be there so I guess it's alright to not have a spec.

@squanto @mrotondo
Add specs for form
dce5d6b 
Co-authored-by: Mike Rotondo <[email protected]>
@squanto
Copy link
Contributor Author

squanto commented Nov 26, 2024

I think you are missing form specs at least. The form is pretty simple so I think you can get away with a quick one with just shoulda_matchers. The controller is so basic as to more or less not be there so I guess it's alright to not have a spec.

Good call, added form specs for the validations

powersurge360
powersurge360 approved these changes Nov 26, 2024
View reviewed changes
Copy link
Contributor

@powersurge360 powersurge360 left a comment

Choose a reason for hiding this comment

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

Looks good! You may consider using shoulda_matchers in the future which can simplify standard rails tests. Not that it's invalid to do things this way, I just personally find writing validator tests a little tedious lol

anisharamnani and others added 10 commits November 27, 2024 12:47
@anisharamnani @tahsinaislam @squanto
reference ID 2024 Schema (#5004)
b337e7d 
Co-authored-by: Tahsina Islam <[email protected]>
Co-authored-by: Anisha Ramnani <[email protected]>
@jnf @squanto
[FYST-1217] Tweak a default parameter so it renders as html (#5028)
ad8b708 
* tweak the default render call for landing page bullets
* hint for the linter; because the key is used as a default, the i18n linter doesn't recognize it as being used
Co-authored-by: Hugo Melo <[email protected]>
@anisharamnani @squanto
Change content for political subdivisions (#5019)
f3a0bd3 
Co-authored-by: Hugo Melo <[email protected]>
@mpidcock @squanto
Upload additional DF XMLs & JSONs for NC (#5032)
cc7cb53 
Co-authored-by: Hugo Melo <[email protected]>
@aloverso @squanto
NJ 118 - Lines 45, 49, 50, 54: Calculating Total Tax Due (#5023)
9aa9f70 
Co-authored-by: Hugo Melo <[email protected]>
@powersurge360 @squanto
Adds persona import/export as a CLI (#5037)
1bf6e95 
Adds persona import/export CLI tool
Co-authored-by: Hugo Melo <[email protected]>
@mpidcock @squanto
Fyst 1220 upload DF xmls & jsons for AZ (#5035)
91f95b2 
* johnny

* alexis

* shell
Co-authored-by: Hugo Melo <[email protected]>
@squanto
normailze number before validation
fc62939 
Co-authored-by: Hugo Melo <[email protected]>
@squanto
Update content and spacing
c462724 
Co-authored-by: Hugo Melo <[email protected]>
@squanto
Preparse phone number and tighten checkboxes
a247a94 
Co-authored-by: Hugo Melo <[email protected]>
@squanto
Normalize i18n
99a4f50 
Co-authored-by: Hugo Melo <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@powersurge360 powersurge360 powersurge360 approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@squanto @powersurge360 @anisharamnani @jnf @mpidcock @aloverso