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

[DISCO-3246] fix: Weather to save normalized city in selected_city field #796

Merged
merged 1 commit into from
Feb 19, 2025

Conversation

tiftran
Copy link
Contributor

@tiftran tiftran commented Feb 19, 2025

References

JIRA: DISCO-3246

Description

We were saving the manipulated/normalized city name in geolocation.city so it changes when we try to fetch from cache, when we try to call the api, we're left with the manipulated/normalized city name with no reference to the original

PR Review Checklist

Put an x in the boxes that apply

  • This PR conforms to the Contribution Guidelines
  • The PR title starts with the JIRA issue reference, format example [DISCO-####], and has the same title (if applicable)
  • [load test: (abort|skip|warn)] keywords are applied to the last commit message (if applicable)
  • Documentation has been updated (if applicable)
  • Functional and performance test coverage has been expanded and maintained (if applicable)

@tiftran tiftran requested a review from a team February 19, 2025 06:11
@@ -546,7 +546,7 @@ async def _fetch_from_cache(
"""Fetch weather data from cache."""
geolocation = weather_context.geolocation
country = geolocation.country
city = geolocation.city
city = weather_context.selected_city
Copy link
Contributor Author

Choose a reason for hiding this comment

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

🙈

@@ -11,7 +11,12 @@
MaybeStr = Optional[str]

LOCALITY_SUFFIX_PATTERN: re.Pattern = re.compile(r"\s+(city|municipality)$", re.IGNORECASE)
SUCCESSFUL_REGIONS_MAPPING: dict[tuple[str, str], str | None] = {("GB", "London"): "LND"}
SUCCESSFUL_REGIONS_MAPPING: dict[tuple[str, str], str | None] = {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Adding the following cities since they require not to have regions and there's a bug with that at the moment

@tiftran tiftran added this pull request to the merge queue Feb 19, 2025
Merged via the queue into main with commit 18d6b5d Feb 19, 2025
11 checks passed
@tiftran tiftran deleted the bug-selected-city branch February 19, 2025 18:34
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.

2 participants