Moodle

Flickr repository not working (changes in Flickr API?)

Details

  • Type: Bug Bug
  • Status: Closed Closed
  • Priority: Critical Critical
  • Resolution: Fixed
  • Affects Version/s: 2.2.4, 2.3.1, 2.4
  • Fix Version/s: 2.2.5, 2.3.2
  • Component/s: Repositories
  • Labels:
  • Testing Instructions:
    Hide

    Test pre-requisites

    • Access to a Flickr account
    • Flickr and Flickr public repository enabled
    • Flickr repository set up
    • Created an instance of Flickr Public using your account's email address
    • Two pics on the Flickr account
      • One massively bigger than 320x240 (1024x768 recommended)
      • One very small (any 16x16 icon from Moodle will do)

    Test steps

    1. Go to your private files, click 'Add'
    2. Browse your Flickr repository
      1. Add the small image
      2. Make sure it is correctly download (check filesize, try to preview it)
      3. Add the large image
      4. Make sure it is correctly download (check filesize, try to preview it)
      5. Make sure the image dimensions are close or exactly the same as the original one
    3. Browse your Flickr Public instance
      1. Add the small image
      2. Make sure it is correctly download (check filesize, try to preview it)
      3. Add the large image
      4. Make sure it is correctly download (check filesize, try to preview it)
      5. Make sure the image dimensions are close or exactly the same as the original one
    Show
    Test pre-requisites Access to a Flickr account Flickr and Flickr public repository enabled Flickr repository set up Created an instance of Flickr Public using your account's email address Two pics on the Flickr account One massively bigger than 320x240 (1024x768 recommended) One very small (any 16x16 icon from Moodle will do) Test steps Go to your private files, click 'Add' Browse your Flickr repository Add the small image Make sure it is correctly download (check filesize, try to preview it) Add the large image Make sure it is correctly download (check filesize, try to preview it) Make sure the image dimensions are close or exactly the same as the original one Browse your Flickr Public instance Add the small image Make sure it is correctly download (check filesize, try to preview it) Add the large image Make sure it is correctly download (check filesize, try to preview it) Make sure the image dimensions are close or exactly the same as the original one
  • Affected Branches:
    MOODLE_22_STABLE, MOODLE_23_STABLE, MOODLE_24_STABLE
  • Fixed Branches:
    MOODLE_22_STABLE, MOODLE_23_STABLE
  • Pull from Repository:
  • Pull Master Branch:
    MDL-34633-master
  • Pull Master Diff URL:

Description

After picking a file from Flickr repository it has 0 size. Seems that Flickr API has changed recently.

After digging a little I noticed that function repository_flickr::build_photo_url() returns empty string. Flickr method flickr.photos.getSizes returns smaller array than we expect.

Did not test on versions smaller than 2.3

Issue Links

is blocked by

Bug - A problem which impairs or prevents Moodle from functioning correctly. MDL-35101 Flickr public fails when picture does not have a license

  • Minor - Minor loss of function where workaround is possible
  • Closed - The issue is considered finished, the resolution is correct. Issues which are closed should not be reopened.

Activity

Ascending order - Click to sort in descending order
Hide
Permalink
Marina Glancy added a comment -

I was trying to test on 2.2 and I don't even see Flickr in the list of available repositories (even though it's enabled).

Was trying to test on qa.moodle.net and it's even worse. I'm trying to add a file in my private files from flickr. I click login, and it displays me an error: Sorry, but portfolio exports are not enabled in this site

Show
Marina Glancy added a comment - I was trying to test on 2.2 and I don't even see Flickr in the list of available repositories (even though it's enabled). Was trying to test on qa.moodle.net and it's even worse. I'm trying to add a file in my private files from flickr. I click login, and it displays me an error: Sorry, but portfolio exports are not enabled in this site
Hide
Permalink
Marina Glancy added a comment -

that's the trick. My files in Flickr are small icons. Picking files works for big files

Show
Marina Glancy added a comment - that's the trick. My files in Flickr are small icons. Picking files works for big files
Hide
Permalink
Frédéric Massart added a comment -

As Marina had discovered it, the error occurs when the file is too small and we're trying to get a bigger size than available. I changed the behaviour so that we are trying to get the greatest one (considering 320x240 the greatest as previously) but will prevent failure when this size is not available.

Show
Frédéric Massart added a comment - As Marina had discovered it, the error occurs when the file is too small and we're trying to get a bigger size than available. I changed the behaviour so that we are trying to get the greatest one (considering 320x240 the greatest as previously) but will prevent failure when this size is not available.
Hide
Permalink
Frédéric Massart added a comment -

Pushing for peer review. I have added some caching so that we don't query twice Flickr to get the file sizes. And if we know the originalsecret (premium member on Flickr) then we use the original size of the image. If not, we now get the greatest image.

Further improvements could be done by merging the logic from both Flickr Public and Flickr into one parent class.

The second commit is there because I rebased my branch on top of MDL-35101.

Show
Frédéric Massart added a comment - Pushing for peer review. I have added some caching so that we don't query twice Flickr to get the file sizes. And if we know the originalsecret (premium member on Flickr) then we use the original size of the image. If not, we now get the greatest image. Further improvements could be done by merging the logic from both Flickr Public and Flickr into one parent class. The second commit is there because I rebased my branch on top of MDL-35101 .
Hide
Permalink
Marina Glancy added a comment -

code looks fine.
Fred, please add to testing instructions attempt to pick a non-downloadable file

Show
Marina Glancy added a comment - code looks fine. Fred, please add to testing instructions attempt to pick a non-downloadable file
Hide
Permalink
Frédéric Massart added a comment -

As discussed IRL, the non-downloadable file does not apply to this issue. Cheers!

Show
Frédéric Massart added a comment - As discussed IRL, the non-downloadable file does not apply to this issue. Cheers!
Hide
Permalink
Eloy Lafuente (stronk7) added a comment -

The main moodle.git repository has just been updated with latest weekly modifications. You may wish to rebase your PULL branches to simplify history and avoid any possible merge conflicts. This would also make integrator's life easier next week.

TIA and ciao

Show
Eloy Lafuente (stronk7) added a comment - The main moodle.git repository has just been updated with latest weekly modifications. You may wish to rebase your PULL branches to simplify history and avoid any possible merge conflicts. This would also make integrator's life easier next week. TIA and ciao
Hide
Permalink
Eloy Lafuente (stronk7) added a comment -

Integrated (22, 23 & master), thanks!

PS: I don't like much to see those blocks of code copied to the 2 repos. Just guessing if it would be possible to create some OOP parentship between them to save those duplications.

Show
Eloy Lafuente (stronk7) added a comment - Integrated (22, 23 & master), thanks! PS: I don't like much to see those blocks of code copied to the 2 repos. Just guessing if it would be possible to create some OOP parentship between them to save those duplications.
Hide
Permalink
Rossiani Wijaya added a comment -

This looks great.

Thanks Fred for fixing.

Test passed.

Show
Rossiani Wijaya added a comment - This looks great. Thanks Fred for fixing. Test passed.
Hide
Permalink
Eloy Lafuente (stronk7) added a comment -

Many thanks for the hard work.

These changes have been spread upstream and are already available in the git and cvs repositories.

Ciao

Show
Eloy Lafuente (stronk7) added a comment - Many thanks for the hard work. These changes have been spread upstream and are already available in the git and cvs repositories. Ciao

People

Vote (0)
Watch (1)

Dates

  • Created:
    Updated:
    Resolved: