Uploaded image for project: 'Moodle'
  1. Moodle
  2. MDL-27931

In bulk user upload, city is required but not marked as required

    Details

    • Type: Bug
    • Status: Closed
    • Priority: Minor
    • Resolution: Fixed
    • Affects Version/s: 1.9.13, 2.0.3, 2.1, 2.2
    • Fix Version/s: 2.0.4, 2.1.1
    • Component/s: Administration
    • Labels:
    • Environment:
      Any
    • Database:
      Any
    • Testing Instructions:
      Hide

      1. login as admin
      2. from side blocks, site admin > users > accounts > upload users
      3. upload user cvs file (file is attached)
      4. make sure city field is empty

      required error should appear on city field.

      Show
      1. login as admin 2. from side blocks, site admin > users > accounts > upload users 3. upload user cvs file (file is attached) 4. make sure city field is empty required error should appear on city field.
    • Difficulty:
      Easy
    • Affected Branches:
      MOODLE_19_STABLE, MOODLE_20_STABLE, MOODLE_21_STABLE, MOODLE_22_STABLE
    • Fixed Branches:
      MOODLE_20_STABLE, MOODLE_21_STABLE
    • Pull from Repository:
    • Pull Master Branch:

      Description

      In uploaduser_form.php the city field is required but not marked as such in the form. It's possible, though unlikely, to run across a use case where there isn't a default value that gets filled in for city, and in any case marking it as required is just best practice.

      Fix for 2.1: https://github.com/mackensen/moodle/commit/116fc1a0f2ba9326500ad8ad42d8548e72f80079

        Gliffy Diagrams

        1. users.cvs
          0.1 kB
          Rossiani Wijaya

          Issue Links

            Activity

            cfulton Charles Fulton created issue -
            cfulton Charles Fulton made changes -
            Field Original Value New Value
            Description In uploaduser_form.php the city field is required but not marked as such in the form. It's possible, though unlikely, to run across a use case where there isn't a default value that gets filled in for city, and in any case marking it as required is just best practice. In uploaduser_form.php the city field is required but not marked as such in the form. It's possible, though unlikely, to run across a use case where there isn't a default value that gets filled in for city, and in any case marking it as required is just best practice.

            Fix for 2.1: https://github.com/mackensen/moodle/commit/116fc1a0f2ba9326500ad8ad42d8548e72f80079
            Hide
            salvetore Michael de Raadt added a comment -

            Thanks for reporting this.

            I've put it on our backlog and we'll try to get to it as soon as we can.

            Show
            salvetore Michael de Raadt added a comment - Thanks for reporting this. I've put it on our backlog and we'll try to get to it as soon as we can.
            salvetore Michael de Raadt made changes -
            Fix Version/s STABLE backlog [ 10463 ]
            Labels patch triaged
            Difficulty Easy
            moodle.com moodle.com made changes -
            Fix Version/s STABLE Sprint 11 [ 10751 ]
            Fix Version/s STABLE backlog [ 10463 ]
            rwijaya Rossiani Wijaya made changes -
            Assignee moodle.com [ moodle.com ] Rossiani Wijaya [ rwijaya ]
            rwijaya Rossiani Wijaya made changes -
            Status Open [ 1 ] Development in progress [ 3 ]
            rwijaya Rossiani Wijaya made changes -
            Pull Master Diff URL https://github.com/rwijaya/moodle/compare/master...MDL-27931
            Pull Master Branch MDL-27931
            Pull 2.0 Diff URL https://github.com/rwijaya/moodle/compare/MOODLE_20_STABLE...MDL-27931_m20
            Pull 2.0 Branch MDL-27931_m20
            Testing Instructions 1. login as admin
            2. from side blocks, site admin > users > accounts > upload users
            3. upload user cvs file (file is attached)
            4. make sure city field is empty

            required error should appear on city field.
            Pull from Repository git://github.com/rwijaya/moodle.git
            Attachment users.cvs [ 24305 ]
            rwijaya Rossiani Wijaya made changes -
            Link This issue has a non-specific relationship to MDL-28211 [ MDL-28211 ]
            Hide
            rwijaya Rossiani Wijaya added a comment -

            Thanks Charles for providing the patch.

            In addition to Charles' patch, I add invalid city field notice to the preview table (right after the upload page).

            Diff url is available above.

            Thanks

            Show
            rwijaya Rossiani Wijaya added a comment - Thanks Charles for providing the patch. In addition to Charles' patch, I add invalid city field notice to the preview table (right after the upload page). Diff url is available above. Thanks
            rwijaya Rossiani Wijaya made changes -
            Status Development in progress [ 3 ] Waiting for peer review [ 10012 ]
            Peer reviewer nebgor
            Hide
            nebgor Aparup Banerjee added a comment -

            Hi Rossi,
            This seems to work fine for me. when city isn't filled in it shows error about the field being required.
            code seems to be inline with rest of code. i haven't looked deeper into it.
            cheers,
            Aparup

            Show
            nebgor Aparup Banerjee added a comment - Hi Rossi, This seems to work fine for me. when city isn't filled in it shows error about the field being required. code seems to be inline with rest of code. i haven't looked deeper into it. cheers, Aparup
            nebgor Aparup Banerjee made changes -
            Original Estimate 0 minutes [ 0 ]
            Remaining Estimate 0 minutes [ 0 ]
            Status Waiting for peer review [ 10012 ] Development in progress [ 3 ]
            Hide
            rwijaya Rossiani Wijaya added a comment -

            Thanks Apu for testing.

            submitting for integration.

            Show
            rwijaya Rossiani Wijaya added a comment - Thanks Apu for testing. submitting for integration.
            rwijaya Rossiani Wijaya made changes -
            Status Development in progress [ 3 ] Waiting for integration review [ 10010 ]
            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
            samhemelryk Sam Hemelryk made changes -
            Currently in integration Yes
            stronk7 Eloy Lafuente (stronk7) made changes -
            Fix Version/s 2.0.4 [ 10652 ]
            Fix Version/s 2.1.1 [ 10750 ]
            Affects Version/s 2.2 [ 10656 ]
            stronk7 Eloy Lafuente (stronk7) made changes -
            Status Waiting for integration review [ 10010 ] Integration review in progress [ 10004 ]
            Integrator stronk7
            Hide
            stronk7 Eloy Lafuente (stronk7) added a comment -

            This has been integrated, thanks! (master, 20 and 21).

            Show
            stronk7 Eloy Lafuente (stronk7) added a comment - This has been integrated, thanks! (master, 20 and 21).
            stronk7 Eloy Lafuente (stronk7) made changes -
            Status Integration review in progress [ 10004 ] Waiting for testing [ 10005 ]
            marina Marina Glancy made changes -
            Status Waiting for testing [ 10005 ] Testing in progress [ 10011 ]
            Tester marina
            marina Marina Glancy made changes -
            Status Testing in progress [ 10011 ] Tested [ 10006 ]
            Hide
            samhemelryk Sam Hemelryk added a comment -

            Congratulations - this fix has just been released in the weeklies.

            Show
            samhemelryk Sam Hemelryk added a comment - Congratulations - this fix has just been released in the weeklies.
            samhemelryk Sam Hemelryk made changes -
            Status Tested [ 10006 ] Closed [ 6 ]
            Resolution Fixed [ 1 ]
            Currently in integration Yes
            Integration date 13/Jul/11
            stronk7 Eloy Lafuente (stronk7) made changes -
            Fix Version/s STABLE Sprint 11 [ 10751 ]

              People

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

                Dates

                • Created:
                  Updated:
                  Resolved:
                  Fix Release Date:
                  1/Aug/11