Details

    • Type: Sub-task Sub-task
    • Status: Closed
    • Priority: Minor 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

            Evan Irving-Pease created issue -
            Evan Irving-Pease made changes -
            Field Original Value New Value
            Attachment MDL-28386.patch [ 24553 ]
            Petr Skoda made changes -
            Parent MDL-28443 [ 46008 ]
            Issue Type Bug [ 1 ] Sub-task [ 5 ]
            Ankit Agarwal made changes -
            Fix Version/s STABLE Sprint 18 [ 11650 ]
            Assignee moodle.com [ moodle.com ] Ankit Agarwal [ 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 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.
            Rajesh Taneja made changes -
            Status Open [ 1 ] Peer review in progress [ 10013 ]
            Peer reviewer rajeshtaneja
            Hide
            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
            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.
            Rajesh Taneja made changes -
            Status Peer review in progress [ 10013 ] Development in progress [ 3 ]
            Hide
            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 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
            Helen Foster made changes -
            Labels patch
            Hide
            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
            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
            Rajesh Taneja added a comment -

            Thanks Helen,

            'User add' seems goods.

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

            Thanks Helen for the feedback!
            up for integration now
            Thanks

            Show
            Ankit Agarwal added a comment - Thanks Helen for the feedback! up for integration now Thanks
            Ankit Agarwal made changes -
            Status Development in progress [ 3 ] Waiting for integration review [ 10010 ]
            Ankit Agarwal made changes -
            Link This issue is duplicated by MDL-20733 [ MDL-20733 ]
            Hide
            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
            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
            Aparup Banerjee made changes -
            Status Waiting for integration review [ 10010 ] Integration review in progress [ 10004 ]
            Integrator nebgor
            Currently in integration Yes [ 10041 ]
            Hide
            Aparup Banerjee added a comment -

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

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

            integrated and up for testing.

            Show
            Aparup Banerjee added a comment - integrated and up for testing.
            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 ]
            Michael de Raadt made changes -
            Tester rwijaya
            Rossiani Wijaya made changes -
            Status Waiting for testing [ 10005 ] Testing in progress [ 10011 ]
            Hide
            Rossiani Wijaya added a comment -

            This is looking great.

            Test passed.

            Show
            Rossiani Wijaya added a comment - This is looking great. Test passed.
            Rossiani Wijaya made changes -
            Status Testing in progress [ 10011 ] Tested [ 10006 ]
            Hide
            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
            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
            Eloy Lafuente (stronk7) made changes -
            Status Tested [ 10006 ] Closed [ 6 ]
            Resolution Fixed [ 1 ]
            Currently in integration Yes [ 10041 ]
            Integration date 01/Mar/12
            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: