From 49280d9679e215c4a94d7b7f7b49eb43e86633df Mon Sep 17 00:00:00 2001 From: Viktor Barzin Date: Sat, 21 Feb 2026 14:04:34 +0000 Subject: [PATCH] fix: QA fixes for property decisions feature - Replace deprecated datetime.utcnow() with datetime.now(UTC) in model and repository - Add listing_type validation to decision_service (RENT/BUY only) - Fix decision filtering tests failing due to rate limiting by patching _match_endpoint - Add SwipeCard component test suite (11 tests covering rendering, interactions, and POI distances) - Add test for invalid listing_type validation --- .../components/__tests__/SwipeCard.test.tsx | 127 ++++++++++++++++++ models/decision.py | 6 +- repositories/decision_repository.py | 4 +- services/decision_service.py | 5 + tests/unit/test_decision_filtering.py | 6 +- tests/unit/test_decision_service.py | 8 ++ 6 files changed, 149 insertions(+), 7 deletions(-) create mode 100644 frontend/src/components/__tests__/SwipeCard.test.tsx diff --git a/frontend/src/components/__tests__/SwipeCard.test.tsx b/frontend/src/components/__tests__/SwipeCard.test.tsx new file mode 100644 index 0000000..2258f55 --- /dev/null +++ b/frontend/src/components/__tests__/SwipeCard.test.tsx @@ -0,0 +1,127 @@ +import { render, screen } from '@testing-library/react'; +import userEvent from '@testing-library/user-event'; +import { SwipeCard } from '@/components/SwipeCard'; +import { createMockFeature } from '@/__tests__/helpers'; + +// Mock react-spring to render plain divs instead of animated ones +vi.mock('@react-spring/web', () => ({ + animated: { + div: ({ children, style: _style, ...rest }: React.HTMLAttributes) => ( +
{children}
+ ), + }, + useSpring: () => [ + { + x: { to: (_fn: (v: number) => number) => 0 }, + y: { to: (_fn: (v: number) => number) => 0 }, + rotate: 0, + scale: 1, + opacity: 1, + }, + { start: vi.fn() }, + ], +})); + +// Mock use-gesture to return a no-op bind function +vi.mock('@use-gesture/react', () => ({ + useDrag: () => () => ({}), +})); + +const defaultFeature = createMockFeature({ + url: 'https://www.rightmove.co.uk/properties/12345', + total_price: 2500, + rooms: 2, + qm: 65, + qmprice: 38, + photo_thumbnail: 'https://example.com/photo.jpg', + listing_type: 'RENT', +}); + +describe('SwipeCard', () => { + const defaultProps = { + feature: defaultFeature, + onSwipe: vi.fn(), + isTop: true, + stackIndex: 0, + }; + + it('renders property price', () => { + render(); + expect(screen.getByText(/2,500/)).toBeInTheDocument(); + }); + + it('renders /mo suffix for rent listings', () => { + render(); + expect(screen.getByText('/mo')).toBeInTheDocument(); + }); + + it('does not render /mo for buy listings', () => { + const buyFeature = createMockFeature({ + listing_type: 'BUY', + total_price: 500000, + }); + render(); + expect(screen.queryByText('/mo')).not.toBeInTheDocument(); + }); + + it('renders bedroom count', () => { + render(); + expect(screen.getByText(/2 bed/)).toBeInTheDocument(); + }); + + it('renders square meters', () => { + render(); + expect(screen.getByText(/65 m²/)).toBeInTheDocument(); + }); + + it('renders price per sqm', () => { + render(); + expect(screen.getByText(/£38\/m²/)).toBeInTheDocument(); + }); + + it('renders photo thumbnail', () => { + render(); + const img = screen.getByRole('img'); + expect(img).toHaveAttribute('src', 'https://example.com/photo.jpg'); + }); + + it('renders external link button', () => { + render(); + // The button contains an ExternalLink icon + const buttons = screen.getAllByRole('button'); + expect(buttons.length).toBeGreaterThan(0); + }); + + it('opens listing URL when external link clicked', async () => { + const user = userEvent.setup(); + const openSpy = vi.spyOn(window, 'open').mockImplementation(() => null); + + render(); + const button = screen.getByRole('button'); + await user.click(button); + + expect(openSpy).toHaveBeenCalledWith( + 'https://www.rightmove.co.uk/properties/12345', + '_blank', + 'noopener,noreferrer', + ); + openSpy.mockRestore(); + }); + + it('renders POI distances when present', () => { + const featureWithPOIs = createMockFeature({ + poi_distances: [ + { poi_id: 1, poi_name: 'Office', travel_mode: 'TRANSIT', duration_seconds: 1800, distance_meters: 5000 }, + ], + }); + render(); + expect(screen.getByText(/Office: 30m/)).toBeInTheDocument(); + }); + + it('does not show POI section when no distances', () => { + const featureNoPOIs = createMockFeature({ poi_distances: undefined }); + const { container } = render(); + // No POI badges should be rendered + expect(container.querySelectorAll('.bg-muted.px-2').length).toBe(0); + }); +}); diff --git a/models/decision.py b/models/decision.py index 116ca00..6c76a11 100644 --- a/models/decision.py +++ b/models/decision.py @@ -1,4 +1,4 @@ -from datetime import datetime +from datetime import datetime, UTC from sqlmodel import SQLModel, Field @@ -9,5 +9,5 @@ class ListingDecision(SQLModel, table=True): listing_id: int = Field(nullable=False, index=True) listing_type: str = Field(nullable=False) # "RENT" or "BUY" decision: str = Field(nullable=False) # "liked" or "disliked" - created_at: datetime = Field(default_factory=datetime.utcnow) - updated_at: datetime = Field(default_factory=datetime.utcnow) + created_at: datetime = Field(default_factory=lambda: datetime.now(UTC)) + updated_at: datetime = Field(default_factory=lambda: datetime.now(UTC)) diff --git a/repositories/decision_repository.py b/repositories/decision_repository.py index fbed9df..45fe72a 100644 --- a/repositories/decision_repository.py +++ b/repositories/decision_repository.py @@ -1,5 +1,5 @@ """Repository for listing decisions (like/dislike).""" -from datetime import datetime +from datetime import datetime, UTC from models.decision import ListingDecision from sqlalchemy import Engine @@ -24,7 +24,7 @@ class DecisionRepository: existing = session.exec(statement).first() if existing: existing.decision = decision - existing.updated_at = datetime.utcnow() + existing.updated_at = datetime.now(UTC) session.add(existing) session.commit() session.refresh(existing) diff --git a/services/decision_service.py b/services/decision_service.py index 4faae8a..ad11556 100644 --- a/services/decision_service.py +++ b/services/decision_service.py @@ -3,6 +3,7 @@ from models.decision import ListingDecision from repositories.decision_repository import DecisionRepository VALID_DECISIONS = {"liked", "disliked"} +VALID_LISTING_TYPES = {"RENT", "BUY"} def set_decision( @@ -16,6 +17,10 @@ def set_decision( raise ValueError( f"Invalid decision '{decision}'. Must be one of: {VALID_DECISIONS}" ) + if listing_type not in VALID_LISTING_TYPES: + raise ValueError( + f"Invalid listing_type '{listing_type}'. Must be one of: {VALID_LISTING_TYPES}" + ) return repository.upsert_decision( user_id=user_id, listing_id=listing_id, diff --git a/tests/unit/test_decision_filtering.py b/tests/unit/test_decision_filtering.py index 7953071..9faa1ac 100644 --- a/tests/unit/test_decision_filtering.py +++ b/tests/unit/test_decision_filtering.py @@ -1,6 +1,7 @@ """Test that disliked listings are filtered from the GeoJSON endpoint.""" import pytest from datetime import datetime +from unittest.mock import patch from httpx import ASGITransport, AsyncClient from sqlalchemy import Engine from sqlmodel import SQLModel, Session, create_engine @@ -73,8 +74,9 @@ async def filter_client(filter_engine: Engine) -> AsyncClient: poi_routes_mod.engine = filter_engine transport = ASGITransport(app=app) - async with AsyncClient(transport=transport, base_url="http://test") as c: - yield c # type: ignore[misc] + with patch("api.rate_limiter._match_endpoint", return_value=None): + async with AsyncClient(transport=transport, base_url="http://test") as c: + yield c # type: ignore[misc] database.engine = original_db api_app.engine = original_app diff --git a/tests/unit/test_decision_service.py b/tests/unit/test_decision_service.py index c7a48a5..73e16cd 100644 --- a/tests/unit/test_decision_service.py +++ b/tests/unit/test_decision_service.py @@ -40,6 +40,14 @@ class TestSetDecision: listing_type="RENT", decision="maybe", ) + def test_invalid_listing_type_raises(self) -> None: + repo = MagicMock() + with pytest.raises(ValueError, match="Invalid listing_type"): + decision_service.set_decision( + repo, user_id=1, listing_id=100, + listing_type="SELL", decision="liked", + ) + class TestGetDecisions: def test_returns_all_decisions(self) -> None: