From 34c0ad2920c73f7f1939d1335fad9c8c0319536e Mon Sep 17 00:00:00 2001 From: jackiettran <41605212+jackiettran@users.noreply.github.com> Date: Wed, 8 Oct 2025 23:03:28 -0400 Subject: [PATCH] fixed cors bug, separating rental confirmation for owner and renter, removing condition checks from my-listings --- backend/server.js | 8 +- backend/services/emailService.js | 76 ++++++-- .../tests/unit/services/emailService.test.js | 168 ++++++++++++++++++ frontend/src/components/ReturnStatusModal.tsx | 20 ++- frontend/src/pages/MyListings.tsx | 71 +------- 5 files changed, 249 insertions(+), 94 deletions(-) diff --git a/backend/server.js b/backend/server.js index aa65c4b..e19d5e7 100644 --- a/backend/server.js +++ b/backend/server.js @@ -73,10 +73,7 @@ app.use(morgan("combined", { stream: logger.stream })); // API request/response logging app.use("/api/", apiLogger); -// General rate limiting for all routes -app.use("/api/", generalLimiter); - -// CORS with security settings +// CORS with security settings (must come BEFORE rate limiter to ensure headers on all responses) app.use( cors({ origin: process.env.FRONTEND_URL || "http://localhost:3000", @@ -85,6 +82,9 @@ app.use( }) ); +// General rate limiting for all routes +app.use("/api/", generalLimiter); + // Body parsing with size limits app.use( bodyParser.json({ diff --git a/backend/services/emailService.js b/backend/services/emailService.js index 8733444..a99cc34 100644 --- a/backend/services/emailService.js +++ b/backend/services/emailService.js @@ -415,6 +415,11 @@ class EmailService { } async sendRentalConfirmationEmails(rental) { + const results = { + ownerEmailSent: false, + renterEmailSent: false, + }; + try { // Get owner and renter emails const owner = await User.findByPk(rental.ownerId, { @@ -444,30 +449,67 @@ class EmailService { metadata: { rentalStart: rental.startDateTime }, }; - // Send email to owner + // Send email to owner - independent error handling if (owner?.email) { - await this.sendRentalConfirmation( - owner.email, - ownerNotification, - rental - ); - console.log(`Rental confirmation email sent to owner: ${owner.email}`); + try { + const ownerResult = await this.sendRentalConfirmation( + owner.email, + ownerNotification, + rental + ); + if (ownerResult.success) { + console.log( + `Rental confirmation email sent to owner: ${owner.email}` + ); + results.ownerEmailSent = true; + } else { + console.error( + `Failed to send rental confirmation email to owner (${owner.email}):`, + ownerResult.error + ); + } + } catch (error) { + console.error( + `Failed to send rental confirmation email to owner (${owner.email}):`, + error.message + ); + } } - // Send email to renter + // Send email to renter - independent error handling if (renter?.email) { - await this.sendRentalConfirmation( - renter.email, - renterNotification, - rental - ); - console.log( - `Rental confirmation email sent to renter: ${renter.email}` - ); + try { + const renterResult = await this.sendRentalConfirmation( + renter.email, + renterNotification, + rental + ); + if (renterResult.success) { + console.log( + `Rental confirmation email sent to renter: ${renter.email}` + ); + results.renterEmailSent = true; + } else { + console.error( + `Failed to send rental confirmation email to renter (${renter.email}):`, + renterResult.error + ); + } + } catch (error) { + console.error( + `Failed to send rental confirmation email to renter (${renter.email}):`, + error.message + ); + } } } catch (error) { - console.error("Error sending rental confirmation emails:", error); + console.error( + "Error fetching user data for rental confirmation emails:", + error + ); } + + return results; } } diff --git a/backend/tests/unit/services/emailService.test.js b/backend/tests/unit/services/emailService.test.js index b9590ce..b34d22c 100644 --- a/backend/tests/unit/services/emailService.test.js +++ b/backend/tests/unit/services/emailService.test.js @@ -248,4 +248,172 @@ describe('EmailService', () => { expect(result.success).toBe(true); }); }); + + describe('sendRentalConfirmationEmails', () => { + const { User } = require('../../../models'); + + beforeEach(async () => { + mockSend.mockResolvedValue({ MessageId: 'test-message-id' }); + await emailService.initialize(); + }); + + it('should send emails to both owner and renter successfully', async () => { + const mockOwner = { email: 'owner@example.com' }; + const mockRenter = { email: 'renter@example.com' }; + + User.findByPk + .mockResolvedValueOnce(mockOwner) // First call for owner + .mockResolvedValueOnce(mockRenter); // Second call for renter + + const rental = { + id: 1, + ownerId: 10, + renterId: 20, + item: { name: 'Test Item' }, + startDateTime: '2024-01-15T10:00:00Z', + endDateTime: '2024-01-17T10:00:00Z' + }; + + const results = await emailService.sendRentalConfirmationEmails(rental); + + expect(results.ownerEmailSent).toBe(true); + expect(results.renterEmailSent).toBe(true); + expect(mockSend).toHaveBeenCalledTimes(2); + }); + + it('should send renter email even if owner email fails', async () => { + const mockOwner = { email: 'owner@example.com' }; + const mockRenter = { email: 'renter@example.com' }; + + User.findByPk + .mockResolvedValueOnce(mockOwner) + .mockResolvedValueOnce(mockRenter); + + // First call (owner) fails, second call (renter) succeeds + mockSend + .mockRejectedValueOnce(new Error('SES Error for owner')) + .mockResolvedValueOnce({ MessageId: 'renter-message-id' }); + + const rental = { + id: 1, + ownerId: 10, + renterId: 20, + item: { name: 'Test Item' }, + startDateTime: '2024-01-15T10:00:00Z', + endDateTime: '2024-01-17T10:00:00Z' + }; + + const results = await emailService.sendRentalConfirmationEmails(rental); + + expect(results.ownerEmailSent).toBe(false); + expect(results.renterEmailSent).toBe(true); + expect(mockSend).toHaveBeenCalledTimes(2); + }); + + it('should send owner email even if renter email fails', async () => { + const mockOwner = { email: 'owner@example.com' }; + const mockRenter = { email: 'renter@example.com' }; + + User.findByPk + .mockResolvedValueOnce(mockOwner) + .mockResolvedValueOnce(mockRenter); + + // First call (owner) succeeds, second call (renter) fails + mockSend + .mockResolvedValueOnce({ MessageId: 'owner-message-id' }) + .mockRejectedValueOnce(new Error('SES Error for renter')); + + const rental = { + id: 1, + ownerId: 10, + renterId: 20, + item: { name: 'Test Item' }, + startDateTime: '2024-01-15T10:00:00Z', + endDateTime: '2024-01-17T10:00:00Z' + }; + + const results = await emailService.sendRentalConfirmationEmails(rental); + + expect(results.ownerEmailSent).toBe(true); + expect(results.renterEmailSent).toBe(false); + expect(mockSend).toHaveBeenCalledTimes(2); + }); + + it('should handle both emails failing gracefully', async () => { + const mockOwner = { email: 'owner@example.com' }; + const mockRenter = { email: 'renter@example.com' }; + + User.findByPk + .mockResolvedValueOnce(mockOwner) + .mockResolvedValueOnce(mockRenter); + + // Both calls fail + mockSend + .mockRejectedValueOnce(new Error('SES Error for owner')) + .mockRejectedValueOnce(new Error('SES Error for renter')); + + const rental = { + id: 1, + ownerId: 10, + renterId: 20, + item: { name: 'Test Item' }, + startDateTime: '2024-01-15T10:00:00Z', + endDateTime: '2024-01-17T10:00:00Z' + }; + + const results = await emailService.sendRentalConfirmationEmails(rental); + + expect(results.ownerEmailSent).toBe(false); + expect(results.renterEmailSent).toBe(false); + expect(mockSend).toHaveBeenCalledTimes(2); + }); + + it('should handle missing owner email', async () => { + const mockOwner = { email: null }; + const mockRenter = { email: 'renter@example.com' }; + + User.findByPk + .mockResolvedValueOnce(mockOwner) + .mockResolvedValueOnce(mockRenter); + + const rental = { + id: 1, + ownerId: 10, + renterId: 20, + item: { name: 'Test Item' }, + startDateTime: '2024-01-15T10:00:00Z', + endDateTime: '2024-01-17T10:00:00Z' + }; + + const results = await emailService.sendRentalConfirmationEmails(rental); + + expect(results.ownerEmailSent).toBe(false); + expect(results.renterEmailSent).toBe(true); + expect(mockSend).toHaveBeenCalledTimes(1); + }); + + it('should handle missing renter email', async () => { + const mockOwner = { email: 'owner@example.com' }; + const mockRenter = { email: null }; + + User.findByPk + .mockResolvedValueOnce(mockOwner) + .mockResolvedValueOnce(mockRenter); + + const rental = { + id: 1, + ownerId: 10, + renterId: 20, + item: { name: 'Test Item' }, + startDateTime: '2024-01-15T10:00:00Z', + endDateTime: '2024-01-17T10:00:00Z' + }; + + const results = await emailService.sendRentalConfirmationEmails(rental); + + expect(results.ownerEmailSent).toBe(true); + expect(results.renterEmailSent).toBe(false); + expect(mockSend).toHaveBeenCalledTimes(1); + }); + }); }); \ No newline at end of file diff --git a/frontend/src/components/ReturnStatusModal.tsx b/frontend/src/components/ReturnStatusModal.tsx index c71b712..171bcea 100644 --- a/frontend/src/components/ReturnStatusModal.tsx +++ b/frontend/src/components/ReturnStatusModal.tsx @@ -1,4 +1,4 @@ -import React, { useState, useEffect, useCallback } from "react"; +import React, { useState, useEffect, useCallback, useMemo } from "react"; import { rentalAPI, conditionCheckAPI } from "../services/api"; import { Rental } from "../types"; @@ -69,6 +69,18 @@ const ReturnStatusModal: React.FC = ({ } }, [show, rental]); + // Create blob URLs for photo previews and clean them up + const photoBlobUrls = useMemo(() => { + return photos.map(photo => URL.createObjectURL(photo)); + }, [photos]); + + // Cleanup blob URLs when photos change or component unmounts + useEffect(() => { + return () => { + photoBlobUrls.forEach(url => URL.revokeObjectURL(url)); + }; + }, [photoBlobUrls]); + const formatCurrency = (amount: number | string | undefined) => { const numAmount = Number(amount) || 0; return `$${numAmount.toFixed(2)}`; @@ -325,7 +337,7 @@ const ReturnStatusModal: React.FC = ({ } }; - const handleClose = () => { + const handleClose = useCallback(() => { // Reset all states setStatusOptions({ returned: false, @@ -345,7 +357,7 @@ const ReturnStatusModal: React.FC = ({ setReplacementCost(""); setProofOfOwnership([]); onHide(); - }; + }, [onHide]); const handleBackdropClick = useCallback( (e: React.MouseEvent) => { @@ -630,7 +642,7 @@ const ReturnStatusModal: React.FC = ({
{`Photo { checkType: string; } | null>(null); const [availableChecks, setAvailableChecks] = useState([]); - const [conditionChecks, setConditionChecks] = useState([]); const [showReturnStatusModal, setShowReturnStatusModal] = useState(false); const [rentalForReturn, setRentalForReturn] = useState(null); @@ -69,12 +68,6 @@ const MyListings: React.FC = () => { fetchAvailableChecks(); }, [user]); - useEffect(() => { - if (ownerRentals.length > 0) { - fetchConditionChecks(); - } - }, [ownerRentals]); - const fetchMyListings = async () => { if (!user) return; @@ -148,31 +141,6 @@ const MyListings: React.FC = () => { } }; - const fetchConditionChecks = async () => { - try { - // Fetch condition checks for all owner rentals - const allChecks: any[] = []; - for (const rental of ownerRentals) { - try { - const response = await conditionCheckAPI.getConditionChecks( - rental.id - ); - const checks = Array.isArray(response.data.conditionChecks) - ? response.data.conditionChecks - : []; - allChecks.push(...checks); - } catch (err) { - // Continue even if one rental fails - console.error(`Failed to fetch checks for rental ${rental.id}:`, err); - } - } - setConditionChecks(allChecks); - } catch (err: any) { - console.error("Failed to fetch condition checks:", err); - setConditionChecks([]); - } - }; - // Owner functionality handlers const handleAcceptRental = async (rentalId: string) => { try { @@ -271,7 +239,6 @@ const MyListings: React.FC = () => { const handleConditionCheckSuccess = () => { fetchAvailableChecks(); - fetchConditionChecks(); alert("Condition check submitted successfully!"); }; @@ -283,20 +250,10 @@ const MyListings: React.FC = () => { ); }; - const getCompletedChecksForRental = (rentalId: string) => { - if (!Array.isArray(conditionChecks)) return []; - return conditionChecks.filter( - (check) => - check.rentalId === rentalId && - (check.checkType === "pre_rental_owner" || - check.checkType === "post_rental_owner") - ); - }; - - // Filter owner rentals + // Filter owner rentals - exclude cancelled (shown in Rental History) const allOwnerRentals = ownerRentals .filter((r) => - ["pending", "confirmed", "active", "cancelled"].includes(r.status) + ["pending", "confirmed", "active"].includes(r.status) ) .sort((a, b) => { const statusOrder = { pending: 0, confirmed: 1, active: 2 }; @@ -502,30 +459,6 @@ const MyListings: React.FC = () => { )}
- {/* Condition Check Status */} - {getCompletedChecksForRental(rental.id).length > 0 && ( -
- {getCompletedChecksForRental(rental.id).map( - (check) => ( -
- - {check.checkType === "pre_rental_owner" - ? "Pre-Rental Check Completed" - : "Post-Rental Check Completed"} - - {new Date( - check.createdAt - ).toLocaleDateString()} - -
- ) - )} -
- )} - {/* Condition Check Buttons */} {getAvailableChecksForRental(rental.id).map((check) => (