Details

    • Type: Sub-task
    • Status: Closed
    • Priority: Minor
    • Resolution: Fixed
    • Affects Version/s: 2.1.4, 2.2.1, 2.3
    • Fix Version/s: 2.1.5, 2.2.2
    • Component/s: Authentication
    • Labels:
    • Testing Instructions:
      Hide
      1. Login as Admin and add a user by visiting site admin>users>add user
      2. Goto site admin>reports>live reports> live report from past hour
      3. Make sure you see a "user add" row and make sure the links points to the profile of correct user.
      4. Now as admin update an user's profile and visit the report page again.
      5. Make sure you see a "update user" row pointing to correct user profile.
      Show
      Login as Admin and add a user by visiting site admin>users>add user Goto site admin>reports>live reports> live report from past hour Make sure you see a "user add" row and make sure the links points to the profile of correct user. Now as admin update an user's profile and visit the report page again. Make sure you see a "update user" row pointing to correct user profile.
    • Affected Branches:
      MOODLE_21_STABLE, MOODLE_22_STABLE, MOODLE_23_STABLE
    • Fixed Branches:
      MOODLE_21_STABLE, MOODLE_22_STABLE
    • Pull Master Branch:
      MDL-28386-master

      Description

      When you add a new user using the /user/editadvanced.php page the log entry says the action was "update" not "add" and the url it includes links to a fatal error because the action is logged before the user is created and therefor before the user id is known (which creates a link like "view.php?id=-1&course=1")

        Gliffy Diagrams

          Issue Links

            Activity

            evanirvingpease Evan Irving-Pease created issue -
            evanirvingpease Evan Irving-Pease made changes -
            Field Original Value New Value
            Attachment MDL-28386.patch [ 24553 ]
            skodak Petr Skoda made changes -
            Parent MDL-28443 [ 46008 ]
            Issue Type Bug [ 1 ] Sub-task [ 5 ]
            ankit_frenz Ankit Agarwal made changes -
            Fix Version/s STABLE Sprint 18 [ 11650 ]
            Assignee moodle.com [ moodle.com ] Ankit Agarwal [ ankit_frenz ]
            ankit_frenz Ankit Agarwal made changes -
            Pull Master Diff URL https://github.com/ankitagarwal/moodle/compare/MDL-28386-master
            Pull Master Branch MDL-28386-master
            Fix Version/s STABLE Sprint 17 [ 11550 ]
            Fix Version/s STABLE Sprint 18 [ 11650 ]
            Pull from Repository git://github.com/ankitagarwal/moodle.git
            ankit_frenz Ankit Agarwal made changes -
            Testing Instructions Manually add a new user, visit the live logs report and click on the newly created "user update" link. # Login as Admin and add a user by visiting site admin>users>add user
            # Goto site admin>reports>live reports> live report from past hour
            # Make sure you see a "user add" row and make sure the links points to the profile of correct user.
            # Now as admin update an user's profile and visit the report page again.
            # Make sure you see a "update user" row pointing to correct user profile.
            rajeshtaneja Rajesh Taneja made changes -
            Status Open [ 1 ] Peer review in progress [ 10013 ]
            Peer reviewer rajeshtaneja
            Hide
            rajeshtaneja Rajesh Taneja added a comment -

            Patch looks Good Ankit,
            Although, you should add log entry after $DB->update_record (in else part)

            In addition to this, I think action string should be more elaborative for "new user" then just "add".

            Added Helen, for her feedback, as she can advice better.

            Show
            rajeshtaneja Rajesh Taneja added a comment - Patch looks Good Ankit, Although, you should add log entry after $DB->update_record (in else part) In addition to this, I think action string should be more elaborative for "new user" then just "add". Added Helen, for her feedback, as she can advice better.
            rajeshtaneja Rajesh Taneja made changes -
            Status Peer review in progress [ 10013 ] Development in progress [ 3 ]
            Hide
            ankit_frenz Ankit Agarwal added a comment -

            Hi Rajesh,
            Thanks for the review.
            I have moved the entry after update_Record()

            About the string, since we don't add phrases to the log entry the best I can think of beside "add" is "create". I will wait for Helen's feedback on it.
            Thanks

            Show
            ankit_frenz Ankit Agarwal added a comment - Hi Rajesh, Thanks for the review. I have moved the entry after update_Record() About the string, since we don't add phrases to the log entry the best I can think of beside "add" is "create". I will wait for Helen's feedback on it. Thanks
            tsala Helen Foster made changes -
            Labels patch
            Hide
            tsala Helen Foster added a comment -

            Hi Rajesh and Ankit,

            Thanks for asking my opinion

            Looking at how other new things are reported in the logs e.g. creating a new forum or glossary is reported as 'forum add' and 'glossary add', I think that for adding a new user it should say 'user add'. This is also consistent with the admin UI where it says 'Add a new user' (not 'Create a new user').

            Show
            tsala Helen Foster added a comment - Hi Rajesh and Ankit, Thanks for asking my opinion Looking at how other new things are reported in the logs e.g. creating a new forum or glossary is reported as 'forum add' and 'glossary add', I think that for adding a new user it should say 'user add'. This is also consistent with the admin UI where it says 'Add a new user' (not 'Create a new user').
            Hide
            rajeshtaneja Rajesh Taneja added a comment -

            Thanks Helen,

            'User add' seems goods.

            Show
            rajeshtaneja Rajesh Taneja added a comment - Thanks Helen, 'User add' seems goods.
            Hide
            ankit_frenz Ankit Agarwal added a comment -

            Thanks Helen for the feedback!
            up for integration now
            Thanks

            Show
            ankit_frenz Ankit Agarwal added a comment - Thanks Helen for the feedback! up for integration now Thanks
            ankit_frenz Ankit Agarwal made changes -
            Status Development in progress [ 3 ] Waiting for integration review [ 10010 ]
            ankit_frenz Ankit Agarwal made changes -
            Link This issue is duplicated by MDL-20733 [ MDL-20733 ]
            Hide
            stronk7 Eloy Lafuente (stronk7) 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
            stronk7 Eloy Lafuente (stronk7) 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
            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 -

            Thanks, this looks perfect for coming integration cycle.
            To be Integrated

            Show
            nebgor Aparup Banerjee added a comment - Thanks, this looks perfect for coming integration cycle. To be Integrated
            Hide
            nebgor Aparup Banerjee added a comment -

            integrated and up for testing.

            Show
            nebgor Aparup Banerjee added a comment - integrated and up for testing.
            nebgor Aparup Banerjee made changes -
            Status Integration review in progress [ 10004 ] Waiting for testing [ 10005 ]
            Affects Version/s 2.2.1 [ 11456 ]
            Affects Version/s 2.1.4 [ 11452 ]
            Affects Version/s 2.3 [ 10657 ]
            Affects Version/s 2.1 [ 10370 ]
            Fix Version/s 2.1.5 [ 11553 ]
            Fix Version/s 2.2.2 [ 11552 ]
            salvetore Michael de Raadt made changes -
            Tester rwijaya
            rwijaya Rossiani Wijaya made changes -
            Status Waiting for testing [ 10005 ] Testing in progress [ 10011 ]
            Hide
            rwijaya Rossiani Wijaya added a comment -

            This is looking great.

            Test passed.

            Show
            rwijaya Rossiani Wijaya added a comment - This is looking great. Test passed.
            rwijaya Rossiani Wijaya made changes -
            Status Testing in progress [ 10011 ] Tested [ 10006 ]
            Hide
            stronk7 Eloy Lafuente (stronk7) added a comment -

            Well,

            I wish I said it every time
            you do the things you do.
            You always lend a helping hand,
            and I'm filled with gratitude.

            You are strong and generous
            for each and everyone one of us.
            I am eternally grateful,
            I cannot say thanks enough.

            Sorry for the (un)cool bit above, lol. Closing this as fixed. Ciao

            Show
            stronk7 Eloy Lafuente (stronk7) added a comment - Well, I wish I said it every time you do the things you do. You always lend a helping hand, and I'm filled with gratitude. You are strong and generous for each and everyone one of us. I am eternally grateful, I cannot say thanks enough. Sorry for the (un)cool bit above, lol. Closing this as fixed. Ciao
            stronk7 Eloy Lafuente (stronk7) made changes -
            Status Tested [ 10006 ] Closed [ 6 ]
            Resolution Fixed [ 1 ]
            Currently in integration Yes [ 10041 ]
            Integration date 01/Mar/12
            stronk7 Eloy Lafuente (stronk7) made changes -
            Fix Version/s STABLE Sprint 17 [ 11550 ]

              People

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

                Dates

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