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

          Attachments

            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