-
Notifications
You must be signed in to change notification settings - Fork 3
<WIP> Admin Data Editor #407
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
base: main
Are you sure you want to change the base?
Conversation
|
[diff-counting] Significant lines: 770. |
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.
Although just a WIP PR, this is a great first step towards the larger admin data manipulation feature. The code is organized very well and well written. No current comments on the code or logic. Good job!
\ - Implemented apartment information edit endpoint - Implemented create new apartment endpoint - Implemented admin page apartment pagination, sorting, and editing functionalities
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.
This looks great, the admin page structure is coming together well. I've left a few comments on small things I noticed—mainly some error handling for the apartments data loading, a type safety improvement (using ApartmentData[] instead of any[]), and a couple of efficiency suggestions. Excited to see the full admin dashboard come together!
|
|
||
| // Use the same API endpoint as AdminPage | ||
| const response = await axios.get<ApiResponse>( | ||
| 'http://localhost:8080/api/page-data/home/1000/numReviews' |
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.
Error message says localhost:3000 but the code uses localhost:8080.
| const host = req.get('host'); | ||
| const baseUrl = `${protocol}://${host}`; | ||
|
|
||
| const travelTimesResponse = await axios.post(`${baseUrl}/api/calculate-travel-times`, { |
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.
Extracting the travel time calculation logic into a shared function could prevent an unnecessary HTTP request which might be more efficient
| const [reportedData, setReportedData] = useState<ReviewWithId[]>([]); | ||
| const [reportedExpanded, setReportedExpanded] = useState(true); | ||
| const { container, sectionHeader, expand, expandOpen } = useStyles(); | ||
| const [apartments, setApartments] = useState<any[]>([]); |
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.
should use ApartmentData[] instead of any[]
| if (data && data.buildingData && Array.isArray(data.buildingData)) { | ||
| setApartments(data.buildingData); | ||
| } else { | ||
| console.error('Failed to load apartments data'); |
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.
This is missing error handling to let the user know if something went wrong
|
This is a really strong PR overall. The new Admin Data Editor adds a lot of practical functionality, from apartment pagination and inline editing to new admin-only endpoints that support secure apartment creation and updates. The backend routes are well-structured, with clear validation and consistent error handling, and the front-end flow feels intuitive with good use of optimistic updates and sorting logic. That said, there are a few fixes needed before merge: the Firestore query in add-apartment uses multi-field range filters on both latitude and longitude, which Firestore doesn’t support—you’ll need a geospatial workaround. The backend should also move admins out of the frontend constants, rename REACT_APP_MAPS_API_KEY to a server-only key, and call the travel-time logic directly instead of making an HTTP request to itself. Finally, return 201 for successful apartment creation and 200 for preliminary confirmations. Once those issues are addressed, this will be a great, production-ready addition. |
Summary
This pull request is the first step towards implementing feature of allowing the admins of CUAPTS to add, edit, delete apartment, reviews, and landlord data from the application through UI. This would simplify the data changing process, making data change a feature rather than a development chores. This feature will also allow more dynamic response to newly found apartments, as we could reflect these findings quickly on the website.
Test Plan
Notes
Breaking Changes