Details

    • Type: New Feature
    • Status: Closed
    • Priority: Minor
    • Resolution: Fixed
    • Affects Version/s: 2.0
    • Fix Version/s: 2.0.2
    • Component/s: Administration
    • Labels:
      None
    • Difficulty:
      Easy
    • Affected Branches:
      MOODLE_20_STABLE
    • Fixed Branches:
      MOODLE_20_STABLE

      Description

      Similar to $CFG->country - i.e. pre-fill in the add/edit user and sign-up forms with a default city based on a new $CFG->city setting.

      Useful for small school Moodles where the students and teachers all come from the same city.

        Gliffy Diagrams

          Issue Links

            Activity

            Show
            jonathan Jonathan Harker added a comment - Git branch here: http://git.catalyst.net.nz/gw?p=moodle-r2.git;a=shortlog;h=refs/heads/MDL-25778
            Hide
            samhemelryk Sam Hemelryk added a comment -

            Hi Jonathan,

            This gets my vote!

            The only thing I don't really like about it is the setting name city, I think it would be clearer to call it defaultcity, or perhaps defaultusercity.
            But then there is already a default country settings called country so I suppose just calling it city is like sticking to the standard

            Cheer
            Sam

            Show
            samhemelryk Sam Hemelryk added a comment - Hi Jonathan, This gets my vote! The only thing I don't really like about it is the setting name city , I think it would be clearer to call it defaultcity, or perhaps defaultusercity. But then there is already a default country settings called country so I suppose just calling it city is like sticking to the standard Cheer Sam
            Hide
            jonathan Jonathan Harker added a comment -

            I agree - I was tempted to call it defaultcity - no harm in doing so I guess.

            Show
            jonathan Jonathan Harker added a comment - I agree - I was tempted to call it defaultcity - no harm in doing so I guess.
            Hide
            mudrd8mz David Mudrak added a comment -

            Would it make sense to come with a robust solution to provide default values for other profile settings, too? If so, which fields?

            Show
            mudrd8mz David Mudrak added a comment - Would it make sense to come with a robust solution to provide default values for other profile settings, too? If so, which fields?
            Hide
            jonathan Jonathan Harker added a comment -

            Perhaps, but this was mainly to cover the two "outlier" fields that MUST be filled in (country and city) other than the usual name, email, and password.

            Show
            jonathan Jonathan Harker added a comment - Perhaps, but this was mainly to cover the two "outlier" fields that MUST be filled in (country and city) other than the usual name, email, and password.
            Hide
            jonathan Jonathan Harker added a comment - - edited

            Otherwise I suppose we would have a set of key value pairs, where $key is the form element name. If ! empty($value) we could then $form->setDefault($key, $value) in the form definition. Then we'd need to have some way for the user to control the values, probably a "Default User Settings" admin page, with elements for each field; I doubt we could auto-generate the admin page, so we'd have to set the elements up manually. Either way we can then easily cobble a simple dictionary class/assoc array from the default user settings to pass around for use in the form definitions. This would be useful elsewhere too, such as the authentication and enrolment plug-ins that auto-generate users as needed. Wish I had time!

            Show
            jonathan Jonathan Harker added a comment - - edited Otherwise I suppose we would have a set of key value pairs, where $key is the form element name. If ! empty($value) we could then $form->setDefault($key, $value) in the form definition. Then we'd need to have some way for the user to control the values, probably a "Default User Settings" admin page, with elements for each field; I doubt we could auto-generate the admin page, so we'd have to set the elements up manually. Either way we can then easily cobble a simple dictionary class/assoc array from the default user settings to pass around for use in the form definitions. This would be useful elsewhere too, such as the authentication and enrolment plug-ins that auto-generate users as needed. Wish I had time!
            Hide
            jonathan Jonathan Harker added a comment -

            Meanwhile, how do we get this into master?

            Show
            jonathan Jonathan Harker added a comment - Meanwhile, how do we get this into master?
            Show
            jonathan Jonathan Harker added a comment - Updated branch: http://git.catalyst.net.nz/gw?p=moodle-r2.git;a=shortlog;h=refs/heads/MDL-25778_20110209
            Hide
            skodak Petr Skoda added a comment -

            I have submitted a bit improved PULL request, thanks a lot!

            Show
            skodak Petr Skoda added a comment - I have submitted a bit improved PULL request, thanks a lot!
            Hide
            stronk7 Eloy Lafuente (stronk7) added a comment -

            From PULL-313:

            Side comment: Just guessing why there aren't any "$CFG->country" occurrences in:

            • admin/uploaduser.php
            • auth/db/auth.php
            • auth/ldap/auth.php

            Does that mean that those methods are not observing the default country? Should we add that?

            Ciao

            Show
            stronk7 Eloy Lafuente (stronk7) added a comment - From PULL-313: Side comment: Just guessing why there aren't any "$CFG->country" occurrences in: admin/uploaduser.php auth/db/auth.php auth/ldap/auth.php Does that mean that those methods are not observing the default country? Should we add that? Ciao
            Hide
            stronk7 Eloy Lafuente (stronk7) added a comment -

            I've created MDL-26409 about other "defaults" (country...) potentially not being applied in all the user creation options.

            Ciao

            Show
            stronk7 Eloy Lafuente (stronk7) added a comment - I've created MDL-26409 about other "defaults" (country...) potentially not being applied in all the user creation options. Ciao
            Hide
            jonathan Jonathan Harker added a comment -

            I don't mean to cause too much of a stir but
            1. I can't comment on either PULL request,
            2. having $CFG->defaultcity instead of $CFG->city is inconsistent with the existing $CFG->country setting. If we're going to have $CFG->defaultcity then we must change $CFG->country to $CFG->defaultcountry as well, which is why I went with $CFG->city. Such a change would be reasonably disruptive and should be done on a separate issue number, and
            3. Petr's proposed commit on PULL-313 does not retain my authorship.

            Show
            jonathan Jonathan Harker added a comment - I don't mean to cause too much of a stir but 1. I can't comment on either PULL request, 2. having $CFG->defaultcity instead of $CFG->city is inconsistent with the existing $CFG->country setting. If we're going to have $CFG->defaultcity then we must change $CFG->country to $CFG->defaultcountry as well, which is why I went with $CFG->city. Such a change would be reasonably disruptive and should be done on a separate issue number, and 3. Petr's proposed commit on PULL-313 does not retain my authorship.
            Hide
            stronk7 Eloy Lafuente (stronk7) added a comment -

            Hi Jonathan,

            about commenting on PULL request... I think the idea was to keep discussion active here (in the MDL) and use the PULL exclusively for comments related to the integration & testing phase. Mainly because if we mix comments in both places, surely some of them will be forgotten.

            about the name (defaultcity), I remember it was discussed when Petr was performing modifications to your original code and we agreed "default" was better for the future, with others like country and friends be renamed. But we didn't want to perform that change for existing ones in the middle of stable. Yes, it causes somehow a inconsistency, but aimed to be cleaned in the future with all them being "defaultxxxx"

            about authorship, I cannot tell much, I don't know which git magic was used nor the differences between your original changes and the final ones, "git blame Petr".

            Thanks for your support! Ciao

            Show
            stronk7 Eloy Lafuente (stronk7) added a comment - Hi Jonathan, about commenting on PULL request... I think the idea was to keep discussion active here (in the MDL) and use the PULL exclusively for comments related to the integration & testing phase. Mainly because if we mix comments in both places, surely some of them will be forgotten. about the name (defaultcity), I remember it was discussed when Petr was performing modifications to your original code and we agreed "default" was better for the future, with others like country and friends be renamed. But we didn't want to perform that change for existing ones in the middle of stable. Yes, it causes somehow a inconsistency, but aimed to be cleaned in the future with all them being "defaultxxxx" about authorship, I cannot tell much, I don't know which git magic was used nor the differences between your original changes and the final ones, "git blame Petr". Thanks for your support! Ciao
            Hide
            skodak Petr Skoda added a comment -

            Opps, sorry I did not give you the 'git author' credit for the string 'A city entered here will be the default city when creating new user accounts.', I thought the info in commit message would be enough. I will use my own strings next time, sorry. I have written the rest of the code myself - I just copy-pasted country code and added fixes for upload user and originally for auth plugins too, that is why I used myself as author and only gave you credit for the idea in the commit message.

            Normally I would not file a PULL request myself, but it was the only chance to get this issue resolved in 2.0.2. I have discussed the defaultcity name with David and Eloy, we all agree that country was a bad string and setting name.

            In any case I have added you to the list of ppl that are allowed to create PULL requests, please tell other developers to ping me if necessary. Next time this happens your PULL requests will be simply rejected and you will have time to rework or fix them so that you get all the due credit.

            I am going to bring up the topic of authorship and handling of git contributions, it would be great if there were some solid guidelines for people doing the code reviews and integrations, it used to be a lot easier in the old CVS days, we are still learning the new and better git way....

            Thanks a lot for your contribution and notifying us about this problem, I am looking forward to more PULL requests from you soon!

            Petr

            Show
            skodak Petr Skoda added a comment - Opps, sorry I did not give you the 'git author' credit for the string 'A city entered here will be the default city when creating new user accounts.', I thought the info in commit message would be enough. I will use my own strings next time, sorry. I have written the rest of the code myself - I just copy-pasted country code and added fixes for upload user and originally for auth plugins too, that is why I used myself as author and only gave you credit for the idea in the commit message. Normally I would not file a PULL request myself, but it was the only chance to get this issue resolved in 2.0.2. I have discussed the defaultcity name with David and Eloy, we all agree that country was a bad string and setting name. In any case I have added you to the list of ppl that are allowed to create PULL requests, please tell other developers to ping me if necessary. Next time this happens your PULL requests will be simply rejected and you will have time to rework or fix them so that you get all the due credit. I am going to bring up the topic of authorship and handling of git contributions, it would be great if there were some solid guidelines for people doing the code reviews and integrations, it used to be a lot easier in the old CVS days, we are still learning the new and better git way.... Thanks a lot for your contribution and notifying us about this problem, I am looking forward to more PULL requests from you soon! Petr
            Hide
            jonathan Jonathan Harker added a comment -

            Sorry Petr I did not see the extra line in the commit message

            Show
            jonathan Jonathan Harker added a comment - Sorry Petr I did not see the extra line in the commit message

              People

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

                Dates

                • Created:
                  Updated:
                  Resolved:
                  Fix Release Date:
                  21/Feb/11