Moodle
  1. Moodle
  2. MDL-27931

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

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Minor 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:
    • Rank:
      17573

      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

      1. users.cvs
        0.1 kB
        Rossiani Wijaya

        Issue Links

          Activity

          Hide
          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
          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.
          Hide
          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
          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
          Hide
          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
          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
          Hide
          Rossiani Wijaya added a comment -

          Thanks Apu for testing.

          submitting for integration.

          Show
          Rossiani Wijaya added a comment - Thanks Apu for testing. submitting for integration.
          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
          Hide
          Eloy Lafuente (stronk7) added a comment -

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

          Show
          Eloy Lafuente (stronk7) added a comment - This has been integrated, thanks! (master, 20 and 21).
          Hide
          Sam Hemelryk added a comment -

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

          Show
          Sam Hemelryk added a comment - Congratulations - this fix has just been released in the weeklies.

            People

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

              Dates

              • Created:
                Updated:
                Resolved: