Skip to content

Conversation

@mubashirzamir
Copy link

@mubashirzamir mubashirzamir commented Oct 16, 2025

Summary by CodeRabbit

  • New Features

    • Player profiles now include an optional image field; the app fetches and surfaces player images when available, falling back gracefully when absent.
  • Tests

    • Test suite updated to validate the new optional player image field and ensure compatibility with existing player data structures.

@coderabbitai
Copy link

coderabbitai bot commented Oct 16, 2025

Walkthrough

Adds optional image field to player schema, adds XPath selectors for player images, updates the player service to fetch and include image data, and adjusts tests to validate the new optional image field.

Changes

Cohort / File(s) Summary
Player schema & tests
app/schemas/clubs/players.py, tests/clubs/test_clubs_players.py
Added optional image: Optional[str] = None to ClubPlayer and updated test expected schema to accept None or a non-empty string for image.
XPath selectors
app/utils/xpath.py
Added IMAGE XPath constant (".//img[contains(@class, 'bilderrahmen-fixed')]//@data-src") to Players and Clubs.Players namespaces for extracting image data-src.
Service layer
app/services/clubs/players.py
Updated player retrieval to fetch images via Clubs.Players.IMAGE, defaulting to None entries when absent, and added image to each returned player dict.

Sequence Diagram

sequenceDiagram
    participant Client
    participant Service as Player Service
    participant XPath as XPath Utils
    participant Response

    Client->>Service: Request players
    Service->>XPath: Query Clubs.Players.IMAGE
    alt images found
        XPath-->>Service: list of image URLs
    else no images
        XPath-->>Service: fallback list of None
    end
    Service->>Service: zip player fields with image values
    Service-->>Response: players (each includes image field)
    Response-->>Client: Return players
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐇 I hopped through code to find a face,
A tiny URL now takes its place.
Images fetched from HTML streams,
Players gleam inside my dreams. 📸

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The pull request title "feat: add images to club players" clearly and accurately summarizes the main changes across all modified files. The changeset adds an image field to the ClubPlayer schema, implements image retrieval logic in the service layer, introduces XPath selectors to extract player images, and updates tests to validate the new field. The title is concise (6 words), specific, and uses clear language without vague terminology, emojis, or file lists. A teammate scanning the commit history would immediately understand that this PR adds image support for club players.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 696a143 and d7b9952.

📒 Files selected for processing (1)
  • app/services/clubs/players.py (3 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • app/services/clubs/players.py

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between bee4c49 and 696a143.

📒 Files selected for processing (4)
  • app/schemas/clubs/players.py (1 hunks)
  • app/services/clubs/players.py (3 hunks)
  • app/utils/xpath.py (1 hunks)
  • tests/clubs/test_clubs_players.py (1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
tests/clubs/test_clubs_players.py (1)
tests/conftest.py (1)
  • len_greater_than_0 (6-7)
app/services/clubs/players.py (2)
app/services/base.py (1)
  • get_list_by_xpath (132-150)
app/utils/xpath.py (4)
  • Clubs (105-178)
  • Clubs (197-199)
  • Players (1-102)
  • Players (148-178)
🔇 Additional comments (4)
app/utils/xpath.py (1)

156-156: LGTM! XPath selector is correct for lazy-loaded images.

The XPath selector correctly targets the data-src attribute of img elements with the class 'bilderrahmen-fixed', which is the standard pattern for lazy-loaded images in the Transfermarkt DOM structure.

app/schemas/clubs/players.py (1)

11-11: LGTM! Schema definition is correct.

The optional image field is properly typed and positioned in the model.

tests/clubs/test_clubs_players.py (1)

39-39: LGTM! Test validation is correct.

The test schema correctly validates that the image field can be either None or a non-empty string, which aligns with the Optional[str] schema definition.

app/services/clubs/players.py (1)

91-91: LGTM! Image field correctly integrated into player dictionaries.

The image field is properly added to each player dictionary and correctly positioned in the zip function to align with other fields. Once the fallback value at line 59 is corrected, this implementation will be complete.

Also applies to: 105-109

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant