-
-
Notifications
You must be signed in to change notification settings - Fork 1
Division Airport Request Deletion #94
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
Conversation
… code and name fields to airports table
… code and name fields to airports table
Greptile OverviewGreptile SummaryThis PR adds two main features: airport country information storage and a new endpoint for deleting division airport requests. Changes OverviewAirport Country Information:
Division Airport Request Deletion:
Critical Issues Found🔴 Runtime Crash on Missing Input (CRITICAL): Type Inconsistency: Positive Aspects
Confidence Score: 2/5
Important Files ChangedFile Analysis
|
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.
3 files reviewed, 3 comments
| `DELETE FROM division_airports | ||
| WHERE division_id = ? AND icao = ? AND status IN ('pending', 'rejected') | ||
| RETURNING id`, | ||
| [divisionId, icao.toUpperCase()], |
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.
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:
| [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 }; |
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.
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:
| 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.| country?: { | ||
| code: string; | ||
| name: string; | ||
| }; |
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.
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:
- Transforming the database result to match the interface (add a
countryobject whencountry_codeexists) - 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.
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:
airportstable schema to includecountry_codeandcountry_namecolumns for storing airport country informationAirportDatainterface and related TypeScript code to handle the newcountryfield, including reading, writing, and merging country data from the databaseDivision airport request management:
DELETE /divisions/:id/airportsallowing division members to delete airport requests with pending or rejected statusdeleteAirportRequestmethod in theDivisionServiceclass with proper role and status validationAdditional 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: