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
This commit is contained in:
parent
8452f65d25
commit
49280d9679
6 changed files with 149 additions and 7 deletions
127
frontend/src/components/__tests__/SwipeCard.test.tsx
Normal file
127
frontend/src/components/__tests__/SwipeCard.test.tsx
Normal file
|
|
@ -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<HTMLDivElement>) => (
|
||||
<div {...rest}>{children}</div>
|
||||
),
|
||||
},
|
||||
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(<SwipeCard {...defaultProps} />);
|
||||
expect(screen.getByText(/2,500/)).toBeInTheDocument();
|
||||
});
|
||||
|
||||
it('renders /mo suffix for rent listings', () => {
|
||||
render(<SwipeCard {...defaultProps} />);
|
||||
expect(screen.getByText('/mo')).toBeInTheDocument();
|
||||
});
|
||||
|
||||
it('does not render /mo for buy listings', () => {
|
||||
const buyFeature = createMockFeature({
|
||||
listing_type: 'BUY',
|
||||
total_price: 500000,
|
||||
});
|
||||
render(<SwipeCard {...defaultProps} feature={buyFeature} />);
|
||||
expect(screen.queryByText('/mo')).not.toBeInTheDocument();
|
||||
});
|
||||
|
||||
it('renders bedroom count', () => {
|
||||
render(<SwipeCard {...defaultProps} />);
|
||||
expect(screen.getByText(/2 bed/)).toBeInTheDocument();
|
||||
});
|
||||
|
||||
it('renders square meters', () => {
|
||||
render(<SwipeCard {...defaultProps} />);
|
||||
expect(screen.getByText(/65 m²/)).toBeInTheDocument();
|
||||
});
|
||||
|
||||
it('renders price per sqm', () => {
|
||||
render(<SwipeCard {...defaultProps} />);
|
||||
expect(screen.getByText(/£38\/m²/)).toBeInTheDocument();
|
||||
});
|
||||
|
||||
it('renders photo thumbnail', () => {
|
||||
render(<SwipeCard {...defaultProps} />);
|
||||
const img = screen.getByRole('img');
|
||||
expect(img).toHaveAttribute('src', 'https://example.com/photo.jpg');
|
||||
});
|
||||
|
||||
it('renders external link button', () => {
|
||||
render(<SwipeCard {...defaultProps} />);
|
||||
// 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(<SwipeCard {...defaultProps} />);
|
||||
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(<SwipeCard {...defaultProps} feature={featureWithPOIs} />);
|
||||
expect(screen.getByText(/Office: 30m/)).toBeInTheDocument();
|
||||
});
|
||||
|
||||
it('does not show POI section when no distances', () => {
|
||||
const featureNoPOIs = createMockFeature({ poi_distances: undefined });
|
||||
const { container } = render(<SwipeCard {...defaultProps} feature={featureNoPOIs} />);
|
||||
// No POI badges should be rendered
|
||||
expect(container.querySelectorAll('.bg-muted.px-2').length).toBe(0);
|
||||
});
|
||||
});
|
||||
|
|
@ -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))
|
||||
|
|
|
|||
|
|
@ -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)
|
||||
|
|
|
|||
|
|
@ -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,
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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:
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue