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
    • Rank:
      18091

      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")

        Issue Links

          Activity

          Evan Irving-Pease created issue -
          Evan Irving-Pease made changes -
          Field Original Value New Value
          Attachment MDL-28386.patch [ 24553 ]
          Petr Škoda 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: