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

Revisit all TODOs, creating issues for the ones that still need to be completed #528

Merged
merged 3 commits into from
Sep 16, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
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 infra/helm/meshdb/charts/celery/templates/deployment.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ spec:
{{- toYaml $.Values.resources | nindent 12 }}
command: {{ toJson .command }}
# TODO (willnilges): Fix this in a later PR
# https://github.com/nycmeshnet/meshdb/issues/519
envFrom:
- configMapRef:
name: meshdbconfig
Expand Down
15 changes: 8 additions & 7 deletions pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,8 @@ version = "0.1"
dependencies = [
"celery[redis]==5.3.*",
"django==4.2.*",
# We need djangorestframework features that are coming in 3.15.*
# but that release hasn't shipped yet, so grab the commit that is likely to ship.
# This will likely have a small diff with the final 3.15.* release, but is much better
# then pulling their master branch (which changes constantly)
# FIXME: Go back to PyPI once https://github.com/encode/django-rest-framework/pull/8794 is merged
# FIXME: Go back to PyPI once DRF adds support for using Django's read-only permissions
# https://github.com/nycmeshnet/meshdb/issues/524
"djangorestframework@git+https://github.com/encode/django-rest-framework.git@4bbfa8d4556b5847e91ba95f457cc862b7a0f027",
"drf-hooks==0.1.3",
"psycopg2-binary==2.9.*",
Expand All @@ -24,15 +21,19 @@ dependencies = [
"django-cors-headers==4.3.*",
"nameparser==1.1.*",
"inflect==7.0.*",
"fastkml[lxml]==1.0a12", # FIXME: Update me to stable, once 1.0 is released officially
# FIXME: Update me to stable, once 1.0 is released officially
# https://github.com/nycmeshnet/meshdb/issues/525
"fastkml[lxml]==1.0a12",
"drf-spectacular==0.27.*",
"djangorestframework-dataclasses==1.3.*",
"django-nonrelated-inlines==0.2.*",
"django-filter==24.1",
# FIXME: Go back to PyPI when https://github.com/bhch/django-jsonform/pull/162 is merged
# FIXME: Go back to PyPI when https://github.com/bhch/django-jsonform/pull/162 is available on PyPi
# https://github.com/nycmeshnet/meshdb/issues/526
"django-jsonform@git+https://github.com/willnilges/django-jsonform.git@51cbed42ccdaec81a35c97a919d054e2c9ca0207",
"faker==24.3.*",
# FIXME: Go back to PyPI when https://github.com/jazzband/django-dbbackup/pull/515 or https://github.com/jazzband/django-dbbackup/pull/511 is merged
# https://github.com/nycmeshnet/meshdb/issues/527
"django-dbbackup@git+https://github.com/willnilges/django-dbbackup.git@62048411ff5beac4beac1578f686824214f1f33a",
"django-storages==1.14.*",
"django-import-export==4.0.*",
Expand Down
2 changes: 0 additions & 2 deletions src/meshapi/management/commands/scramble_members.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,6 @@


# Uses faker to get fake names, emails, and phone numbers
# TODO: Instead of modifying real data, generate a completely fake database from
# scratch :)
WillNilges marked this conversation as resolved.
Show resolved Hide resolved
class Command(BaseCommand):
help = "Updates all members with fake name, email, and phone number. Clears notes."

Expand Down
1 change: 0 additions & 1 deletion src/meshapi/templates/admin/install_tabular.html
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,6 @@ <h2>{{ inline_admin_formset.opts.verbose_name_plural|capfirst }}</h2>
</tbody>
</table>
</fieldset>
<!--TODO: Add collapsible menu to add existing buildings...?-->
<div class="inline-action-row">
{% if inline_admin_formset.opts.add_button %}
<a href="{{ request.scheme }}://{{ request.get_host }}{% url 'admin:'|add:inline_admin_formset.opts.opts.db_table|add:'_add' %}?{{ inline_admin_formset.opts.reverse_relation }}={{ object_id }}" class="addlink">Add {{ inline_admin_formset.opts.verbose_name|capfirst }}</a>
Expand Down
1 change: 1 addition & 0 deletions src/meshapi/tests/test_uisp_import.py
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,7 @@ def test_get_link_type(self):
self.assertEqual(
# TODO: Once 6 GHz becomes a thing, this will probably need to be tweaked
# for now we consider anything < 7 GHz as 5 GHz
# https://github.com/nycmeshnet/meshdb/issues/518
get_link_type({"type": "wireless", "frequency": 6100}),
Link.LinkType.FIVE_GHZ,
)
Expand Down
1 change: 1 addition & 0 deletions src/meshapi/tests/test_update_panos_github.py
Original file line number Diff line number Diff line change
Expand Up @@ -176,6 +176,7 @@ def test_parse_pano_title(self):
result = PanoramaTitle.from_filename(case)
self.assertEqual(result, expected, f"Expected: {expected}. Result: {result}.")
# TODO: Assert that all the URL functions and stuff work
# https://github.com/nycmeshnet/meshdb/issues/520

# The GitHub does not have a perfectly uniform set of files.
def test_parse_pano_bad_title(self):
Expand Down
11 changes: 4 additions & 7 deletions src/meshapi/validation.py
Original file line number Diff line number Diff line change
Expand Up @@ -92,10 +92,6 @@ def __init__(self, street_address: str, city: str, state: str, zip_code: int):
# the closest matching street address it can find, so check that
# the ZIP of what we entered matches what we got.

# FIXME (willnilges): Found an edge case where if you enter an address
# that's not in the Zip code, it will print the "not within city limits"
# error. Either the error message needs to be re-worked, or additional
# validation is required to figure out exactly what is wrong.
found_zip = int(nyc_planning_resp["features"][0]["properties"]["postalcode"])
if found_zip != zip_code:
raise AddressError(
Expand All @@ -112,9 +108,10 @@ def __init__(self, street_address: str, city: str, state: str, zip_code: int):
self.state = addr_props["region_a"]
self.zip = int(addr_props["postalcode"])

# TODO (willnilges): Bail if no BIN. Given that we're guaranteeing this is NYC, if
# there is no BIN, then we've really foweled something up
if int(addr_props["addendum"]["pad"]["bin"]) in INVALID_BIN_NUMBERS:
if (
not addr_props.get("addendum", {}).get("pad", {}).get("bin")
or int(addr_props["addendum"]["pad"]["bin"]) in INVALID_BIN_NUMBERS
):
raise AddressError(
f"(NYC) Could not find address '{street_address}, {city}, {state} {zip_code}'. "
f"DOB API returned invalid BIN: {addr_props['addendum']['pad']['bin']}"
Expand Down
1 change: 1 addition & 0 deletions src/meshapi/views/map.py
Original file line number Diff line number Diff line change
Expand Up @@ -182,6 +182,7 @@ class MapDataLinkList(generics.ListAPIView):
# TODO: Possibly re-enable the below filters? They make make the map arguably more accurate,
# but less consistent with the current one by removing links between devices that are
# inactive in UISP
# https://github.com/nycmeshnet/meshdb/issues/521
# .exclude(from_device__status=Device.DeviceStatus.INACTIVE)
# .exclude(to_device__status=Device.DeviceStatus.INACTIVE)
)
Expand Down
1 change: 1 addition & 0 deletions src/meshdb/templates/drf_spectacular/swagger_ui.html
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
// convention that a closed padlock represents an unlocked endpoint.
// Much anger expressed here: https://github.com/swagger-api/swagger-ui/issues/4402
// in the future we should try to get this working
// https://github.com/nycmeshnet/meshdb/issues/523
{#console.log(JSON.stringify(versions));#}
{#var lockedIcon = document.querySelector(".svg-assets defs symbol#locked");#}
{#var unlockedIcon = document.querySelector(".svg-assets defs symbol#unlocked");#}
Expand Down
8 changes: 1 addition & 7 deletions src/meshdb/utils/spreadsheet_import/parse_building.py
Original file line number Diff line number Diff line change
Expand Up @@ -203,13 +203,7 @@ def nop(*args, **kwargs):

latitude = row.latitude
longitude = row.longitude
altitude = (
# TODO: Change this to match new DOB ID if changed from spreadsheet?
# Would require another API call
row.altitude
if row.altitude and row.altitude >= 0
else None
)
altitude = row.altitude if row.altitude and row.altitude >= 0 else None

existing_building = get_existing_building(
address_result.discovered_bin or dob_bin, address_result.address, (latitude, longitude)
Expand Down
4 changes: 3 additions & 1 deletion src/meshdb/utils/spreadsheet_import/parse_install.py
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,9 @@ def create_install(row: SpreadsheetRow) -> Optional[models.Install]:
install = models.Install(
install_number=row.id,
status=translate_spreadsheet_status_to_db_status(row.status),
ticket_id=None, # TODO: Figure out if we can export data from OSTicket to back-fill this
ticket_id=None,
# TODO: Figure out if we can export data from OSTicket to back-fill this
# https://github.com/nycmeshnet/meshdb/issues/510
request_date=row.request_date.date(),
install_date=row.installDate,
abandon_date=row.abandonDate,
Expand Down
1 change: 1 addition & 0 deletions src/meshdb/utils/spreadsheet_import/parse_link.py
Original file line number Diff line number Diff line change
Expand Up @@ -258,6 +258,7 @@ def load_links_supplement_with_uisp(spreadsheet_links: List[SpreadsheetLink]):
for spreadsheet_link in spreadsheet_links:
try:
# TODO: this method of node lookup doesn't work for campus links like the vernon APs
# https://github.com/nycmeshnet/meshdb/issues/514
from_node = get_node_from_spreadsheet_id(spreadsheet_link.from_node_id)
to_node = get_node_from_spreadsheet_id(spreadsheet_link.to_node_id)

Expand Down
Loading