Skip to content

Conversation

@llamavert
Copy link
Contributor

@llamavert llamavert commented Jan 13, 2026

Summary

This PR introduces support for storing and handling airport country information in the database and API, and adds a new endpoint to allow division members to delete airport requests with pending or rejected status.

Changes Made

  • Airport country information support:

    • Updated the airports table schema to include country_code and country_name columns for storing airport country information
    • Updated the AirportData interface and related TypeScript code to handle the new country field, including reading, writing, and merging country data from the database
  • Division airport request management:

    • Added new API endpoint DELETE /divisions/:id/airports allowing division members to delete airport requests with pending or rejected status
    • Implemented deleteAirportRequest method in the DivisionService class with proper role and status validation
    • Added OpenAPI documentation and authorization checks for the new endpoint
    • Integrated analytics tracking for deletion events

Additional Information

The airport deletion endpoint includes proper validation to ensure only requests with pending or rejected status can be deleted, maintaining data integrity for approved requests.

Author Information

Discord Username:
VATSIM CID:


Checklist:

  • Have you followed the guidelines in our Contributing document?
  • Have you checked to ensure there aren't other open Pull Requests for the same update/change?

@greptile-apps
Copy link

greptile-apps bot commented Jan 13, 2026

Greptile Overview

Greptile Summary

This PR adds two main features: airport country information storage and a new endpoint for deleting division airport requests.

Changes Overview

Airport Country Information:

  • Adds country_code and country_name columns to the airports table
  • Updates the AirportData interface to include a nested country object
  • Modifies airport fetching logic to store and retrieve country data from the external API

Division Airport Request Deletion:

  • Adds DELETE /divisions/:id/airports endpoint allowing division members to delete pending or rejected airport requests
  • Implements authorization check to ensure only division members can delete requests
  • Restricts deletion to requests with pending or rejected status (approved requests cannot be deleted)
  • Includes analytics tracking for deletion events

Critical Issues Found

🔴 Runtime Crash on Missing Input (CRITICAL):
The new DELETE endpoint and the deleteAirportRequest method lack input validation for the icao parameter. If a request is sent without the icao field in the JSON body (or with null/undefined), the code will crash when attempting to call .toUpperCase() on an undefined value. This is a critical bug that will cause 500 errors in production.

Type Inconsistency:
The AirportData interface defines country as a nested object, but the actual data returned from getAirport() uses flat country_code and country_name fields, creating a mismatch between the type definition and runtime behavior.

Positive Aspects

  • Authorization logic correctly ensures only division members can delete requests
  • Status-based deletion restriction properly prevents deletion of approved requests
  • Analytics tracking is properly implemented
  • Database schema changes are clean and use nullable columns appropriately

Confidence Score: 2/5

  • This PR contains a critical bug that will cause runtime crashes in production
  • The missing input validation on the icao parameter in both the API endpoint and service method will cause the application to crash when the field is missing or null. This is a guaranteed runtime error that will result in 500 responses. The issue must be fixed before merging.
  • src/index.ts and src/services/divisions.ts require immediate attention - add input validation for the icao parameter to prevent runtime crashes

Important Files Changed

File Analysis

Filename Score Overview
bun.lock 5/5 Minor dependency version updates (hono, @types/node, typescript-eslint, wrangler) - routine maintenance
schema.sql 4/5 Added country_code and country_name columns to airports table - clean addition but lacks migration strategy
src/services/airport.ts 3/5 Added country field support to AirportData interface and database operations - has type inconsistency issue
src/services/divisions.ts 2/5 Added deleteAirportRequest method with authorization - has critical null/undefined handling bug
src/index.ts 2/5 Added DELETE /divisions/:id/airports endpoint - missing input validation causes runtime crash

Copy link

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

3 files reviewed, 3 comments

Edit Code Review Agent Settings | Greptile

`DELETE FROM division_airports
WHERE division_id = ? AND icao = ? AND status IN ('pending', 'rejected')
RETURNING id`,
[divisionId, icao.toUpperCase()],
Copy link

Choose a reason for hiding this comment

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

The code will crash with a runtime error if icao is undefined or null when calling toUpperCase(). This can happen if the API endpoint receives a JSON body without the icao field or with icao: null.

Add validation before using the parameter:

Suggested change
[divisionId, icao.toUpperCase()],
if (!icao) throw new HttpError(400, 'ICAO code is required');
// Delete only if the division has requested this ICAO and status is pending or rejected
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/services/divisions.ts
Line: 245:245

Comment:
The code will crash with a runtime error if `icao` is `undefined` or `null` when calling `toUpperCase()`. This can happen if the API endpoint receives a JSON body without the `icao` field or with `icao: null`.

Add validation before using the parameter:

```suggestion
		if (!icao) throw new HttpError(400, 'ICAO code is required');
		
		// Delete only if the division has requested this ICAO and status is pending or rejected
```

How can I resolve this? If you propose a fix, please make it concise.

if (!vatsimToken) return c.text('Unauthorized', 401);

const divisionId = parseInt(c.req.param('id'));
const { icao } = (await c.req.json()) as { icao: string };
Copy link

Choose a reason for hiding this comment

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

Missing input validation for the icao field. If the request body doesn't include icao or if it's null/undefined, the code will crash when deleteAirportRequest tries to call icao.toUpperCase() on line 245 of divisions.ts.

Add validation after parsing the JSON:

Suggested change
const { icao } = (await c.req.json()) as { icao: string };
const { icao } = (await c.req.json()) as { icao: string };
if (!icao) {
return c.text('ICAO code is required', 400);
}
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/index.ts
Line: 2196:2196

Comment:
Missing input validation for the `icao` field. If the request body doesn't include `icao` or if it's `null`/`undefined`, the code will crash when `deleteAirportRequest` tries to call `icao.toUpperCase()` on line 245 of divisions.ts.

Add validation after parsing the JSON:

```suggestion
	const { icao } = (await c.req.json()) as { icao: string };
	if (!icao) {
		return c.text('ICAO code is required', 400);
	}
```

How can I resolve this? If you propose a fix, please make it concise.

Comment on lines +12 to +15
country?: {
code: string;
name: string;
};
Copy link

Choose a reason for hiding this comment

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

The AirportData interface defines country as a nested object with code and name properties, but the database stores these as separate country_code and country_name columns. When airports are returned from the database (lines 286, 419, 430), they contain the flat structure (country_code, country_name) rather than the nested structure defined in this interface.

This creates a type mismatch between the interface definition and the actual runtime data structure. Consider either:

  1. Transforming the database result to match the interface (add a country object when country_code exists)
  2. Or updating the interface to reflect the actual flat structure returned by getAirport()
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/services/airport.ts
Line: 12:15

Comment:
The `AirportData` interface defines `country` as a nested object with `code` and `name` properties, but the database stores these as separate `country_code` and `country_name` columns. When airports are returned from the database (lines 286, 419, 430), they contain the flat structure (`country_code`, `country_name`) rather than the nested structure defined in this interface.

This creates a type mismatch between the interface definition and the actual runtime data structure. Consider either:
1. Transforming the database result to match the interface (add a `country` object when `country_code` exists)
2. Or updating the interface to reflect the actual flat structure returned by `getAirport()`

How can I resolve this? If you propose a fix, please make it concise.

@llamavert llamavert deleted the feat(airports) branch January 13, 2026 13:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants