search

josephlacey / com.jlacey.electoral

GNU Affero General Public License v3.0
6 stars 12 forks source link

US legislative districts - State info - Display error - How-to FIX #30

Closed privaterra closed 3 years ago

privaterra commented 3 years ago

When I ran the job to add the - Country Districts (Daily)- information (ie. US legislative districts to contacts via the Google Civic Information API) - the State information did not display properly. Specifically, instead of seeing the state name, the state ID was displayed instead.

Going through the code and my CiviCRM database, I was able to - FIX - the issue by manually updating the - civicrm_custom_field Table . Specifically, I changed the entry for - electoral_states_provinces - so that the fields are set as follows:

label: States/Provinces data_type: StateProvince html_type: Select

After making the change, the State Name is now displayed.

laryn commented 3 years ago

:+1: I can confirm the fix on an install -- I changed the table value directly (as described) and then the "Electoral Districts" tab on the contact began showing the text value for states/provinces instead of the ID.

I see @privaterra has a related PR here: https://github.com/josephlacey/com.jlacey.electoral/pull/31

josephlacey commented 3 years ago

Thanks @privaterra and @laryn. I'm going to leave this open and unmerged because I don't have time to test myself and there's been a version 3 rewrite that you'll eventually want to upgrade to. That's tracked on a forked repo and involves an update to the code base to more contemporary coding practices. It's not up to feature parity yet, so I wouldn't upgrade until that happens, but version 2 is minimally supported.

https://github.com/MegaphoneJon/com.jlacey.electoral

privaterra commented 3 years ago

Joseph,

I wasn’t aware of the V3 forked repo, so thanks for sharing the link!

I tried it briefly, but unfortunately ran into issues having it work with the Google API & the wordpress install I have setup :(

I tried the Cicero API, but it wasn’t ideal as it DID NOT add - Bioguide ID - that I use to merge in other datasets into the US congress based data that our system manages.

If I can be of help to test the new repo, please, let me know!

regards,

Robert

-- Robert Guerra Cel/Tel +1 416 893 0377 Twitter: twitter.com/netfreedom Email: @.***

On 11 May 2021, at 10:23, Joseph Lacey wrote:

Thanks @privaterra and @laryn. I'm going to leave this open and unmerged because I don't have time to test myself and there's been a version 3 rewrite that you'll eventually want to upgrade to. That's tracked on a forked repo and involves an update to the code base to more contemporary coding practices. It's not up to feature parity yet, so I wouldn't upgrade until that happens, but version 2 is minimally supported.

https://github.com/MegaphoneJon/com.jlacey.electoral

-- You are receiving this because you were mentioned. Reply to this email directly or view it on GitHub: https://github.com/josephlacey/com.jlacey.electoral/issues/30#issuecomment-838567679

MegaphoneJon commented 3 years ago

Hi @privaterra - this is valuable feedback. I can add the Bioguide ID. Can you say more about the issues you ran into?

privaterra commented 3 years ago

On a clean/fresh install of Wordpres / CiviCRM, I did the following:

John Doe Address: 815 Eddy Street, San Francisco, CA, 94109, USA

Data provider: Google Civic API Key: Districts to Look Up: Country Address location for district lookup: Primary Countries: United States States: none - (but, All States/Provinces is selected)

District Lookup on Address Update - Off

There has been a critical error on this website. Please check your site admin email inbox for instructions.

Looking at the debug file, here’s what’s there:

[16-May-2021 12:10:14 America/New_York] PHP Fatal error: Declaration of Civi\Electoral\Api\GoogleCivicInformation::reps() must be compatible with Civi\Electoral\AbstractApi::reps(): array in /var/www/html/wp-content/uploads/civicrm/ext/com.jlacey.electoral/Civi/Electoral/Api/GoogleCivicInformation.php on line 12

Which has me think that it’s some sort of API related error. But, not sure.

Please advise!

-- Robert Guerra Cel/Tel +1 416 893 0377 Twitter: twitter.com/netfreedom Email: @.***

On 12 May 2021, at 9:54, Jon wrote:

Hi @privaterra - this is valuable feedback. I can add the Bioguide ID. Can you say more about the issues you ran into?

-- You are receiving this because you were mentioned. Reply to this email directly or view it on GitHub: https://github.com/josephlacey/com.jlacey.electoral/issues/30#issuecomment-839791752

MegaphoneJon commented 3 years ago

Hi Robert - that's my fault. One of the changes in Electoral API 3.0 is that there's more shared code between the data providers - which makes it easier to add new ones.

However, I was under contract from Cicero and in a bit of deadline pressure, didn't go back and test the Google code after making some changes.

This should really be its own issue, and I'm not at a computer I can test with, but I'll post a fix in the next day or so. If you want to test the fix and are comfortable editing the files, change Civi/Electoral/Api/GoogleCivicInformation.php line 25 from this:

  public function reps() {

to this:

  public function reps() : array {
privaterra commented 3 years ago

OK.

I made the revision to line 25 in Civi/Electoral/Api/GoogleCivicInformation.php as you mention.

I then executed the Electoral API - Districts Lookup Job…

Get another error this time. There’s the message in the debug file:

[16-May-2021 13:29:24 America/New_York] PHP Warning: usort() expects parameter 2 to be a valid callback, function 'electoral_division_sort' not found or invalid function name in /var/www/html/wp-content/uploads/civicrm/ext/com.jlacey.electoral/Civi/Electoral/Api/GoogleCivicInformation.php on line 61 [16-May-2021 13:29:24 America/New_York] PHP Fatal error: Uncaught ArgumentCountError: Too few arguments to function Civi\Electoral\AbstractApi::writeDistrictData(), 8 passed in /var/www/html/wp-content/uploads/civicrm/ext/com.jlacey.electoral/Civi/Electoral/Api/GoogleCivicInformation.php on line 113 and exactly 13 expected in /var/www/html/wp-content/uploads/civicrm/ext/com.jlacey.electoral/Civi/Electoral/AbstractApi.php:233 Stack trace:

0 /var/www/html/wp-content/uploads/civicrm/ext/com.jlacey.electoral/Civi/Electoral/Api/GoogleCivicInformation.php(113): Civi\Electoral\AbstractApi->writeDistrictData(25, 'locality', 1004, NULL, 'California's 12...', NULL, '12', false)

1 /var/www/html/wp-content/uploads/civicrm/ext/com.jlacey.electoral/Civi/Electoral/AbstractApi.php(74): Civi\Electoral\Api\GoogleCivicInformation->parseDistrictData(Array)

2 /var/www/html/wp-content/uploads/civicrm/ext/com.jlacey.electoral/api/v3/Electoral/Districts.php(24): Civi\Electoral\AbstractApi->districts(50, true)

3 /var/www/html/wp-content/plugins/civicrm/civicrm/Civi/API/Provider/MagicFu in /var/www/html/wp-content/uploads/civicrm/ext/com.jlacey.electoral/Civi/Electoral/AbstractApi.php on line 233

regards,

Robert

-- Robert Guerra Cel/Tel +1 416 893 0377 Twitter: twitter.com/netfreedom Email: @.***

On 16 May 2021, at 12:24, Jon wrote:

Hi Robert - that's my fault. One of the changes in Electoral API 3.0 is that there's more shared code between the data providers - which makes it easier to add new ones.

However, I was under contract from Cicero and in a bit of deadline pressure, didn't go back and test the Google code after making some changes.

This should really be its own issue, and I'm not at a computer I can test with, but I'll post a fix in the next day or so. If you want to test the fix and are comfortable editing the files, change Civi/Electoral/Api/GoogleCivicInformation.php line 25 from this:

  public function reps() {

to this:

  public function reps() : array {

-- You are receiving this because you were mentioned. Reply to this email directly or view it on GitHub: https://github.com/josephlacey/com.jlacey.electoral/issues/30#issuecomment-841840074

MegaphoneJon commented 3 years ago

Ugh - this is real amateur hour stuff on my part.

I'm going to sit down and do a fresh install, I'll update you when I've got it working. I'll issue a version 3.01.

MegaphoneJon commented 3 years ago

Hi Robert,

OK, I fixed both of the issues you described, and it's now looking up district data successfully. However, I did get a "too many requests per minute" error - which is odd, because I don't think I've seen that before, and earlier versions of this extension don't seem to have any sort of rate limiter (there's the "maximum requests" limit, but that's all).

I'd be curious to know how it's working for you once you install my latest version! It's available on my repo now: https://github.com/MegaphoneJon/com.jlacey.electoral

privaterra commented 3 years ago

Jon,

First, Thanks for your help troubleshooting this :)

Second, perhaps indeed this should be moved to a new ticket/issue

Third, a partial success with your new revision ….

I did a git pull to update the code for the extension, and then ran the - Electoral API - Districts Lookup - Job… SUCCESS!

There’s good and not so great news…

  1. The Good News - The - Electoral API - Districts Lookup join no longer returns an error…

here’s what appears in the Log…

2021-05-16 16:41:21 Electoral API - Districts Lookup Entity: Electoral Action: districts

Summary Finished execution of Electoral API - Districts Lookup with result: Success (a:1:{i:0;s:23:"3 addresses districted.";})

Details

Parameters raw (from db settings): limit=50 update=false

Parameters parsed (and passed to API method): a:3:{s:7:"version";i:3;s:5:"limit";s:2:"50";s:6:"update";s:5:"false";}

Full message: Finished execution of Electoral API - Districts Lookup with result: Success (a:1:{i:0;s:23:"3 addresses districted.";})

  1. No so great news - Looking at the details, more was fetched than was desired. Looks like it fetched - Everything -

Although I just have - Districts to look up - Countries , in my - Electoral settings, I’m getting ALL the lookups.

Specifically, I looked at my - test - entry “John Doe” with the following address - 815 Eddy Street, San Francisco, CA 94109, United States

Level, States/Provinces, County, City, Chamber, District, Note , In Office, Last Updated, City, California, x , California's 12th congressional district, x , 12, x, x, 05/16/2021 City: California, x , California State Senate district 11, x, 11, x, x, 05/16/2021 City: California, x , San Francisco County, x, san_francisco, x,x, 05/16/2021 City: California, x , California Assembly district 17, x, 17, x, x, 05/16/2021 County, California, California, , , ca, x, x, 05/16/2021 City: California, x, San Francisco CA city-county council district 5, x, san_francisco/council_district, x, x , 05/16/2021

x = blank

-- Robert Guerra Cel/Tel +1 416 893 0377 Twitter: twitter.com/netfreedom Email: @.***

On 16 May 2021, at 16:27, Jon wrote:

Hi Robert,

OK, I fixed both of the issues you described, and it's now looking up district data successfully. However, I did get a "too many requests per minute" error - which is odd, because I don't think I've seen that before, and earlier versions of this extension don't seem to have any sort of rate limiter (there's the "maximum requests" limit, but that's all).

I'd be curious to know how it's working for you once you install my latest version! It's available on my repo now: https://github.com/MegaphoneJon/com.jlacey.electoral

-- You are receiving this because you were mentioned. Reply to this email directly or view it on GitHub: https://github.com/josephlacey/com.jlacey.electoral/issues/30#issuecomment-841872983

MegaphoneJon commented 3 years ago

Hi Robert - thank YOU for being an accidental guinea pig! I'll look into this latest issue, I created a ticket #32.