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

          Attachments

            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