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.

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

fix: fixed issue with missing chapter content on Juno hero page #195

Merged
merged 2 commits into from
Sep 15, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 1 addition & 1 deletion app/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,7 @@ class Settings(BaseSettings):
home_path: str = "/"

# Route for Overwatch heroes pages
heroes_path: str = "/heroes"
heroes_path: str = "/heroes/"

# Route for players career pages
career_path: str = "/career"
Expand Down
6 changes: 3 additions & 3 deletions app/parsers/hero_parser.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ def __init__(self, **kwargs):
self.locale = kwargs.get("locale") or Locale.ENGLISH_US

def get_blizzard_url(self, **kwargs) -> str:
return f"{super().get_blizzard_url(**kwargs)}/{kwargs.get('hero_key')}"
return f"{super().get_blizzard_url(**kwargs)}{kwargs.get('hero_key')}/"

def parse_data(self) -> dict:
# We must check if we have the expected section for hero. If not,
Expand Down Expand Up @@ -66,7 +66,7 @@ def __get_summary(self, overview_section: Tag) -> dict:
header_section.find("p", slot="description").get_text().strip()
),
"role": get_role_from_icon_url(extra_list_items[0].find("image")["href"]),
"location": extra_list_items[1].get_text(),
"location": extra_list_items[1].get_text().strip(),
"birthday": birthday,
"age": age,
}
Expand Down Expand Up @@ -163,7 +163,7 @@ def __get_story_chapters(accordion: Tag) -> list[dict]:
" ".join(
[
paragraph.get_text()
for paragraph in content_container.find_all("p")
for paragraph in content_container.find_all(["p", "pr"])
],
).strip()
)
Expand Down
2 changes: 1 addition & 1 deletion pyproject.toml
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
[project]
name = "overfast-api"
version = "2.37.0"
version = "2.37.1"
description = "Overwatch API giving data about heroes, maps, and players statistics."
license = {file = "LICENSE"}
authors = [
Expand Down
16 changes: 8 additions & 8 deletions tests/commands/test_check_and_update_cache.py
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ def test_check_and_update_gamemodes_cache_to_update(
settings.expired_cache_refresh_limit + 30,
)
cache_manager.update_parser_cache(
f"HeroParser-{settings.blizzard_host}/{locale}{settings.heroes_path}/ana",
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): Update test cases to reflect changes in URL structure

The changes in the URL structure (removing a slash before 'ana' and adding one after) are consistently applied across multiple test cases. This is good for maintaining consistency, but we should ensure that these changes align with the actual behavior of the updated code.

Suggested change
f"HeroParser-{settings.blizzard_host}/{locale}{settings.heroes_path}/ana",
f"HeroParser-{settings.blizzard_host}/{locale}{settings.heroes_path}/ana",

f"HeroParser-{settings.blizzard_host}/{locale}{settings.heroes_path}ana/",
{},
settings.expired_cache_refresh_limit + 5,
)
Expand Down Expand Up @@ -89,7 +89,7 @@ def test_check_and_update_specific_hero_to_update(
hero_json_data: dict,
):
ana_cache_key = (
f"HeroParser-{settings.blizzard_host}/{locale}{settings.heroes_path}/ana"
f"HeroParser-{settings.blizzard_host}/{locale}{settings.heroes_path}ana/"
)

# Add some data (to update and not to update)
Expand Down Expand Up @@ -163,7 +163,7 @@ def test_check_and_update_cache_no_update(cache_manager: CacheManager, locale: s
settings.expired_cache_refresh_limit + 30,
)
cache_manager.update_parser_cache(
f"HeroParser-{settings.blizzard_host}/{locale}{settings.heroes_path}/ana",
f"HeroParser-{settings.blizzard_host}/{locale}{settings.heroes_path}ana/",
{},
settings.expired_cache_refresh_limit + 5,
)
Expand Down Expand Up @@ -202,7 +202,7 @@ def test_check_and_update_specific_player_to_update(
settings.expired_cache_refresh_limit - 5,
)
cache_manager.update_parser_cache(
f"HeroParser-{settings.blizzard_host}/{locale}{settings.heroes_path}/ana",
f"HeroParser-{settings.blizzard_host}/{locale}{settings.heroes_path}ana/",
{},
settings.expired_cache_refresh_limit + 5,
)
Expand Down Expand Up @@ -254,7 +254,7 @@ def test_check_and_update_player_stats_summary_to_update(
settings.expired_cache_refresh_limit - 5,
)
cache_manager.update_parser_cache(
f"HeroParser-{settings.blizzard_host}/{locale}{settings.heroes_path}/ana",
f"HeroParser-{settings.blizzard_host}/{locale}{settings.heroes_path}ana/",
{},
settings.expired_cache_refresh_limit + 5,
)
Expand Down Expand Up @@ -297,7 +297,7 @@ def test_check_and_update_player_stats_summary_to_update(
def test_check_internal_error_from_blizzard(cache_manager: CacheManager, locale: str):
# Add some data (to update and not to update)
cache_manager.update_parser_cache(
f"HeroParser-{settings.blizzard_host}/{locale}{settings.heroes_path}/ana",
f"HeroParser-{settings.blizzard_host}/{locale}{settings.heroes_path}ana/",
{},
settings.expired_cache_refresh_limit - 5,
)
Expand Down Expand Up @@ -325,7 +325,7 @@ def test_check_internal_error_from_blizzard(cache_manager: CacheManager, locale:
def test_check_timeout_from_blizzard(cache_manager: CacheManager, locale: str):
# Add some data (to update and not to update)
cache_manager.update_parser_cache(
f"HeroParser-{settings.blizzard_host}/{locale}{settings.heroes_path}/ana",
f"HeroParser-{settings.blizzard_host}/{locale}{settings.heroes_path}ana/",
{},
settings.expired_cache_refresh_limit - 5,
)
Expand Down Expand Up @@ -475,7 +475,7 @@ def test_check_and_update_namecard_to_update(

# Add some data which doesn't need update
cache_manager.update_parser_cache(
f"HeroParser-{settings.blizzard_host}/{locale}{settings.heroes_path}/ana",
f"HeroParser-{settings.blizzard_host}/{locale}{settings.heroes_path}ana/",
{},
settings.expired_cache_refresh_limit + 5,
)
Expand Down
2 changes: 1 addition & 1 deletion tests/common/test_cache_manager.py
Original file line number Diff line number Diff line change
Expand Up @@ -168,7 +168,7 @@ def test_get_soon_expired_cache_keys(
is_redis_server_up,
):
cache_manager.update_parser_cache(
f"HeroParser-{settings.blizzard_host}/{locale}{settings.heroes_path}/ana",
f"HeroParser-{settings.blizzard_host}/{locale}{settings.heroes_path}ana/",
{},
settings.expired_cache_refresh_limit + 5,
)
Expand Down