Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Photos and collections returning PageResult #94

Conversation

CiotkaCierpienia
Copy link

I've added the possibility for returning PageResult for photos and collections because for me it is much easier to maintain only one return type across searching and listing photos and collections. I hope it also resolves the apapazisis's issue #72 .

@Dayjo
Copy link

Dayjo commented Nov 4, 2019

@MahdiMajidzadeh @lukechesser Is there any chance of this getting merged in? Just came across the same issue as #72 which makes my sdk calls pretty inconsistent.

Additionally I don't think I'm even able to use this at all as I'm unable to use getHeaders, getTotal, getTotalPages, getResults etc?

@Dayjo
Copy link

Dayjo commented Nov 4, 2019

Just a quick note on this after merging it into my fork so that I could use it.

It looks like headers and such are being added to the results because of this loop;

$pageResults['results'] = [];
        foreach($this->photos["{$page}-{$per_page}"] as $photos2){
            foreach($photos2 as $photo) {
                if(is_array($photo)){
                    $photo = new Photo($photo);
                }
                $pageResults['results'][] = $photo->getParameters();
            }
        }

I updated it to;

$pageResults['results'] = [];
        foreach($this->photos["{$page}-{$per_page}"] as $photos2){
            foreach($photos2 as $photo) {
                if(is_object($photo)){
                   $pageResults['results'][] = $photo->getParameters();
                }
            }
        }

Which only adds the photos to the results and seems to work

@aaronklaassen
Copy link
Member

@CiotkaCierpienia @Dayjo

I just merged a bunch of updates into master getting ready for a 3.0 release. Clearing out a ton of deprecated code, updating dependencies, etc. If you want to update this PR with that, I'll happily merge it.

Also you won't need the $returnArrayObject switch - because this is for a major version bump, you don't need to worry about the backwards compatibility.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants