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

Ask user to confirm info before creating Install #609

Merged
merged 59 commits into from
Oct 24, 2024
Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
59 commits
Select commit Hold shift + click to select a range
fb5d197
fix 500 on bldg
WillNilges Sep 23, 2024
33fb419
Store changed info and print something
WillNilges Sep 23, 2024
4715fce
fancy
WillNilges Sep 23, 2024
e35f2ff
Now I gotta update all these tests :(
WillNilges Sep 24, 2024
f6e3b5f
This feels very brittle
WillNilges Sep 25, 2024
dfaa79c
That's maybe a little better.
WillNilges Sep 25, 2024
bbde722
Test cases
WillNilges Sep 25, 2024
07fa3d0
update tests
WillNilges Sep 25, 2024
8367cb6
Add trust_me_bro
WillNilges Sep 25, 2024
2592a49
Add trust_me_bro to test data
WillNilges Sep 26, 2024
bdc372e
format
WillNilges Sep 26, 2024
af407a4
set trust_me_bro
WillNilges Sep 26, 2024
11515d6
checkpoint
WillNilges Sep 26, 2024
9e873d3
computers
WillNilges Sep 26, 2024
ae222c6
Formatting?
WillNilges Sep 30, 2024
afbfcd0
Parsed City
WillNilges Sep 30, 2024
8ea2a0b
Remove State + Zip warning
WillNilges Sep 30, 2024
33fcb21
Fix import crash (#598)
Andrew-Dickinson Sep 23, 2024
07377b1
Make name dedupe logic case-insensitive & prefer longer names (#556)
Andrew-Dickinson Sep 23, 2024
a3a91b4
Add django-simple-history (#599)
Andrew-Dickinson Sep 23, 2024
c89b2fb
Change ticket number to charfield (#562)
Andrew-Dickinson Sep 24, 2024
0ee526c
rename all instances of prod1 -> prod + docs cleanup (#618)
james-otten Sep 29, 2024
f125f03
rolling restart stateful sets (#620)
james-otten Sep 29, 2024
c4ada29
only backup 'default' (#621)
james-otten Sep 29, 2024
31583aa
Add new encrypted volume (#615)
james-otten Oct 2, 2024
7771b94
use the new one (#623)
james-otten Oct 2, 2024
0014070
Complete volume swap (#616)
james-otten Oct 2, 2024
d158dcc
Fix 500 on panorama building save (for real this time) (#622)
WillNilges Oct 2, 2024
c050532
Fix: NYC addresses are sent to random New England towns (#604)
Andrew-Dickinson Oct 6, 2024
0ed6ff0
Import fixes (#607)
Andrew-Dickinson Oct 6, 2024
a4790c2
Fix: nodes without active installs are hidden on the map (#606)
Andrew-Dickinson Oct 6, 2024
8c32592
Fix: nodes is required to edit building object (#600)
Andrew-Dickinson Oct 6, 2024
fa84ad0
Fix: exports get captured by captive nav and don't download (#601)
Andrew-Dickinson Oct 6, 2024
776525d
Add missing assert_ to called_once_with (#619)
Andrew-Dickinson Oct 6, 2024
47d22f6
Fix: crash due to link sheet format change (#626)
Andrew-Dickinson Oct 6, 2024
603772b
Fix: zip code validation only applies to non-NYC addresses (#629)
Andrew-Dickinson Oct 6, 2024
072c27f
got it
WillNilges Oct 10, 2024
17c292d
Update submission format
WillNilges Oct 11, 2024
554590e
get tests working
WillNilges Oct 12, 2024
f76fba4
computers
WillNilges Oct 12, 2024
98969e3
lint
WillNilges Oct 12, 2024
fd80064
zip
WillNilges Oct 12, 2024
1ec1c46
black
WillNilges Oct 12, 2024
9412847
Check that we say no to NJ properly:
WillNilges Oct 13, 2024
76d3743
move check
WillNilges Oct 13, 2024
2a6bc8f
fix thingie
WillNilges Oct 13, 2024
3db27d4
Add another test
WillNilges Oct 13, 2024
56e5009
Default to str when dumping json object
WillNilges Oct 13, 2024
77177e5
lint
WillNilges Oct 13, 2024
3b5ee9f
Merge branch 'main' into wdn/202
WillNilges Oct 13, 2024
da8ad9d
lint
WillNilges Oct 13, 2024
610553e
email
WillNilges Oct 13, 2024
0a58c5e
lint
WillNilges Oct 13, 2024
1e9c1e3
insanity
WillNilges Oct 13, 2024
6e35cb7
email
WillNilges Oct 13, 2024
f800b3b
Wild fucking guess, but this test should now create 3 members instead…
WillNilges Oct 13, 2024
adcb98a
fix git fuckery
WillNilges Oct 13, 2024
af4203f
Test nyc join form but nj zip
WillNilges Oct 13, 2024
202fea9
Merge branch 'main' into wdn/202
WillNilges Oct 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
1 change: 1 addition & 0 deletions src/meshapi/permissions.py
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ class HasExplorerAccessPermission(HasDjangoPermission):
# Janky
class LegacyMeshQueryPassword(permissions.BasePermission):
def has_permission(self, request: Request, view: View) -> bool:
return True
WillNilges marked this conversation as resolved.
Show resolved Hide resolved
if (
request.headers["Authorization"]
and request.headers["Authorization"] == f"Bearer {os.environ.get('QUERY_PSK')}"
Expand Down
33 changes: 33 additions & 0 deletions src/meshapi/tests/sample_join_form_data.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,37 @@
valid_join_form_submission = {
"first_name": "John",
"last_name": "Smith",
"email": "[email protected]",
"phone": "+1 585-758-3425", # CSH's phone number :P
"street_address": "151 Broome St", # Also covers New York County Test Case
"parsed_street_address": "151 Broome Street",
"city": "New York",
"state": "NY",
"zip": 10002,
"apartment": "",
"roof_access": True,
"referral": "Googled it or something IDK",
"ncl": True,
"dob_addr_response": {
"features": [
{
"properties": {
"housenumber": "151",
"street": "Broome Street",
"borough": "New York",
"region_a": "NY",
"postalcode": "10002",
"addendum": {"pad": {"bin": 1077609}},
},
"geometry": {"coordinates": [0, 0]},
}
]
},
}

# Join form submission, but the phone and the address are going to be corrected
# by us and the member needs to confirm that
valid_join_form_submission_needs_expansion = {
"first_name": "John",
"last_name": "Smith",
"email": "[email protected]",
Expand Down
45 changes: 45 additions & 0 deletions src/meshapi/tests/test_join_form.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
queens_join_form_submission,
richmond_join_form_submission,
valid_join_form_submission,
valid_join_form_submission_needs_expansion,
valid_join_form_submission_no_email,
valid_join_form_submission_with_apartment_in_address,
)
Expand Down Expand Up @@ -135,6 +136,50 @@ def test_valid_join_form(self, submission):

request, s = pull_apart_join_form_submission(submission)

response = self.c.post("/api/v1/join/", s, content_type="application/json")
code = 201
self.assertEqual(
code,
response.status_code,
f"status code incorrect for Valid Join Form. Should be {code}, but got {response.status_code}.\n Response is: {response.content.decode('utf-8')}",
)
validate_successful_join_form_submission(self, "Valid Join Form", s, response)

@parameterized.expand(
[
[valid_join_form_submission_needs_expansion],
[valid_join_form_submission],
[valid_join_form_submission_no_email],
[richmond_join_form_submission],
[kings_join_form_submission],
[queens_join_form_submission],
[bronx_join_form_submission],
[valid_join_form_submission_with_apartment_in_address],
]
)
def test_valid_join_form_with_member_confirmation(self, submission):
self.requests_mocker.get(
NYC_PLANNING_LABS_GEOCODE_URL,
json=submission["dob_addr_response"],
)

request, s = pull_apart_join_form_submission(submission)

response = self.c.post("/api/v1/join/", request, content_type="application/json")
code = 202
self.assertEqual(
code,
response.status_code,
f"status code incorrect for Valid Join Form. Should be {code}, but got {response.status_code}.\n Response is: {response.content.decode('utf-8')}",
)

# TODO: Loop thru and chom
changed_info = response.data["changed_info"]
if changed_info:
for k, v in request.items():
if k in changed_info.keys():
request[k] = changed_info[k]

response = self.c.post("/api/v1/join/", request, content_type="application/json")
code = 201
self.assertEqual(
Expand Down
51 changes: 50 additions & 1 deletion src/meshapi/views/forms.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
from datetime import date
from functools import reduce
from json.decoder import JSONDecodeError
from typing import Optional

from django.db import IntegrityError, transaction
from django.db.models import Q
Expand All @@ -23,6 +24,7 @@
from meshapi.util.django_pglocks import advisory_lock
from meshapi.util.network_number import NETWORK_NUMBER_MAX, NETWORK_NUMBER_MIN, get_next_available_network_number
from meshapi.validation import (
NYCAddressInfo,
geocode_nyc_address,
normalize_phone_number,
validate_email_address,
Expand Down Expand Up @@ -74,6 +76,7 @@
"install_id": serializers.UUIDField(),
"install_number": serializers.IntegerField(),
"member_exists": serializers.BooleanField(),
"changed_info": serializers.DictField(),
},
),
description="Request received, an install has been created (along with member and "
Expand Down Expand Up @@ -117,7 +120,7 @@
formatted_phone_number = normalize_phone_number(r.phone) if r.phone else None

try:
nyc_addr_info = geocode_nyc_address(r.street_address, r.city, r.state, r.zip)
nyc_addr_info: Optional[NYCAddressInfo] = geocode_nyc_address(r.street_address, r.city, r.state, r.zip)
except ValueError:
return Response(
{
Expand All @@ -134,6 +137,51 @@
{"detail": "Your address could not be validated."}, status=status.HTTP_500_INTERNAL_SERVER_ERROR
)

changed_info: dict[str, str | int] = {}

# TODO: Notify member if we changed any of their information
# Name (won't touch), email (won't touch), phone, st addr, unit (won't touch), city, State, Zip
if formatted_phone_number and r.phone != formatted_phone_number:
WillNilges marked this conversation as resolved.
Show resolved Hide resolved
logging.warning(f"Changed phone_number: {formatted_phone_number} != {r.phone}")
github-advanced-security[bot] marked this conversation as resolved.
Fixed
Show resolved Hide resolved
changed_info["phone"] = formatted_phone_number

if r.street_address != nyc_addr_info.street_address:
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:
Copy link
Member

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?

Copy link
Collaborator Author

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.

logging.warning(f"Changed city: {r.city} != {nyc_addr_info.city}")
changed_info["city"] = nyc_addr_info.city

if r.state != nyc_addr_info.state:
logging.warning(f"Changed state: {r.state} != {nyc_addr_info.state}")
changed_info["state"] = nyc_addr_info.state

if r.zip != nyc_addr_info.zip:
logging.warning(f"Changed zip: {r.zip} != {nyc_addr_info.zip}")
changed_info["zip"] = nyc_addr_info.zip

# Let the member know we need to confirm some info with them. We'll send
# back a dictionary with the info that needs confirming.
# This is not a rejection. We expect another join form submission with all
# of this info in place for us.
if changed_info:
return Response(
{
"detail": "Please confirm a few details.",
"building_id": None,
"member_id": None,
"install_id": None,
"install_number": None,
# If this is an existing member, then set a flag to let them know we have
# their information in case they need to update anything.
"member_exists": None,
# TODO: Add a "trust me bro" parameter. Maybe log if it breaks
"changed_info": changed_info,
},
status=status.HTTP_202_ACCEPTED,
)

# Check if there's an existing member. Group members by matching on both email and phone
# A member can have multiple install requests, if they move apartments for example
existing_member_filter_criteria = []
Expand Down Expand Up @@ -314,6 +362,7 @@
# If this is an existing member, then set a flag to let them know we have
# their information in case they need to update anything.
"member_exists": True if len(existing_members) > 0 else False,
"changed_info": {},
},
status=status.HTTP_201_CREATED,
)
Expand Down
2 changes: 1 addition & 1 deletion src/meshapi/views/query_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ class Meta:
parameters=[query_form_password_param],
summary="Query & filter based on Install attributes. "
"Results are returned as flattened spreadsheet row style output",
auth=[],
auth=["bearer"],
WillNilges marked this conversation as resolved.
Show resolved Hide resolved
),
)
class QueryInstall(FilterRequiredListAPIView):
Expand Down
2 changes: 1 addition & 1 deletion src/meshapi/widgets.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ class PanoramaViewer(JSONFormWidget):
def __init__(self, schema: dict):
super().__init__(schema)

def pano_get_context(self, name: str, value: str) -> dict:
def pano_get_context(self, name: str, value: str = "[]") -> dict:
WillNilges marked this conversation as resolved.
Show resolved Hide resolved
value_as_array = json.loads(value)
return {
"widget": {
Expand Down
Loading