Skip to content

Commit

Permalink
Revisit all TODOs, creating issues for the ones that still need to be…
Browse files Browse the repository at this point in the history
… completed (#528)

* Resolve all TODOs (or link issues to track the work for them)

* Kick CI
  • Loading branch information
Andrew-Dickinson authored Sep 16, 2024
1 parent ab0c8a9 commit 0e486c6
Show file tree
Hide file tree
Showing 12 changed files with 22 additions and 25 deletions.
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 :)
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

0 comments on commit 0e486c6

Please sign in to comment.