pods-framework / pods

The Pods Framework is a Content Development Framework for WordPress - It lets you create and extend content types that can be used for any project. Add fields of various types we've built in, or add your own with custom inputs, you have total control.
https://pods.io/
GNU General Public License v2.0
1.07k stars 264 forks source link

pods_attachment_import unable to add dynamic image from URL #5285

Open tiptronic opened 5 years ago

tiptronic commented 5 years ago

Issue Overview

I'm trying to load a dynamic image from somewhere (here: unsplash.com)

$url = 'https://images.unsplash.com/photo-1542838687-d214d04db0aa?ixlib=rb-1.2.1&ixid=eyJhcHBfaWQiOjEyMDd9&auto=format&fit=crop&w=1502&q=80';
$id = pods_attachment_import($url);
$pieces[ 'fields' ][ 'img' ][ 'value' ] = $id;

Expected Behavior

Image should get imported

Current Behavior

pods_attachment_import($id) always returns '0'

Steps to Reproduce (for bugs)

 add_action( 'pods_api_post_save_pod_item_test', 'change_img', 10, 3 );

function change_img($pieces, $is_new_item, $pod_id) {
    //check if is new item, if not return $pieces without making any changes
    if ( ! $is_new_item ) {
        // return $pieces;
    }
    //make sure that all three fields are active
    $fields = array( 'testimage' );
    foreach( $fields as $field ) {
    if ( ! isset( $pieces[ 'fields_active' ][ $field ] ) ) {
        array_push ($pieces[ 'fields_active' ], $field );
    }
    }

    $url = 'https://images.unsplash.com/photo-1542838687-d214d04db0aa?ixlib=rb-1.2.1&ixid=eyJhcHBfaWQiOjEyMDd9&auto=format&fit=crop&w=1502&q=80';
    $id = pods_attachment_import($url);
    $pieces[ 'fields' ][ 'testimage' ][ 'value' ] = $id;

    return $pieces;
}

Possible Solution

replacing the $url with a static image found on the internet, works fine.

WordPress Environment

4.9.7 and 5.0.3

``` WordPress Version: 5.0.3 PHP Version: 7.2.10 MySQL Version: 5.7.23 Server Software: Apache Your User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_14_2) AppleWebKit/605.1.15 (KHTML, like Gecko) Version/12.0.2 Safari/605.1.15 Session Save Path: /Applications/MAMP/tmp/php Session Save Path Exists: Yes Session Save Path Writeable: Yes Session Max Lifetime: 1440 Opcode Cache: Apc: No Memcached: No OPcache: No Redis: No Object Cache: APC: No APCu: No Memcache: No Memcached: No Redis: No WPDB Prefix: wp_ WP Multisite Mode: No WP Memory Limit: 40M Current Memory Usage: 44.442M Current Memory Usage (real): 6.250M Pods Network-Wide Activated: No Pods Install Location: /Users/andy/Sites/ghs4test/wp-content/plugins/pods/ Pods Tableless Mode Activated: No Pods Light Mode Activated: No Currently Active Theme: Vue Twenty Seventeen Child Currently Active Plugins: Admin Columns Pro: 4.4.1 Admin Columns Pro - Pods: 1.3 All-in-One Event Calendar by Time.ly: 2.5.36 Classic Editor: 1.4 Frontity Image ID Extraction: 1.0.2 Golfheroes Events Enabler: 0.1.0 Pods - Custom Content Types and Fields: 2.7.12 Pods Address Field: 1.0 Pods Maps: 1.0 Pronamic Google Maps: 2.3.2 WordPress Importer: 0.6.4 ```
tiptronic commented 5 years ago

Here's another find: I just found out that the above filter works, but generates files in the uploads directory like this: -23-23-23x-23-23-23@-23x-23 -21-21-21x-21-21-21@-21x-21 photo-1542838687-d214d04db0aa ... (everything without extension). The first two from the mapbox-request, the latter from unsplash.com So it seems the error is that somewhere a filename is created, which then can't get successfully linked

jimtrue commented 5 years ago

@tiptronic Do they have Attachment ID's and can you add those Media Items from the Library to a post afterward?

tiptronic commented 5 years ago

For me $id = pods_attachment_import($url); always returns '0' and the filename is like the pattern in my second comment above.

And the media-library doesn't see the files either... so the answer is 'no'.

tiptronic commented 5 years ago

Could it be, that it makes a difference, if I call it in pods_api_post_save_pod_item_test ?

