Skip to content
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.

Sign up for GitHub

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

Jump to bottom

feat: added last_updated_at in players search results and player summary #184

Merged
merged 1 commit into from
Aug 25, 2024
Merged

feat: added last_updated_at in players search results and player summary #184

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion app/commands/update_search_data_cache.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
cache_manager = CacheManager()

# Mapping between the search data type and the variable name in JS
variable_name_mapping: dict[SearchDataType, str] = {
variable_name_mapping: dict[SearchDataType, str | None] = {
SearchDataType.PORTRAIT: "avatars",
SearchDataType.NAMECARD: "namecards",
SearchDataType.TITLE: "titles",
Expand All @@ -37,6 +37,9 @@ def get_search_page() -> httpx.Response:

def extract_search_data(html_content: str, data_type: SearchDataType) -> dict:
variable_name = variable_name_mapping[data_type]
if not variable_name:
return None

data_regexp = rf"const {variable_name} = (\{{.*\}})\n"

result = re.search(data_regexp, html_content)
Expand Down Expand Up @@ -99,6 +102,7 @@ def update_search_data_cache():
search_data = {
data_type: retrieve_search_data(data_type, search_page)
for data_type in SearchDataType
if data_type != SearchDataType.LAST_UPDATED_AT
}
except SearchDataRetrievalError as error:
raise SystemExit from error
Expand Down
1 change: 1 addition & 0 deletions app/common/enums.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -162,3 +162,4 @@ class SearchDataType(StrEnum):
NAMECARD = "namecard"
PORTRAIT = "portrait"
TITLE = "title"
LAST_UPDATED_AT = "lastUpdatedAt"
14 changes: 10 additions & 4 deletions app/handlers/get_player_career_request_handler.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,10 @@

from typing import ClassVar

from app.common.enums import SearchDataType
from app.config import settings
from app.parsers.player_parser import PlayerParser
from app.parsers.search_data_parser import NamecardParser
from app.parsers.search_data_parser import LastUpdatedAtParser, NamecardParser

from .api_request_handler import APIRequestHandler

Expand All @@ -15,12 +16,13 @@ class GetPlayerCareerRequestHandler(APIRequestHandler):
PlayerParser class.
"""

parser_classes: ClassVar[list] = [PlayerParser, NamecardParser]
parser_classes: ClassVar[list] = [PlayerParser, NamecardParser, LastUpdatedAtParser]
timeout = settings.career_path_cache_timeout

def merge_parsers_data(self, parsers_data: list[dict], **kwargs) -> dict:
"""Merge parsers data together : PlayerParser for statistics data,
and NamecardParser for namecard (not here in career page)
NamecardParser for namecard and LastUpdatedAtParser
for last time the player profile was updated
"""

# If the user asked for stats, no need to add the namecard
Expand All @@ -31,14 +33,18 @@ def merge_parsers_data(self, parsers_data: list[dict], **kwargs) -> dict:
summary = (
parsers_data[0] if kwargs.get("summary") else parsers_data[0]["summary"]
)
namecard_value = parsers_data[1]["namecard"]
namecard_value = parsers_data[1][SearchDataType.NAMECARD]
last_updated_at_value = parsers_data[2][SearchDataType.LAST_UPDATED_AT]

# We want to insert the namecard before "title" key in "summary"
title_pos = list(summary.keys()).index("title")
summary_items = list(summary.items())
summary_items.insert(title_pos, ("namecard", namecard_value))
summary = dict(summary_items)

# We can insert the last_updated_at at the end
summary["last_updated_at"] = last_updated_at_value

if kwargs.get("summary"):
return summary

Expand Down
1 change: 1 addition & 0 deletions app/handlers/search_players_request_handler.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,7 @@ def apply_transformations(self, players: Iterable[dict]) -> list[dict]:
"title": self.get_title(player, player_id),
"career_url": f"{settings.app_base_url}/players/{player_id}",
"blizzard_id": player["url"],
"last_updated_at": player["lastUpdated"],
},
)
return transformed_players
Expand Down
18 changes: 18 additions & 0 deletions app/models/players.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,15 @@ class PlayerShort(BaseModel):
description="Blizzard unique identifier of the player (hexadecimal)",
examples=["c65b8798bc61d6ffbba120%7Ccfe9dd77a4382165e2b920bdcc035949"],
)
last_updated_at: int | None = Field(
None,
title="Timestamp",
description=(
"Last time the player profile was updated on Blizzard (timestamp). "
"Can be null if couldn't retrieve any"
),
examples=[1704209332],
)


class PlayerSearchResult(BaseModel):
Expand Down Expand Up @@ -227,6 +236,15 @@ class PlayerSummary(BaseModel):
"or if the player doesn't play competitive at all, it's null."
),
)
last_updated_at: int | None = Field(
None,
title="Timestamp",
description=(
"Last time the player profile was updated on Blizzard (timestamp). "
"Can be null if couldn't retrieve any"
),
examples=[1704209332],
)


class HeroesComparisons(BaseModel):
Expand Down
9 changes: 9 additions & 0 deletions app/parsers/search_data_parser.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -134,3 +134,12 @@ class TitleParser(SearchDataParser):
"""Title Parser class"""

data_type = SearchDataType.TITLE


class LastUpdatedAtParser(SearchDataParser):
"""LastUpdatedAt Parser class"""

data_type = SearchDataType.LAST_UPDATED_AT

def retrieve_data_value(self, player_data: dict) -> int:
Copy link
Contributor

Choose a reason for hiding this comment

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

suggestion: Add error handling for missing 'lastUpdated' key

Consider adding error handling in case the 'lastUpdated' key is missing from the player_data dictionary. You could use a try-except block or the dict.get() method with a default value to make this more robust and prevent potential KeyError exceptions.

Suggested change
def retrieve_data_value(self, player_data: dict) -> int:
def retrieve_data_value(self, player_data: dict) -> int:
return player_data.get("lastUpdated", 0)

return player_data["lastUpdated"]
2 changes: 1 addition & 1 deletion app/routers/players.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ async def search_players(
order_by: str = Query(
"name:asc",
title="Ordering field and the way it's arranged (asc[ending]/desc[ending])",
pattern=r"^(player_id|name):(asc|desc)$",
pattern=r"^(player_id|name|last_updated_at):(asc|desc)$",
),
offset: int = Query(0, title="Offset of the results", ge=0),
limit: int = Query(20, title="Limit of results per page", gt=0),
Expand Down
4 changes: 2 additions & 2 deletions pyproject.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
[project]
name = "overfast-api"
version = "2.34.1"
version = "2.35.0"
description = "Overwatch API giving data about heroes, maps, and players statistics."
license = {file = "LICENSE"}
authors = [
Expand Down Expand Up @@ -125,4 +125,4 @@ allowed-confusables = ["（", "）", "："]

[tool.ruff.lint.isort]
# Consider app as first-party for imports in tests
known-first-party = ["app"]
known-first-party = ["app"]
3 changes: 2 additions & 1 deletion tests/fixtures/json/players/Dekk-2677.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,8 @@
"season": 8
},
"console": null
}
},
"last_updated_at": null
},
"stats": {
"pc": {
Expand Down
3 changes: 2 additions & 1 deletion tests/fixtures/json/players/JohnV1-1190.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,8 @@
"open": null
},
"console": null
}
},
"last_updated_at": null
},
"stats": {
"pc": {
Expand Down
3 changes: 2 additions & 1 deletion tests/fixtures/json/players/KIRIKO-21253.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,8 @@
"open": null,
"season": 1
}
}
},
"last_updated_at": null
},
"stats": {
"pc": {
Expand Down
3 changes: 2 additions & 1 deletion tests/fixtures/json/players/Player-1112937.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,8 @@
"support": null,
"open": null
}
}
},
"last_updated_at": null
},
"stats": {
"pc": {
Expand Down
3 changes: 2 additions & 1 deletion tests/fixtures/json/players/TeKrop-2217.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,8 @@
"open": null
},
"console": null
}
},
"last_updated_at": 1678536999
},
"stats": {
"pc": {
Expand Down
3 changes: 2 additions & 1 deletion tests/fixtures/json/players/copypasting-1216.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,8 @@
"open": null
},
"console": null
}
},
"last_updated_at": null
},
"stats": {
"pc": {
Expand Down
3 changes: 2 additions & 1 deletion tests/fixtures/json/players/quibble-11594.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,8 @@
"open": null
},
"console": null
}
},
"last_updated_at": null
},
"stats": {
"pc": {
Expand Down
45 changes: 30 additions & 15 deletions tests/fixtures/json/search_players/search_players_api_result.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,8 @@
"namecard": null,
"title": null,
"career_url": "https://overfast-api.tekrop.fr/players/DEKK-11775",
"blizzard_id": "d67b87a1fe23caffbca9%7Ce071d6c2df64c91dc7ad8f0eb73d5d05"
"blizzard_id": "d67b87a1fe23caffbca9%7Ce071d6c2df64c91dc7ad8f0eb73d5d05",
"last_updated_at": 1665953911
},
{
"player_id": "DEKK-21259",
Expand All @@ -17,7 +18,8 @@
"namecard": null,
"title": null,
"career_url": "https://overfast-api.tekrop.fr/players/DEKK-21259",
"blizzard_id": "d67b87a1fe23caffbca9%7Ce071d6c2df64c91dc7ad8f0eb73d5d05"
"blizzard_id": "d67b87a1fe23caffbca9%7Ce071d6c2df64c91dc7ad8f0eb73d5d05",
"last_updated_at": 1676555200
},
{
"player_id": "Dekk-11904",
Expand All @@ -26,7 +28,8 @@
"namecard": null,
"title": null,
"career_url": "https://overfast-api.tekrop.fr/players/Dekk-11904",
"blizzard_id": "d67b87a1fe23caffbca9%7Ce071d6c2df64c91dc7ad8f0eb73d5d05"
"blizzard_id": "d67b87a1fe23caffbca9%7Ce071d6c2df64c91dc7ad8f0eb73d5d05",
"last_updated_at": 1666924000
},
{
"player_id": "Dekk-11906",
Expand All @@ -35,7 +38,8 @@
"namecard": null,
"title": null,
"career_url": "https://overfast-api.tekrop.fr/players/Dekk-11906",
"blizzard_id": "d67b87a1fe23caffbca9%7Ce071d6c2df64c91dc7ad8f0eb73d5d05"
"blizzard_id": "d67b87a1fe23caffbca9%7Ce071d6c2df64c91dc7ad8f0eb73d5d05",
"last_updated_at": 1667959199
},
{
"player_id": "Dekk-1380",
Expand All @@ -44,7 +48,8 @@
"namecard": null,
"title": null,
"career_url": "https://overfast-api.tekrop.fr/players/Dekk-1380",
"blizzard_id": "d67b87a1fe23caffbca9%7Ce071d6c2df64c91dc7ad8f0eb73d5d05"
"blizzard_id": "d67b87a1fe23caffbca9%7Ce071d6c2df64c91dc7ad8f0eb73d5d05",
"last_updated_at": 1666538443
},
{
"player_id": "Dekk-1637",
Expand All @@ -53,7 +58,8 @@
"namecard": null,
"title": null,
"career_url": "https://overfast-api.tekrop.fr/players/Dekk-1637",
"blizzard_id": "d67b87a1fe23caffbca9%7Ce071d6c2df64c91dc7ad8f0eb73d5d05"
"blizzard_id": "d67b87a1fe23caffbca9%7Ce071d6c2df64c91dc7ad8f0eb73d5d05",
"last_updated_at": 1670258731
},
{
"player_id": "Dekk-1766",
Expand All @@ -62,7 +68,8 @@
"namecard": null,
"title": null,
"career_url": "https://overfast-api.tekrop.fr/players/Dekk-1766",
"blizzard_id": "d67b87a1fe23caffbca9%7Ce071d6c2df64c91dc7ad8f0eb73d5d05"
"blizzard_id": "d67b87a1fe23caffbca9%7Ce071d6c2df64c91dc7ad8f0eb73d5d05",
"last_updated_at": 1665287787
},
{
"player_id": "Dekk-21129",
Expand All @@ -71,7 +78,8 @@
"namecard": null,
"title": null,
"career_url": "https://overfast-api.tekrop.fr/players/Dekk-21129",
"blizzard_id": "d67b87a1fe23caffbca9%7Ce071d6c2df64c91dc7ad8f0eb73d5d05"
"blizzard_id": "d67b87a1fe23caffbca9%7Ce071d6c2df64c91dc7ad8f0eb73d5d05",
"last_updated_at": 1668372404
},
{
"player_id": "Dekk-21260",
Expand All @@ -80,7 +88,8 @@
"namecard": null,
"title": null,
"career_url": "https://overfast-api.tekrop.fr/players/Dekk-21260",
"blizzard_id": "d67b87a1fe23caffbca9%7Ce071d6c2df64c91dc7ad8f0eb73d5d05"
"blizzard_id": "d67b87a1fe23caffbca9%7Ce071d6c2df64c91dc7ad8f0eb73d5d05",
"last_updated_at": 1668452370
},
{
"player_id": "Dekk-21386",
Expand All @@ -89,7 +98,8 @@
"namecard": null,
"title": null,
"career_url": "https://overfast-api.tekrop.fr/players/Dekk-21386",
"blizzard_id": "d67b87a1fe23caffbca9%7Ce071d6c2df64c91dc7ad8f0eb73d5d05"
"blizzard_id": "d67b87a1fe23caffbca9%7Ce071d6c2df64c91dc7ad8f0eb73d5d05",
"last_updated_at": 1677947164
},
{
"player_id": "Dekk-2162",
Expand All @@ -98,7 +108,8 @@
"namecard": null,
"title": null,
"career_url": "https://overfast-api.tekrop.fr/players/Dekk-2162",
"blizzard_id": "d67b87a1fe23caffbca9%7Ce071d6c2df64c91dc7ad8f0eb73d5d05"
"blizzard_id": "d67b87a1fe23caffbca9%7Ce071d6c2df64c91dc7ad8f0eb73d5d05",
"last_updated_at": 1671155709
},
{
"player_id": "Dekk-21771",
Expand All @@ -107,7 +118,8 @@
"namecard": null,
"title": null,
"career_url": "https://overfast-api.tekrop.fr/players/Dekk-21771",
"blizzard_id": "d67b87a1fe23caffbca9%7Ce071d6c2df64c91dc7ad8f0eb73d5d05"
"blizzard_id": "d67b87a1fe23caffbca9%7Ce071d6c2df64c91dc7ad8f0eb73d5d05",
"last_updated_at": 1666737903
},
{
"player_id": "Dekk-21904",
Expand All @@ -116,7 +128,8 @@
"namecard": null,
"title": null,
"career_url": "https://overfast-api.tekrop.fr/players/Dekk-21904",
"blizzard_id": "d67b87a1fe23caffbca9%7Ce071d6c2df64c91dc7ad8f0eb73d5d05"
"blizzard_id": "d67b87a1fe23caffbca9%7Ce071d6c2df64c91dc7ad8f0eb73d5d05",
"last_updated_at": 1676730323
},
{
"player_id": "Dekk-2677",
Expand All @@ -125,7 +138,8 @@
"namecard": "https://d15f34w2p8l1cc.cloudfront.net/overwatch/757219956129146d84617a7e713dfca1bc33ea27cf6c73df60a33d02a147edc1.png",
"title": "Bytefixer",
"career_url": "https://overfast-api.tekrop.fr/players/Dekk-2677",
"blizzard_id": "d65ba781fe23cdfabe%7C9b3063608098cdbf1b9825d8664f4d96"
"blizzard_id": "d65ba781fe23cdfabe%7C9b3063608098cdbf1b9825d8664f4d96",
"last_updated_at": 1678488893
},
{
"player_id": "dekk-2548",
Expand All @@ -134,7 +148,8 @@
"namecard": null,
"title": null,
"career_url": "https://overfast-api.tekrop.fr/players/dekk-2548",
"blizzard_id": "d67b87a1fe23caffbca9%7Ce071d6c2df64c91dc7ad8f0eb73d5d05"
"blizzard_id": "d67b87a1fe23caffbca9%7Ce071d6c2df64c91dc7ad8f0eb73d5d05",
"last_updated_at": 1676811985
}
]
}
4 changes: 3 additions & 1 deletion tests/parsers/test_player_parser.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,9 +25,11 @@ async def test_player_page_parsing_with_filters(
player_json_data: dict,
kwargs_filter: dict,
):
# Remove "namecard" key from player_json_data, it's been added from another page
# Remove "namecard" and "last_updated_at" keys from player_json_data,
# it's been added from others parsers
player_data = player_json_data.copy()
del player_data["summary"]["namecard"]
del player_data["summary"]["last_updated_at"]
Copy link
Contributor

Choose a reason for hiding this comment

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

suggestion (testing): Add test for LastUpdatedAtParser

While the test has been updated to remove the last_updated_at field from the player data, it would be beneficial to add a separate test case for the LastUpdatedAtParser to ensure it correctly parses and returns the last_updated_at value.

@pytest.mark.asyncio
async def test_last_updated_at_parser():
    mock_data = {"summary": {"last_updated_at": "2023-05-01T12:00:00Z"}}
    parser = LastUpdatedAtParser()
    result = await parser.parse(mock_data)
    assert isinstance(result, datetime)
    assert result.isoformat() == "2023-05-01T12:00:00+00:00"


parser = PlayerParser(player_id=player_id)
update_parser_cache_last_update_mock = Mock()
Expand Down
Loading