-
Notifications
You must be signed in to change notification settings - Fork 13
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
Ask user to confirm info before creating Install #609
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #609 +/- ##
==========================================
+ Coverage 94.15% 94.20% +0.04%
==========================================
Files 80 80
Lines 3252 3278 +26
==========================================
+ Hits 3062 3088 +26
Misses 190 190 ☔ View full report in Codecov by Sentry. |
State is a dropdown and Zip breaks if it's wrong.
Idea: Instead of logging PII, add some marker to the join form log expressing that there was a problem. That would require appending to an existing S3 object in meshforms, since it would save the submission immediately upon entry. |
Yeah i think i already thought of this but http code |
I think now all's I gotta do is update Meshforms. Having just spent 30+ hours learning more React, I have i d e a s. |
Maybe I could include the PII in the Notes of the created user as well...? |
logging.warning(f"Changed street_address: {r.street_address} != {nyc_addr_info.street_address}") | ||
changed_info["street_address"] = nyc_addr_info.street_address | ||
|
||
if r.city != nyc_addr_info.city: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can the ZIP or state mismatch? Or are we excluding that possibility already above?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
They totally can, but I think I have a test for it now, (at least with NJ state but NYC address). I think it's acceptable to have this validation in the backend with the rest of our address code, but maybe we should have some explicit check for it so we can return a more specific error. Right now, it just tells the member that Non-NYC registrations are not accepted.
This is almost done, but I'm seeing some inconsistencies with how the phone number parsing works and how it is stored in the DB. For example, I see every phone number stored like |
Maybe Django is formatting it after the fact? IDK I need to do some more testing. |
@@ -961,7 +961,7 @@ def test_member_moved_and_used_non_primary_email_join_form(self): | |||
f"but got {response3.status_code}.\n Response is: {response3.content.decode('utf-8')}", | |||
) | |||
|
|||
validate_successful_join_form_submission(self, "Valid Join Form", s, response3, expected_member_count=2) | |||
validate_successful_join_form_submission(self, "Valid Join Form", s, response3, expected_member_count=3) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's 5AM so I'm making a wild fucking guess, but this should be making 3 instead of 2 now, since phone number is no longer deduped
yolo'ing it with approval to do so from @Andrew-Dickinson |
Closes nycmeshnet/meshforms#56