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

Backport webservice/upload.php 2.2 changes in 2.1 when possible

    Details

    • Type: Bug
    • Status: Closed
    • Priority: Blocker
    • Resolution: Fixed
    • Affects Version/s: 2.1.3, 2.2, 2.3
    • Fix Version/s: 2.1.4, 2.2
    • Component/s: Web Services
    • Labels:

      Description

      We missed few fixes in 2.1:

      2.2 commit history: MDL-27497, MDL-29036, MDL-29602, MDL-28646, MDL-28646, MDL-30459 (integrated), MDL-30459(integrated), MDL-30496 (integration review)
      2.1 commit history: MDL-27497, MDL-29036

        Gliffy Diagrams

          Attachments

            Issue Links

              Activity

              jerome Jérôme Mouneyrac created issue -
              jerome Jérôme Mouneyrac made changes -
              Field Original Value New Value
              Priority Minor [ 4 ] Blocker [ 1 ]
              jerome Jérôme Mouneyrac made changes -
              Link This issue has been marked as being related by MDL-30496 [ MDL-30496 ]
              jerome Jérôme Mouneyrac made changes -
              Fix Version/s DEV backlog [ 10464 ]
              Pull 2.1 Branch MDL-30542
              Pull 2.1 Diff URL https://github.com/mouneyrac/moodle/compare/MOODLE_21_STABLE...MDL-30542
              Pull from Repository git://github.com/mouneyrac/moodle.git
              jerome Jérôme Mouneyrac made changes -
              Status Open [ 1 ] Waiting for peer review [ 10012 ]
              Hide
              jerome Jérôme Mouneyrac added a comment -

              Apu I added you as peer reviewer (for after 2.2 release). You can give it to someone else if you don't have time. The only difference with 2.2 is that I didn't integrate MDL-29036 as it's not in 2.1.

              Show
              jerome Jérôme Mouneyrac added a comment - Apu I added you as peer reviewer (for after 2.2 release). You can give it to someone else if you don't have time. The only difference with 2.2 is that I didn't integrate MDL-29036 as it's not in 2.1.
              jerome Jérôme Mouneyrac made changes -
              Peer reviewer nebgor
              salvetore Michael de Raadt made changes -
              Labels triaged
              jerome Jérôme Mouneyrac made changes -
              Peer reviewer nebgor rwijaya
              rwijaya Rossiani Wijaya made changes -
              Original Estimate 0 minutes [ 0 ]
              Remaining Estimate 0 minutes [ 0 ]
              Status Waiting for peer review [ 10012 ] Peer review in progress [ 10013 ]
              Hide
              rwijaya Rossiani Wijaya added a comment -

              Hi Jerome,

              Some minor issues that I spotted:

              1. In webservice/upload.php, typo on line 22 'aaera'.
              on line 126, there is double ';'.

              2. In lang/en/webservice.php, for 'servicenotavailable' string, the first letter need to uppercase and maybe it should be re-word to something like "Web service is not available (it is not exist or might be set to disable)".

              3. In webservice/lib.php, on line 83, should be 'cannot' and probably should be re-word to 'Refuse non-admin authentication in maintenance mode).
              on line 68, do you mean 'retrieve user info from the token'?

              4. It needs some test instructions.

              I will continue reviewing the patch tomorrow.

              Show
              rwijaya Rossiani Wijaya added a comment - Hi Jerome, Some minor issues that I spotted: 1. In webservice/upload.php, typo on line 22 'aaera'. on line 126, there is double ';'. 2. In lang/en/webservice.php, for 'servicenotavailable' string, the first letter need to uppercase and maybe it should be re-word to something like "Web service is not available (it is not exist or might be set to disable)". 3. In webservice/lib.php, on line 83, should be 'cannot' and probably should be re-word to 'Refuse non-admin authentication in maintenance mode). on line 68, do you mean 'retrieve user info from the token'? 4. It needs some test instructions. I will continue reviewing the patch tomorrow.
              Hide
              rwijaya Rossiani Wijaya added a comment -

              Hi Jerome,

              The rest of the code looks good.

              Show
              rwijaya Rossiani Wijaya added a comment - Hi Jerome, The rest of the code looks good.
              rwijaya Rossiani Wijaya made changes -
              Status Peer review in progress [ 10013 ] Development in progress [ 3 ]
              Hide
              jerome Jérôme Mouneyrac added a comment -

              Thanks Rosie, I'll make the changes and submit it

              Show
              jerome Jérôme Mouneyrac added a comment - Thanks Rosie, I'll make the changes and submit it
              jerome Jérôme Mouneyrac made changes -
              Status Development in progress [ 3 ] Waiting for integration review [ 10010 ]
              jerome Jérôme Mouneyrac made changes -
              Testing Instructions Try you can upload a file with the demo file upload client: https://github.com/moodlehq/sample-ws-clients/tree/master/PHP-HTTP-filehandling
              nebgor Aparup Banerjee made changes -
              Status Waiting for integration review [ 10010 ] Integration review in progress [ 10004 ]
              Integrator nebgor
              Currently in integration Yes [ 10041 ]
              Hide
              nebgor Aparup Banerjee added a comment -

              Hi Jerome,
              I just had a quick glance (gtg play futsal with you): $string['servicenotavailable'] = 'Web service is not available (it is not existing or might be set to disable)'; --> disabled , not sure if thats in the other branches as well

              Show
              nebgor Aparup Banerjee added a comment - Hi Jerome, I just had a quick glance (gtg play futsal with you): $string ['servicenotavailable'] = 'Web service is not available (it is not existing or might be set to disable)'; --> disabled , not sure if thats in the other branches as well
              Hide
              jerome Jérôme Mouneyrac added a comment - - edited

              Hi Apu,
              good catch, I'm going to add a quick fix for the 2.2 branch here if it's ok

              Show
              jerome Jérôme Mouneyrac added a comment - - edited Hi Apu, good catch, I'm going to add a quick fix for the 2.2 branch here if it's ok
              Hide
              nebgor Aparup Banerjee added a comment -

              I've spoken to Jerome,
              we'll reopen this as there are few minor (ie:typos/strings) issues to look at (both back porting and the fixes already in 2.2).

              Show
              nebgor Aparup Banerjee added a comment - I've spoken to Jerome, we'll reopen this as there are few minor (ie:typos/strings) issues to look at (both back porting and the fixes already in 2.2).
              nebgor Aparup Banerjee made changes -
              Status Integration review in progress [ 10004 ] Reopened [ 4 ]
              Hide
              jerome Jérôme Mouneyrac added a comment -

              No worries I'll backport the fix of the backport.

              Show
              jerome Jérôme Mouneyrac added a comment - No worries I'll backport the fix of the backport.
              Hide
              jerome Jérôme Mouneyrac added a comment -

              I added a commit for the string fix to 2.2 and HEAD

              Show
              jerome Jérôme Mouneyrac added a comment - I added a commit for the string fix to 2.2 and HEAD
              jerome Jérôme Mouneyrac made changes -
              Status Reopened [ 4 ] Waiting for integration review [ 10010 ]
              nebgor Aparup Banerjee made changes -
              Status Waiting for integration review [ 10010 ] Integration review in progress [ 10004 ]
              Hide
              nebgor Aparup Banerjee added a comment -

              Thanks this all has been integrated (patch into 2.1 and picked fixes into master/2.2)

              had some git madness (empty picks) there for a sec , i gotta upgrade my git version to get more verbosity.

              ciao!

              Show
              nebgor Aparup Banerjee added a comment - Thanks this all has been integrated (patch into 2.1 and picked fixes into master/2.2) had some git madness (empty picks) there for a sec , i gotta upgrade my git version to get more verbosity. ciao!
              nebgor Aparup Banerjee made changes -
              Status Integration review in progress [ 10004 ] Waiting for testing [ 10005 ]
              Affects Version/s 2.2 [ 10656 ]
              Affects Version/s 2.3 [ 10657 ]
              Fix Version/s 2.1.4 [ 11452 ]
              Fix Version/s 2.2 [ 10656 ]
              Fix Version/s DEV backlog [ 10464 ]
              stronk7 Eloy Lafuente (stronk7) made changes -
              Status Waiting for testing [ 10005 ] Testing in progress [ 10011 ]
              Tester stronk7
              Hide
              stronk7 Eloy Lafuente (stronk7) added a comment -

              Tested and passed...

              But when running the PHP-HTTP-filehandling against 21_STABLE I detected that the download option is missing in that branch, so upload worked ok but download did not.

              Up to you if that needs something extra or is ok for 21_STABLE (I guess it's ok, but just to be 100% sure).

              Ciao

              Show
              stronk7 Eloy Lafuente (stronk7) added a comment - Tested and passed... But when running the PHP-HTTP-filehandling against 21_STABLE I detected that the download option is missing in that branch, so upload worked ok but download did not. Up to you if that needs something extra or is ok for 21_STABLE (I guess it's ok, but just to be 100% sure). Ciao
              stronk7 Eloy Lafuente (stronk7) made changes -
              Status Testing in progress [ 10011 ] Tested [ 10006 ]
              Hide
              stronk7 Eloy Lafuente (stronk7) added a comment -

              Yes, you did it!

              Now your code is part of the best weeklies released ever, many thanks!

              Closing, ciao

              Show
              stronk7 Eloy Lafuente (stronk7) added a comment - Yes, you did it! Now your code is part of the best weeklies released ever, many thanks! Closing, ciao
              stronk7 Eloy Lafuente (stronk7) made changes -
              Status Tested [ 10006 ] Closed [ 6 ]
              Resolution Fixed [ 1 ]
              Currently in integration Yes [ 10041 ]
              Integration date 09/Dec/11
              Hide
              jerome Jérôme Mouneyrac added a comment -

              "Web service" download solution is not supported in 2.1. It's a new feature specially created for promoting 2.2

              Show
              jerome Jérôme Mouneyrac added a comment - "Web service" download solution is not supported in 2.1. It's a new feature specially created for promoting 2.2

                People

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

                  Dates

                  • Created:
                    Updated:
                    Resolved:
                    Fix Release Date:
                    5/Dec/11