Moodle
  1. Moodle
  2. MDL-35644

user_get_details: admin should be able to see description

    Details

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

      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.

        Issue Links

          Activity

          Hide
          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
          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
          Jérôme Mouneyrac added a comment -

          I agree too.

          Show
          Jérôme Mouneyrac added a comment - I agree too.
          Hide
          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
          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
          Eloy Lafuente (stronk7) added a comment -

          Integrated (22, 23 and master), thanks!

          Show
          Eloy Lafuente (stronk7) added a comment - Integrated (22, 23 and master), thanks!
          Hide
          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
          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
          Jérôme Mouneyrac added a comment - - edited

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

          Show
          Jérôme Mouneyrac added a comment - - edited Thanks Eloy. I'll add some test instructions for the tester. Update: test instructions added
          Hide
          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
          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
          Eloy Lafuente (stronk7) added a comment -

          Closing as fixed, many thanks for your awesome collaboration.

          Show
          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: