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

Replaces non-existent route by the correct one in geolocation controller #29590

Merged
merged 1 commit into from Sep 12, 2022
Merged

Replaces non-existent route by the correct one in geolocation controller #29590

merged 1 commit into from Sep 12, 2022

Conversation

nicosomb
Copy link
Contributor

@nicosomb nicosomb commented Sep 9, 2022

Questions Answers
Branch? develop
Description? A non existent route was used. I replaced it with the correct one.
Type? bug fix
Category? BO
BC breaks? no
Deprecations? no
Fixed ticket? Fixes #29119
Related PRs
How to test? See #29119
Possible impacts?

@nicosomb nicosomb requested a review from a team as a code owner September 9, 2022 09:37
@prestonBot
Copy link
Collaborator

Hello @nicosomb!

This is your first pull request on the PrestaShop project. Thank you, and welcome to this Open Source community!

@prestonBot prestonBot added develop Branch Bug fix Type: Bug fix labels Sep 9, 2022
@matks matks added the Waiting for QA Status: action required, waiting for test feedback label Sep 9, 2022
@PrestaEdit
Copy link
Contributor

Not going into 8.0.x ? 🤔

@MhiriFaten MhiriFaten self-assigned this Sep 9, 2022
@nicosomb
Copy link
Contributor Author

nicosomb commented Sep 9, 2022

Not going into 8.0.x ? 🤔

@PrestaEdit as the issue affects 1.7.7.8, 1.7.8.x & 8.0.x, we told me to target develop.

MhiriFaten
MhiriFaten approved these changes Sep 12, 2022
View reviewed changes
Copy link

@MhiriFaten MhiriFaten left a comment

Choose a reason for hiding this comment

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

Hello @nicosomb ,

Thank you for your PR, I have checked it. The issue is well fixed.
Here is the checked list :

Check the automated tests : OK
Check with different languages (RTL and no RTL) : OK
Check Check with different browsers (Chrome & Firefox) : OK
Check with multistore: OK

Geolocation.prestashop.mp4

Thank you 😄

@MhiriFaten MhiriFaten added QA ✔️ Status: check done, code approved and removed Waiting for QA Status: action required, waiting for test feedback labels Sep 12, 2022
@prestonBot
Copy link
Collaborator

QA approved, well done! Message to the maintainers: do not forget to milestone it before the merge.

@matthieu-rolland
Copy link
Contributor

thank you @nicosomb and @MhiriFaten !

@matthieu-rolland matthieu-rolland merged commit 1788f2b into PrestaShop:develop Sep 12, 2022
@nicosomb nicosomb deleted the fix-29119 branch September 12, 2022 14:58
@PrestaEdit
Copy link
Contributor

I really don't understand why this is not added to the 8.0.x, at least.

@matks matks added this to the 8.1.0 milestone Oct 11, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@FabienPapet FabienPapet FabienPapet approved these changes

@kpodemski kpodemski kpodemski approved these changes

@matks matks matks approved these changes

@MhiriFaten MhiriFaten MhiriFaten approved these changes

Assignees

@MhiriFaten MhiriFaten

Labels
Bug fix Type: Bug fix develop Branch QA ✔️ Status: check done, code approved
Projects
None yet
Milestone
8.1.0
Development

Successfully merging this pull request may close these issues.

BO - Geolocation by IP address throws exception for read-only employee
8 participants
@nicosomb @prestonBot @PrestaEdit @matthieu-rolland @FabienPapet @kpodemski @matks @MhiriFaten