Moodle
  1. Moodle
  2. MDL-34633

Flickr repository not working (changes in Flickr API?)

    Details

    • Type: Bug Bug
    • Status: 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
    • Rank:
      43074

      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

          Activity

          Hide
          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
          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
          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
          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
          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
          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
          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
          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
          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
          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

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

              Dates

              • Created:
                Updated:
                Resolved: