Piwigo / piwigo-openstreetmap

OpenStreetMap integration for Piwigo
http://piwigo.org/ext/extension_view.php?eid=701
GNU General Public License v3.0
35 stars 35 forks source link

Error on (attempted) synchronize #8

Closed marjoleink closed 10 years ago

marjoleink commented 10 years ago

Not knowing which of the photos uploaded so far already might have geo information embedded, I tried a synchronize on just one sub-album. Running with the "only perform a simulation (nothing will be changed in the database)" on, the result was "1 photos candidates for metadata synchronization". I then turned simulation off and ran it on the same sub-album with result:

Notice: Undefined variable: infos in /home/mkatsma/public_html/pwg/plugins/piwigo-openstreetmap/admin/admin_sync.php on line 110

Warning: Cannot modify header information - headers already sent by (output started at /home/mkatsma/public_html/pwg/plugins/piwigo-openstreetmap/admin/admin_sync.php:110) in /home/mkatsma/public_html/pwg/include/page_header.php on line 101

(I also noticed a whole series of messages flashing by when activating the plugin; but these disappeared because the page refreshed!)

Running Piwigo 2.5.2 (installed yesterday) and freshly-installed (today) openstreetmap plugin.

xbgmsharp commented 10 years ago

Hello,

Are you using the latest version from github or the release from piwigo. After a first thought it seen that it is missing the declaration. Add the following line after lines 71.

$infos = array();

It should do the trick. If it works i will update the code this weekend.

marjoleink commented 10 years ago

Hi, thanks for the fast reaction. I installed via Softaculous (which many hosters provide via cPanel), which as far as I know always pulls the latest stable release from 'official' download. So that would likely be from the Piwigo site in this case. I'll try adding that line and report back!

marjoleink commented 10 years ago

OK, added the extra line (with $infos, not $info). Now the error message no longer appears - but I still cannot do actual synchronization: for instance I select a sub-album with 2 photos, and running the simulation tells me there are 2 'candidates'. When I try actual synchronization, nothing happens, and there still are 2 candidates.

Maybe something else is wrong? Is there a way to reveal the error messages I saw flashing by (before P did a redirect) when activating the plugin? (BTW the menu entry OS World Map does appear in the menu.)

marjoleink commented 10 years ago

Found the problem, after analysing admin_sync.php and the accompanying template: I was initially confused by the fact that neither $infos nor $errors were reported - this is a result of incomplete error trapping, where not all possible cases of 'no lat lon information, or incomplete' were actually reported in $errors, while continue statements skip past entries in $infos.

I was also confused by the term 'candidates' which I understood to be 'files that have lat lon info' or at least 'files that have EXIF info'. It would be helpful to explain on the documentation page what 'candidates' actually means (just all the files in the current selection).

I have now tweaked admin_sync.php so that all cases of missing or incomplete lat-lon info in the EXIF are reported in $errors. (Though it is not strictly an error, as lat-lon information is not required to be in EXIF.)

The result shows that none of my (current) files actually have lat-lon information...

So, problem solved. If you'd like my version of admin_sync.php please let me know how to get it to you!

xbgmsharp commented 10 years ago

Hello,

As you mention, missing EXIF are not reported, because it is not an errors. Incomplete lat-lon info in the EXIF are reported in $errors. I add a info message saying, "No photos have exif_data", if no photos with EXIF data are found. The candidates number is for the number elements having lat , lon in the DB according to your filter (eg: category). However it is not necessarily an images, specially if you have piwigo-videojs installed. So I replaced the message: "photos candidates for metadata synchronization" by "elements with metadata"

Is it more clear?

You can check with this SQL query, why the candidates numbers was not the number of photos getting sync. Replace the '(5)' by the number of the category you want to check.

SELECT `id`, `path` ,`lat` ,`lon` FROM piwigo_images INNER JOIN piwigo_image_category ON id=image_id WHERE category_id IN (5) GROUP BY id 

The message you have when activating the plugin, was more likely with this commit ba8a79c867 not release.

You might want to use the latest Github version if you can. It would be great to get feedback prior the next release.

xbgmsharp commented 10 years ago

To avoid changing message for translators, I updated the SQL query to list only files which support EXIF headers. See commit 1d581b4c53

xbgmsharp commented 10 years ago

if you could test with latest Github version, it would be great to get more feedback prior the next release.

marjoleink commented 10 years ago

I'll do my best to test later today! Sorry, fried brain... tomorrow, for sure!

marjoleink commented 10 years ago

OK, tested! Much better already!

Still, I have a few issues, mainly:

I adapted admin_sync.php (with comments) and admin_sync.tpl and will email those to you (plus some more screenshots to show you my results) - can I use the email found in the file headers? Sorry, I have not git set up yet (and I was in a hurry to get Piwigo up and running with some extra functionality :))

Here's one teaser: warning simulation only

xbgmsharp commented 10 years ago

Thanks for the feedback. You can use the email found in the file headers.

marjoleink commented 10 years ago

Done!