Moodle
  1. Moodle
  2. MDL-18518

User profile: Add member since field to profile

    Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Trivial Trivial
    • Resolution: Fixed
    • Affects Version/s: 1.9.4
    • Fix Version/s: 1.9.5
    • Component/s: Administration
    • Labels:
      None
    • Affected Branches:
      MOODLE_19_STABLE
    • Fixed Branches:
      MOODLE_19_STABLE
    • Rank:
      36465

      Description

      Sometimes it is helpful to know how long someone has been registered. We currently do not save when the user account was created. I was thinking of using first access; however, it appears that sometimes that data does not necessarily get populated. Where I would find this helpful is in reviewing CVS write applications it would give me an idea of how long someone has been around in the community. With the logs not being stored beyond a couple of weeks it is hard to see if someone is a quiet person that has been around for a while or a new person. I generally assume new and can use the user id as an estimate but figured it might not be a bad idea to suggest having a 'Member since' type field that would be set when the user account is created. Peace - Anthony

      1. firstaccess.diff
        0.9 kB
        Anthony Borrow
      2. MDLSITE-615.diff
        2 kB
        Anthony Borrow
      3. MDLSITE-615.diff
        2 kB
        Anthony Borrow
      4. MDLSITE-615-complete.diff
        5 kB
        Anthony Borrow

        Issue Links

          Activity

          Hide
          Helen Foster added a comment -

          Anthony, thanks for your suggestion - sounds good to me Reassigning to Eloy for further consideration.

          Show
          Helen Foster added a comment - Anthony, thanks for your suggestion - sounds good to me Reassigning to Eloy for further consideration.
          Hide
          Anthony Borrow added a comment -

          Thanks Helen. The more I think about it the more I wonder if this might not best be a feature request for CORE. I'm not sure if we have any actual data that would make it meaningful on moodle.org but we will see what Eloy thinks. Peace - Anthony

          Show
          Anthony Borrow added a comment - Thanks Helen. The more I think about it the more I wonder if this might not best be a feature request for CORE. I'm not sure if we have any actual data that would make it meaningful on moodle.org but we will see what Eloy thinks. Peace - Anthony
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Hi,

          I think user->firstaccess uses to have that info (time when signup form was fulfilled... or equivalent in other authentication methods).

          Adding that to profile page seems easy to do. Vote please. Ciao

          Show
          Eloy Lafuente (stronk7) added a comment - Hi, I think user->firstaccess uses to have that info (time when signup form was fulfilled... or equivalent in other authentication methods). Adding that to profile page seems easy to do. Vote please. Ciao
          Hide
          Anthony Borrow added a comment -

          Eloy - On my test site I noticed that many of the firstaccess fields were 0. The code to set the first access is dependent not so much on logging on as it was on user_confirm. When an account was created manually and no confirmation was required it is possible that firstaccess would never be set. Attached is a quick check during login that if it is in fact their firstaccess or firstaccess is empty then it will set it. Let me know if you want me to file this as a separate MDL issue. Peace - Anthony

          Show
          Anthony Borrow added a comment - Eloy - On my test site I noticed that many of the firstaccess fields were 0. The code to set the first access is dependent not so much on logging on as it was on user_confirm. When an account was created manually and no confirmation was required it is possible that firstaccess would never be set. Attached is a quick check during login that if it is in fact their firstaccess or firstaccess is empty then it will set it. Let me know if you want me to file this as a separate MDL issue. Peace - Anthony
          Hide
          Anthony Borrow added a comment -

          Eloy - Here is a patch to show first access in /user/view.php. Note that it creates a new string in /lang/en_utf8/moodle.php (alternatively we could use the firstaccess string in /lang/en_utf8/filters.php). Peace - Anthony

          Show
          Anthony Borrow added a comment - Eloy - Here is a patch to show first access in /user/view.php. Note that it creates a new string in /lang/en_utf8/moodle.php (alternatively we could use the firstaccess string in /lang/en_utf8/filters.php). Peace - Anthony
          Hide
          Eloy Lafuente (stronk7) added a comment -

          I like the idea of having that info available. And the fix in login seems correct. My only "but" is about sites wanting to have that info hidden... should we control it based in some config setting? We have the "hiddenuserfields" config field... should we enable firstaccess there?

          Comments?

          Show
          Eloy Lafuente (stronk7) added a comment - I like the idea of having that info available. And the fix in login seems correct. My only "but" is about sites wanting to have that info hidden... should we control it based in some config setting? We have the "hiddenuserfields" config field... should we enable firstaccess there? Comments?
          Hide
          Anthony Borrow added a comment -

          Thanks Eloy - Yes, I was thinking of checking for the hidden field, and just forgot to add it to the admin settings page. Here is a revised diff that adds the ability to check if firstaccess is a hidden field. Initially I was just thinking of this for Moodle.org; however, I see no reason for it not to go in core. Peace - Anthony

          Show
          Anthony Borrow added a comment - Thanks Eloy - Yes, I was thinking of checking for the hidden field, and just forgot to add it to the admin settings page. Here is a revised diff that adds the ability to check if firstaccess is a hidden field. Initially I was just thinking of this for Moodle.org; however, I see no reason for it not to go in core. Peace - Anthony
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Hi Anthony,

          some minor comments about the patch/es:

          1) In moodlelib.php, when setting the firstaccess field if empty:

          a) I'd execute the update by id ($user->id)
          b) Would be a better option to use user->timemodified as value to put instead of time() ?

          2) Changed behaviour: Once applied the behaviour changes, with "firstaccess" being showed by default. And that could be sensible in some servers. Perhaps it would be better to add one upgrade step in order to add the "firstaccess" field to $CFG->hiddenuserfields always, so current behaviour will be respected in ALL existing servers (while it will be showed in NEW servers).

          Show
          Eloy Lafuente (stronk7) added a comment - Hi Anthony, some minor comments about the patch/es: 1) In moodlelib.php, when setting the firstaccess field if empty: a) I'd execute the update by id ($user->id) b) Would be a better option to use user->timemodified as value to put instead of time() ? 2) Changed behaviour: Once applied the behaviour changes, with "firstaccess" being showed by default. And that could be sensible in some servers. Perhaps it would be better to add one upgrade step in order to add the "firstaccess" field to $CFG->hiddenuserfields always, so current behaviour will be respected in ALL existing servers (while it will be showed in NEW servers).
          Hide
          Anthony Borrow added a comment -

          Eloy,

          I agree with 1a - $user->id seems to be the better way to go. I was just trying to be consistent with MD's set_field above. I think we should probably change them both.

          What would be the advantage to $user->timemodified instead of time? Would $user->timemodified be the time that the user was created? My thinking was that the most accurate time of firstaccess is that moment when they first login (i.e. successfully authenticate). I think time might be more accurate for what firstaccess is supposed to be but you may have a different perspective.

          As for the upgrade step, could you help me understand that a little better. It would have to be based on version since in mdl_config the hiddenuserfields entry just contains a list of the hidden fields and that may or may not already be set so there would be no way of determining that. If you could just point me in the right direction I'll work on it and update the patch.

          Peace - Anthony

          Show
          Anthony Borrow added a comment - Eloy, I agree with 1a - $user->id seems to be the better way to go. I was just trying to be consistent with MD's set_field above. I think we should probably change them both. What would be the advantage to $user->timemodified instead of time? Would $user->timemodified be the time that the user was created? My thinking was that the most accurate time of firstaccess is that moment when they first login (i.e. successfully authenticate). I think time might be more accurate for what firstaccess is supposed to be but you may have a different perspective. As for the upgrade step, could you help me understand that a little better. It would have to be based on version since in mdl_config the hiddenuserfields entry just contains a list of the hidden fields and that may or may not already be set so there would be no way of determining that. If you could just point me in the right direction I'll work on it and update the patch. Peace - Anthony
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Hi Anthony,

          the point about to fill $user->firstaccess with $user->timemodified instead of time() is to get some older time, different than now. Perhaps we could look logs instead of something else. I was, simply, trying to get some date in the past to avoid that field being filled with current time (to avoid users claiming about that). Not important, as far as 99% of users will have it properly filled, just trying to get an older time than time().

          About the upgrade... I mean we should respect current behaviour... and current behaviour is that the firstaccess information isn't showed right now. So, after upgrade... that behaviour should remain the same. To do that we need to:

          1) increase main version.php in a small increment.
          2) perform one "upgrade block" that will do this:
          a) if $CFG->hiddenuserfields is empty set it to "firstaccess"
          b) if CFG->hiddenuserfields is not empty, add "firstaccess"

          That way, the field will continue being not showed by default (current behaviour) and sites wanting to enable it only will need to un-hide it from admin.

          Does that sound ok?

          Show
          Eloy Lafuente (stronk7) added a comment - Hi Anthony, the point about to fill $user->firstaccess with $user->timemodified instead of time() is to get some older time, different than now. Perhaps we could look logs instead of something else. I was, simply, trying to get some date in the past to avoid that field being filled with current time (to avoid users claiming about that). Not important, as far as 99% of users will have it properly filled, just trying to get an older time than time(). About the upgrade... I mean we should respect current behaviour... and current behaviour is that the firstaccess information isn't showed right now. So, after upgrade... that behaviour should remain the same. To do that we need to: 1) increase main version.php in a small increment. 2) perform one "upgrade block" that will do this: a) if $CFG->hiddenuserfields is empty set it to "firstaccess" b) if CFG->hiddenuserfields is not empty, add "firstaccess" That way, the field will continue being not showed by default (current behaviour) and sites wanting to enable it only will need to un-hide it from admin. Does that sound ok?
          Hide
          Anthony Borrow added a comment -

          Eloy - Yep, sounds OK. Now I see your logic with $user->timemodified and I agree that would be better. I think combing the logs would be more accurate but overkill. I'll work on the upgrade script. I am presuming in the diff that a small increment change would be from 2007101541 to 2007101542? But I saw reference in upgrade.php to micro versions but I have not been keeping up on how those work. Peace - Anthony

          Show
          Anthony Borrow added a comment - Eloy - Yep, sounds OK. Now I see your logic with $user->timemodified and I agree that would be better. I think combing the logs would be more accurate but overkill. I'll work on the upgrade script. I am presuming in the diff that a small increment change would be from 2007101541 to 2007101542? But I saw reference in upgrade.php to micro versions but I have not been keeping up on how those work. Peace - Anthony
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Yup,

          in theory you could use 2007101541.01 or similar... but I think you can safely use 2007101542. Later if we run short of versions... we'll use the micro-incremements, np!

          Ciao

          Show
          Eloy Lafuente (stronk7) added a comment - Yup, in theory you could use 2007101541.01 or similar... but I think you can safely use 2007101542. Later if we run short of versions... we'll use the micro-incremements, np! Ciao
          Hide
          Anthony Borrow added a comment -

          Eloy - OK - here is a diff that combines firstaccess.diff and MDLSITE-615.diff and implements the version bump. Sorry for the delay. I got a call to play tennis and then had a friend arrive from Omaha, NE. Peace - Anthony

          Show
          Anthony Borrow added a comment - Eloy - OK - here is a diff that combines firstaccess.diff and MDLSITE-615 .diff and implements the version bump. Sorry for the delay. I got a call to play tennis and then had a friend arrive from Omaha, NE. Peace - Anthony
          Hide
          Anthony Borrow added a comment -

          One area of the patch that you may want to take a look at, I was la bit azy with the if statement containing the strpos function to check if firstaccess was already part of the string. What I wanted to avoid was if by some chance (not likely) someone already had it listed not having a repeat like 'firstaccess, firstaccess'. Technically since strpos returns a number I could have done something like: if (strpos($CFG->hiddenuserfields,'firstaccess')>0); however, as long as it finds some value the statement should evaluate to true unless the string is not found in which case it will be zero/i.e. false but stylistically more seasoned developers may want that to be more grammatically correct. Peace - Anthony

          Show
          Anthony Borrow added a comment - One area of the patch that you may want to take a look at, I was la bit azy with the if statement containing the strpos function to check if firstaccess was already part of the string. What I wanted to avoid was if by some chance (not likely) someone already had it listed not having a repeat like 'firstaccess, firstaccess'. Technically since strpos returns a number I could have done something like: if (strpos($CFG->hiddenuserfields,'firstaccess')>0); however, as long as it finds some value the statement should evaluate to true unless the string is not found in which case it will be zero/i.e. false but stylistically more seasoned developers may want that to be more grammatically correct. Peace - Anthony
          Hide
          Martin Dougiamas added a comment -

          Sounds good! +1 for the concept! is this ready to try on moodle.org?

          Show
          Martin Dougiamas added a comment - Sounds good! +1 for the concept! is this ready to try on moodle.org?
          Hide
          Eloy Lafuente (stronk7) added a comment -

          I'll review it tomorrow and commit to 19_STABLE, oki?

          Show
          Eloy Lafuente (stronk7) added a comment - I'll review it tomorrow and commit to 19_STABLE, oki?
          Hide
          Martin Dougiamas added a comment -

          Just looked at the patch - seems non-controversial and backward compatible too ... can you merge into 1.9.4+ ?

          Show
          Martin Dougiamas added a comment - Just looked at the patch - seems non-controversial and backward compatible too ... can you merge into 1.9.4+ ?
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Just changed this:

          if (!strpos($CFG->hiddenuserfields, 'firstaccess'))

          to:

          if (strpos($CFG->hiddenuserfields, 'firstaccess') === false)

          And added one missing:

          upgrade_main_savepoint($result, 2007101542);

          in the upgrade script. Going to test in a bit more... and commit in a few minutes. Ciao

          Show
          Eloy Lafuente (stronk7) added a comment - Just changed this: if (!strpos($CFG->hiddenuserfields, 'firstaccess')) to: if (strpos($CFG->hiddenuserfields, 'firstaccess') === false) And added one missing: upgrade_main_savepoint($result, 2007101542); in the upgrade script. Going to test in a bit more... and commit in a few minutes. Ciao
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Oki I've:

          • committed to 19_STABLE
          • updated moodle.org and enabled it there (was disabled by default - correct approach).

          And then:

          • merged to HEAD

          But, in HEAD, I've omitted the upgrade step, because it will cause people coming from 1.9 to have that field hidden again. So current approach in HEAD is:

          • The firstaccess field will be visible on new installs
          • The firstaccess field will be visible if was visible or non-existing in the version we are upgrading from

          Does that sound ok? It's the best way to respect updaters I've found. And I haven't been able to imagine an alternative way to know what was done before (but adding some flag somewhere in DB) but I think this isn't critical enough to do so.

          Thanks Anthony, Martin, ciao

          Show
          Eloy Lafuente (stronk7) added a comment - Oki I've: committed to 19_STABLE updated moodle.org and enabled it there (was disabled by default - correct approach). And then: merged to HEAD But, in HEAD, I've omitted the upgrade step, because it will cause people coming from 1.9 to have that field hidden again. So current approach in HEAD is: The firstaccess field will be visible on new installs The firstaccess field will be visible if was visible or non-existing in the version we are upgrading from Does that sound ok? It's the best way to respect updaters I've found. And I haven't been able to imagine an alternative way to know what was done before (but adding some flag somewhere in DB) but I think this isn't critical enough to do so. Thanks Anthony, Martin, ciao
          Hide
          Helen Foster added a comment -

          First access info on moodle.org is great! Thanks a lot Anthony, Eloy and Martin. I couldn't resist browsing the profiles of some of our Particularly Helpful Moodlers

          Show
          Helen Foster added a comment - First access info on moodle.org is great! Thanks a lot Anthony, Eloy and Martin. I couldn't resist browsing the profiles of some of our Particularly Helpful Moodlers
          Hide
          Anthony Borrow added a comment -

          Eloy - The behavior you outline sounds good to me. Thanks for your work on testing and getting it committed. Peace - Anthony

          Show
          Anthony Borrow added a comment - Eloy - The behavior you outline sounds good to me. Thanks for your work on testing and getting it committed. Peace - Anthony
          Hide
          Petr Škoda added a comment -

          The last step missing is the upgrade in HEAD, right?
          The rest looks ok

          Show
          Petr Škoda added a comment - The last step missing is the upgrade in HEAD, right? The rest looks ok
          Hide
          Anthony Borrow added a comment -

          Petr - You may want to double check with Eloy but as I understood it, in HEAD the version upgrade was not necessary because we were going to have it be shown by default. We just didn't want folks seeing information in 1.9 without having the admin explicitly say it was OK so we added it to the list of hidden fields by default in 1.9 but it will not be hidden by default in HEAD. Does that make sense? Peace -Anthony

          Show
          Anthony Borrow added a comment - Petr - You may want to double check with Eloy but as I understood it, in HEAD the version upgrade was not necessary because we were going to have it be shown by default. We just didn't want folks seeing information in 1.9 without having the admin explicitly say it was OK so we added it to the list of hidden fields by default in 1.9 but it will not be hidden by default in HEAD. Does that make sense? Peace -Anthony
          Hide
          Petr Škoda added a comment -

          hmm, it might be better to include these in security overview report, I suppose this might be considered privacy issue by some people

          Show
          Petr Škoda added a comment - hmm, it might be better to include these in security overview report, I suppose this might be considered privacy issue by some people
          Hide
          Anthony Borrow added a comment -

          Petr - At least informing the system admin that some new information is being revealed by default would not be a bad idea. Peace - Anthony

          Show
          Anthony Borrow added a comment - Petr - At least informing the system admin that some new information is being revealed by default would not be a bad idea. Peace - Anthony
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Well,

          I agree could be considered privacy issue... more or less.. IMO the name, or picture... are far more private, and we don't include anything about them in security report, correct?

          Ciao

          Show
          Eloy Lafuente (stronk7) added a comment - Well, I agree could be considered privacy issue... more or less.. IMO the name, or picture... are far more private, and we don't include anything about them in security report, correct? Ciao
          Hide
          Petr Škoda added a comment -

          ok, can we close this?

          Show
          Petr Škoda added a comment - ok, can we close this?
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Doing that, thanks Petr for QA!

          Show
          Eloy Lafuente (stronk7) added a comment - Doing that, thanks Petr for QA!

            People

            • Votes:
              2 Vote for this issue
              Watchers:
              2 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved: