Uploaded image for project: 'Moodle'
  1. Moodle
  2. MDL-34633

Flickr repository not working (changes in Flickr API?)

    Details

    • Type: Bug
    • Status: Closed
    • Priority: 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

      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

        Gliffy Diagrams

          Issue Links

            Activity

            Hide
            marina 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 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
            marina Marina Glancy added a comment -

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

            Show
            marina Marina Glancy added a comment - that's the trick. My files in Flickr are small icons. Picking files works for big files
            Hide
            fred 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
            fred 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
            fred 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
            fred 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
            marina Marina Glancy added a comment -

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

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

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

            Show
            fred Frédéric Massart added a comment - As discussed IRL, the non-downloadable file does not apply to this issue. Cheers!
            Hide
            stronk7 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
            stronk7 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
            stronk7 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
            stronk7 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
            rwijaya Rossiani Wijaya added a comment -

            This looks great.

            Thanks Fred for fixing.

            Test passed.

            Show
            rwijaya Rossiani Wijaya added a comment - This looks great. Thanks Fred for fixing. Test passed.
            Hide
            stronk7 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
            stronk7 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

              • Votes:
                0 Vote for this issue
                Watchers:
                1 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved:
                  Fix Release Date:
                  10/Sep/12