Problem/Motivation

Today I found the option called: Show a status message warning in case of geo-coding failure. in the field settings page of the geocoder field, and in first instance looks great because I don't want to bother my users if such error happens.

But that options becomes meaninless because even if that checkbox is unchecked the geocoder service throws some errors anyway when something went wrong:

public static function log($message, $type) {
    \Drupal::logger('geocoder')->error($message);
    drupal_set_message($message, $type);
  }

So anyway the users see the errors there.

Proposed resolution

Remove the drupal_set_message from the Geocoder service, these errors are for the developers/site builders and they should know how to check the log of the site to get more info, no necessary to display this info to the users.

Remaining tasks

Review the patch.
Commit? :-p

User interface changes

None

API changes

None

Thanks!

CommentFileSizeAuthor
#4 2838411-interdiff-2-4.txt492 bytesgnuget
#4 2838411-geocoder-remove-drupal_set_message-4.patch954 bytesgnuget
#2 2838411-geocoder-remove-drupal_set_message.patch800 bytesgnuget
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

gnuget created an issue. See original summary.

gnuget’s picture

gnuget
Primary language Spanish
CreditAttribution: gnuget as a volunteer and at Agaric commented
FileSize
2838411-geocoder-remove-drupal_set_message.patch800 bytes

Patch attached.

Thanks!

Pol’s picture

Pol
the/that
Location Brussels
CreditAttribution: Pol as a volunteer commented
Status: Active » Needs work

Hi,

I'm not against that.

Here's my quick review:

+++ b/src/Geocoder.php
@@ -80,7 +80,6 @@ public function reverse(\double $latitude, \double $longitude, array $plugins, a
   public static function log($message, $type) {

This $type argument could go away too then ?

gnuget’s picture

gnuget
Primary language Spanish
CreditAttribution: gnuget as a volunteer and at Agaric commented
Status: Needs work » Needs review
FileSize
2838411-geocoder-remove-drupal_set_message-4.patch954 bytes
2838411-interdiff-2-4.txt492 bytes

True!

Adding a new version of the patch.

Thanks for your review.

zerolab’s picture

zerolab CreditAttribution: zerolab at Torchbox commented
Status: Needs review » Reviewed & tested by the community

Works like a charm!

  • Pol committed 443ef9b on 8.x-2.x authored by gnuget 
    Issue #2838411 by gnuget: Remove unnecessary drupal_set_messages in the...
Pol’s picture

Pol
the/that
Location Brussels
CreditAttribution: Pol as a volunteer commented
Status: Reviewed & tested by the community » Fixed

Thanks!

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.