jimtrue commented 5 years ago

I don't think it has to do with the filter; it has to do with the pods_attachment_import function. That's where it breaks; something about the dynamic URL's isn't being processed properly.

tiptronic commented 5 years ago

I copied Scott's code into my filter above and still get $id of '0'

The image is saved as '600' (without suffix) and shows the kitten (in Mac's Finder), but the media-library doesn't see it.

Could someone maybe try it in a filter as well (like the snippet above)? (Right now I am using WP 5.0.3 and Pods 2.7.12).

jimtrue commented 5 years ago

@tiptronic So, is it writing into the wp-content/uploads/ directory but not adding it to the Media Library? If it doesn't add it to Media Library, there's no Media Attachment ID.

How did you configure this field? Is it configured as Media Library or PLUpload?

tiptronic commented 5 years ago

Media Library

jimtrue commented 5 years ago

@tiptronic So what was the answer to this question:

So, is it writing into the wp-content/uploads/ directory but not adding it to the Media Library?

You said the Mac Finder, that's why I asked.

tiptronic commented 5 years ago

image

tiptronic commented 5 years ago

I guess the media-library doesn't see it, because it has no extension. I found a different mapbox-api where I can add an extension (it's basically the same api) and then everything works like a treat

jimtrue commented 5 years ago

@tiptronic Awesome! That's my guess as well (on the lack of an extension). glad to hear you're working. This still seems like something we should look into, but obviously it's less 'hot' now.

It seems odd to be I can add that Unsplash image into the WordPress Media Library using the 'Import from URL' option that is only available in the Media Modal (it's not an available way to add media using just the straight Media Library in the admin menu; it's only available when you 'add media' to a post). So something they are doing in that process, is creating an extension.

tiptronic commented 5 years ago

I'm not a PHP guy, but for me it bails out here:

$wp_filetype = wp_check_filetype( $filename );

if ( ! $wp_filetype['type'] || ! $wp_filetype['ext'] ) {
    return 0;
}

From my understanding, the first lines of the pods_attachment_import are a bit problematic, because they squeeze and split the passed url and compile a new $filename, without checking, if this is a possible valid file. Since this filename (in my case) doesn't have an extension, the wordpress-function above returns 0.

A simple solution would be to check the file itself (e.g. with exif_imagetype ( string $filename ), which reads the first couple of bytes of the file and thereby detects the most common filetypes.

Another way would be to pass a default filetype and pass this to another wp-function: wp_check_filetype_and_ext( $filename, $filename_with_extension )

I personally wanted to pass a dedicated filename anyway, so I extended the function:

function pods_attachment_import( $url, $post_parent = null, $featured = false, $requested_filename = '' )

This allows me to a) verify the filetype based on the $requested_filename parameter (which contains an extension) b) renames the file before adding it to the media-library - so I have a proper filename.

Ideally I would pass a $requested_filename_and_path, so my added files are moved to a nice path inside the media-library... But I'm not sure, if there are other techniques to do this which are preferable.

tiptronic commented 5 years ago

For completeness sake, here's how the modified function looks like:

function pods_attachment_import( $url, $post_parent = null, $featured = false, $requested_filename ) {

    if ($requested_filename && !empty($requested_filename)) {
        $filename = $requested_filename;
    } else {
        $filename = explode( '?', $url );
        $filename = $filename[0];
        $filename = explode( '#', $filename );
        $filename = $filename[0];
        $filename = substr( $filename, ( strrpos( $filename, '/' ) ) + 1 );
    }

        $title = substr( $filename, 0, ( strrpos( $filename, '.' ) ) );
        ...
jimtrue commented 5 years ago

@sc0ttkclark or @pglewis Can you two look at this one, when you get a chance. @tiptronic last update gives the best details I think on what is the crux of the issue.

If you think his additions are actually valid (since they enhance the existing function, without breaking the function), this might be a good fix.

niarchos commented 5 years ago

Is there any estimation on when this is going to be fixed? or... is there a workaround?

niarchos commented 5 years ago

As part of the solution, note that 'post_mime_type' should be set in case there is a dynamic image

tiptronic commented 5 years ago

Unfortunately, the update from 2.7.12 to 2.7.15 broke it again for me... I have yet to find out what the problem is, but since this issue is still flagged as open, it seems it has some side-effects... For my current project, I had to go back to 2.7.12, so I will setup a test-project to see what's going on.