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

user_get_details: admin should be able to see description

    Details

    • Type: Bug
    • Status: Closed
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: 2.2.4, 2.2.5, 2.3.2
    • Fix Version/s: 2.2.6, 2.3.3
    • Component/s: Web Services
    • Labels:
    • Testing Instructions:
      Hide

      Cherry-pick MDL-34971
      phpunit core_user_external_testcase user/tests/externallib_test.php

      For 2.2, tester with web service experience required:
      a) setup the web services (see Moodledocs)
      b) call as admin, core_user_get_users_by_id() check the description is returned.

      Show
      Cherry-pick MDL-34971 phpunit core_user_external_testcase user/tests/externallib_test.php For 2.2, tester with web service experience required: a) setup the web services (see Moodledocs) b) call as admin, core_user_get_users_by_id() check the description is returned.
    • Affected Branches:
      MOODLE_22_STABLE, MOODLE_23_STABLE
    • Fixed Branches:
      MOODLE_22_STABLE, MOODLE_23_STABLE
    • Pull Master Branch:

      Description

      In user/lib.php:get_user_details()

      Currently, sometimes, the admin is not allowed to see the user description. This is wrong as an admin always can see the description. At worst the UI can decide to not display it, but the admin can always see it.

        Gliffy Diagrams

          Attachments

            Issue Links

              Activity

              Hide
              poltawski Dan Poltawski added a comment -

              The change looks ok to me.

              Though personally I do not like the way these logic statements are written, as I find it difficult to understand. To me, it is hard to 'process' such chaining of logic in one statement. And I also find it really hard to understand 'negative' flags, especially when combined with Unable to render embedded object: File (. IF not a not = true) not found.

              So, if I were to rewrite the whole thing, i'd be going for something that removed those aspects of the logic and could be explained in sentences.

              Show
              poltawski Dan Poltawski added a comment - The change looks ok to me. Though personally I do not like the way these logic statements are written, as I find it difficult to understand. To me, it is hard to 'process' such chaining of logic in one statement. And I also find it really hard to understand 'negative' flags, especially when combined with Unable to render embedded object: File (. IF not a not = true) not found. So, if I were to rewrite the whole thing, i'd be going for something that removed those aspects of the logic and could be explained in sentences.
              Hide
              jerome Jérôme Mouneyrac added a comment -

              I agree too.

              Show
              jerome Jérôme Mouneyrac added a comment - I agree too.
              Hide
              poltawski Dan Poltawski 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
              poltawski Dan Poltawski 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 and master), thanks!

              Show
              stronk7 Eloy Lafuente (stronk7) added a comment - Integrated (22, 23 and master), thanks!
              Hide
              stronk7 Eloy Lafuente (stronk7) added a comment - - edited

              Note MDL-34971 has been integrated and test passed. So this can be considered tested for both 23_STABLE and master.

              It would be great to set some testing instructions for 22_STABLE (create user, enable WSs, use some of the testing scripts... check the description arrives).

              (Or alternatively, be able to test the user_get_user_details() )

              Ciao

              Show
              stronk7 Eloy Lafuente (stronk7) added a comment - - edited Note MDL-34971 has been integrated and test passed. So this can be considered tested for both 23_STABLE and master. It would be great to set some testing instructions for 22_STABLE (create user, enable WSs, use some of the testing scripts... check the description arrives). (Or alternatively, be able to test the user_get_user_details() ) Ciao
              Hide
              jerome Jérôme Mouneyrac added a comment - - edited

              Thanks Eloy. I'll add some test instructions for the tester.
              Update: test instructions added

              Show
              jerome Jérôme Mouneyrac added a comment - - edited Thanks Eloy. I'll add some test instructions for the tester. Update: test instructions added
              Hide
              dmonllao David Monllaó added a comment -

              It passes, tested forcing $cannotviewdescription to false. In 22 using the moodle client test and the sample-ws-client project

              Show
              dmonllao David Monllaó added a comment - It passes, tested forcing $cannotviewdescription to false. In 22 using the moodle client test and the sample-ws-client project
              Hide
              stronk7 Eloy Lafuente (stronk7) added a comment -

              Closing as fixed, many thanks for your awesome collaboration.

              Show
              stronk7 Eloy Lafuente (stronk7) added a comment - Closing as fixed, many thanks for your awesome collaboration.

                People

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

                  Dates

                  • Created:
                    Updated:
                    Resolved:
                    Fix Release Date:
                    12/Nov/12