Problem/Motivation

Currently this module doesn't checks if IP of current request is from EU or not. It always defaults to TRUE.

Proposed resolution

While there isn't a defined way how IP match should work, I suggest add an alter to that $geo_match variable, so we, as developers, can hook in to the process and implement the IP matching logic for our own needs.

I'm providing a small patch below.

CommentFileSizeAuthor
#10 eu_cookie_compliance-enable_hook_for_geo_ip_match-2848074-9.patch1.32 KBsvenryen
#2 enable_hook_for_geo_ip_match-2848074-2.patch632 bytesSpurlos
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Spurlos created an issue. See original summary.

Spurlos’s picture

Spurlos CreditAttribution: Spurlos at FFW commented
FileSize
enable_hook_for_geo_ip_match-2848074-2.patch632 bytes
killua99’s picture

killua99 CreditAttribution: killua99 commented
Status: Active » Needs work
svenryen’s picture

svenryen CreditAttribution: svenryen at Ramsalt Lab commented

@killua99 what are the reasons why this issue needs work? Is it in need of a review only or do you notice any issues with the patch?

killua99’s picture

killua99 CreditAttribution: killua99 commented
Priority: Major » Normal
Status: Needs work » Postponed

I forgot about that, to tell why need work and I won't thing is needed.

Because the patch is a hack, to solve a situation about a thirdparty module. It add just a simple function to create a custom module to alter that variable. It should be handle for that module directly. If that guy will create a custom module to handle IP then why not submit that code on the thirdparty module that we're waiting for?

svenryen’s picture

svenryen CreditAttribution: svenryen at Ramsalt Lab commented

I disagree. I think we can add this patch to the module, but I recommend that we name the hook "eu_cookie_compliance_geoip_match"

svenryen’s picture

svenryen CreditAttribution: svenryen at Ramsalt Lab commented
Status: Postponed » Needs review
svenryen’s picture

svenryen CreditAttribution: svenryen at Ramsalt Lab commented
Status: Needs review » Needs work

I does however need an additional `eu_cookie_compliance.api.php` file to describe the alter hook.

svenryen’s picture

svenryen CreditAttribution: svenryen at Ramsalt Lab commented

Here's a complete patch, with the necessary api documentation.

svenryen’s picture

svenryen CreditAttribution: svenryen at Ramsalt Lab commented
FileSize
eu_cookie_compliance-enable_hook_for_geo_ip_match-2848074-9.patch1.32 KB

  • svenryen committed f7b9c5e on 8.x-1.x authored by Spurlos 
    Issue #2848074 by svenryen, Spurlos: Enable hook for altering geo IP...
svenryen’s picture

svenryen CreditAttribution: svenryen at Ramsalt Lab commented
Status: Needs work » Fixed
svenryen’s picture

svenryen CreditAttribution: svenryen at Ramsalt Lab commented
Status: Fixed » Closed (fixed)