Skip to content

Conversation

@eliorivero
Copy link
Contributor

@eliorivero eliorivero commented Jul 2, 2019

This fixes #12913 by checking not only that the status is correct, but also if the response has results before attempting to use their geometric data.

May be superseeded by #12474

Changes proposed in this Pull Request:

  • check that the array of results is not empty

Testing instructions:

  • Go to /wp-admin/widgets.php
  • Add a Contact info widget to the footer
  • ensure there are no notices in wp-content/debug.log
  • alternatively check the JS console. This was also causing errors visible in the ajax response.

Proposed changelog entry for your changes:

  • Not needed.

@eliorivero eliorivero requested a review from a team July 2, 2019 17:27
@eliorivero eliorivero self-assigned this Jul 2, 2019
@eliorivero eliorivero added [Status] Needs Review This PR is ready for review. [Type] Bug When a feature is broken and / or not performing as intended [Feature] Extra Sidebar Widgets labels Jul 2, 2019
@eliorivero eliorivero added this to the 7.5.1 milestone Jul 2, 2019
@eliorivero
Copy link
Contributor Author

eliorivero commented Jul 2, 2019

While this PR solves this, the error is due to Google blocking the request. The complete error response is:

[02-Jul-2019 15:49:25 UTC] stdClass Object
(
    [error_message] => You must use an API key to authenticate each request to Google Maps Platform APIs. For additional information, please refer to http://g.co/dev/maps-no-account
    [results] => Array
        (
        )

    [status] => REQUEST_DENIED
)

Perhaps we should consider letting the user know what is going on? There's message in the widget saying that it's not working normally as it should.

cc @MichaelArestad

@jetpackbot
Copy link
Collaborator

Thank you for the great PR description!

When this PR is ready for review, please apply the [Status] Needs Review label. If you are an a11n, please have someone from your team review the code if possible. The Jetpack team will also review this PR and merge it to be included in the next Jetpack release.

Scheduled Jetpack release: August 6, 2019.
Scheduled code freeze: July 30, 2019

Generated by 🚫 dangerJS against 398d3ac

@tyxla tyxla modified the milestones: 7.5.1 , 7.6 Jul 3, 2019
@jeherve
Copy link
Member

jeherve commented Jul 3, 2019

Perhaps we should consider letting the user know what is going on? There's message in the widget saying that it's not working normally as it should.

#12474 should improve the user experience in that regard.

@jeherve jeherve added [Status] Ready to Merge Go ahead, you can push that green button! and removed [Status] Needs Review This PR is ready for review. labels Jul 3, 2019
Copy link
Member

@jeherve jeherve left a comment

Choose a reason for hiding this comment

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

This looks good to me. I would hold on that PR though, and see if we can focus on #12474 instead, as it will also solve the issue by getting rid of that part of the code altogether.

@jeherve jeherve added [Status] Blocked / Hold and removed [Status] Ready to Merge Go ahead, you can push that green button! labels Jul 3, 2019
@jeherve jeherve removed this from the 7.6 milestone Jul 29, 2019
@jeherve jeherve added this to the 7.7 milestone Aug 7, 2019
@jeherve jeherve assigned jeherve and kbrown9 and unassigned eliorivero Aug 21, 2019
@kraftbj kraftbj deleted the fix/contact-info-map-notice branch April 17, 2025 13:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Contact Info Widget: PHP Notice on adding to footer

7 participants