Details

    • Type: Sub-task
    • Status: Closed
    • Priority: Minor
    • Resolution: Fixed
    • Affects Version/s: 2.1
    • Fix Version/s: 2.1.5, 2.2.2
    • Component/s: Administration
    • Labels:
    • Testing Instructions:
      Hide
      1. Login as admin
      2. goto user list and delete a user
      3. Goto settings>reports>live log
      4. Make sure you can see the action of user deletion in the logs and make sure all data is correct.
      5. Click the button that says "test passed" on the top right of this page
      Show
      Login as admin goto user list and delete a user Goto settings>reports>live log Make sure you can see the action of user deletion in the logs and make sure all data is correct. Click the button that says "test passed" on the top right of this page
    • Affected Branches:
      MOODLE_21_STABLE
    • Fixed Branches:
      MOODLE_21_STABLE, MOODLE_22_STABLE
    • Pull Master Branch:
      MDL-28387-master

      Description

      When you delete a user from the 'Browse list of users' page no log entry is made.

        Gliffy Diagrams

        1. MDL-28387.patch
          0.5 kB
          Evan Irving-Pease

          Activity

          Hide
          ankit_frenz Ankit Agarwal added a comment -

          Hi,
          I have modifed the patch a bit.
          Also I think it would be better to log directly inside the delete user api rather than embeding it somewhere in the work flow, since there are many places that call delete user api (example: spam cleaner)

          And finally I have directly used the name variables instead of calling "fullname" function that should reduce the DB call a bit, and at other places (user view) , we don't check for any caps (ex:- viewfullname?). Any other opinion appreciated.

          Submitting for review!
          Thanks

          Show
          ankit_frenz Ankit Agarwal added a comment - Hi, I have modifed the patch a bit. Also I think it would be better to log directly inside the delete user api rather than embeding it somewhere in the work flow, since there are many places that call delete user api (example: spam cleaner) And finally I have directly used the name variables instead of calling "fullname" function that should reduce the DB call a bit, and at other places (user view) , we don't check for any caps (ex:- viewfullname?). Any other opinion appreciated. Submitting for review! Thanks
          Hide
          poltawski Dan Poltawski added a comment -

          As me and Ankit discussed:

          • We discussed if was probably preferable to use fullname() to display the name - however this will vary the entry added to the log depending on the user who deleted the users settings.
          • I suggested separating out the alphabetical ordering for reability
          Show
          poltawski Dan Poltawski added a comment - As me and Ankit discussed: We discussed if was probably preferable to use fullname() to display the name - however this will vary the entry added to the log depending on the user who deleted the users settings. I suggested separating out the alphabetical ordering for reability
          Hide
          ankit_frenz Ankit Agarwal added a comment -

          Hi Dan,
          I have made changes as we discussed. and the readability commit is pushed to master only.
          Thanks

          Show
          ankit_frenz Ankit Agarwal added a comment - Hi Dan, I have made changes as we discussed. and the readability commit is pushed to master only. Thanks
          Hide
          samhemelryk Sam Hemelryk added a comment -

          Hi Ankit,

          Why have you moved the user view entry in the log.php file?
          Were you meaning to duplicate it and change it to delete?

          Reviewing paused pending your reply

          Cheers
          Sam

          Show
          samhemelryk Sam Hemelryk added a comment - Hi Ankit, Why have you moved the user view entry in the log.php file? Were you meaning to duplicate it and change it to delete? Reviewing paused pending your reply Cheers Sam
          Hide
          samhemelryk Sam Hemelryk added a comment -

          (forgot to mention only in master)

          Show
          samhemelryk Sam Hemelryk added a comment - (forgot to mention only in master)
          Hide
          samhemelryk Sam Hemelryk added a comment -

          Thanks Ankit this has been integrated now

          Show
          samhemelryk Sam Hemelryk added a comment - Thanks Ankit this has been integrated now
          Hide
          salvetore Michael de Raadt added a comment - - edited

          Test result: Success in 2.1, 2.2 and master. I'm glad you described how to pass this test

          Show
          salvetore Michael de Raadt added a comment - - edited Test result: Success in 2.1, 2.2 and master. I'm glad you described how to pass this test
          Hide
          stronk7 Eloy Lafuente (stronk7) added a comment -

          Your changes are now upstream and will be included in the next minor released scheduled for March 13th (next Monday!).

          icao_reverse('arreis olik rebemevon afla letoh ognat');

          Closing, ciao

          Show
          stronk7 Eloy Lafuente (stronk7) added a comment - Your changes are now upstream and will be included in the next minor released scheduled for March 13th (next Monday!). icao_reverse('arreis olik rebemevon afla letoh ognat'); Closing, ciao

            People

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

              Dates

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