Moodle
  1. Moodle
  2. MDL-42850

Bulk user upload (csv) invalid csv file errors are inadequate.

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 2.4.7, 2.5, 2.6
    • Fix Version/s: 2.6.1
    • Component/s: Administration
    • Labels:
    • Testing Instructions:
      Hide
      1. Edit the supplied users.csv file and add another column to one of the entries.
      2. Try to upload the file [Administration ► Site administration ► Users ► Accounts ► Upload users].
      3. You should get an error that says "error/Invalid CSV file format - number of columns is not constant!"
      4. Try uploading an empty csv file. You should get an error message saying you have an empty file (from the file picker, if you could get past using an empty file in the file picker then the next page would also generate an error specifying that the file is empty).
      Show
      Edit the supplied users.csv file and add another column to one of the entries. Try to upload the file [Administration ► Site administration ► Users ► Accounts ► Upload users] . You should get an error that says "error/Invalid CSV file format - number of columns is not constant!" Try uploading an empty csv file. You should get an error message saying you have an empty file (from the file picker, if you could get past using an empty file in the file picker then the next page would also generate an error specifying that the file is empty).
    • Affected Branches:
      MOODLE_24_STABLE, MOODLE_25_STABLE, MOODLE_26_STABLE
    • Fixed Branches:
      MOODLE_26_STABLE
    • Pull from Repository:
    • Pull Master Branch:
      wip-MDL-42850-master

      Description

      Any incorrect user csv files that are uploaded just pop up a message saying that the file didn't load properly. The csv class stores more meaningful error messages which the bulk user upload file doesn't use.
      Additional information as to why the file didn't load properly would be invaluable.

        Gliffy Diagrams

          Activity

          Hide
          Frédéric Massart added a comment -

          Hi Adrian,

          Your patch looks good. I would only suggest using a proper language string with the print_error() function. You could keep the csvloaderror string, but add a {$a} to append the reason of the failure. The $a can be passed as 4th argument of print_error().

          Cheers,
          Fred

          Show
          Frédéric Massart added a comment - Hi Adrian, Your patch looks good. I would only suggest using a proper language string with the print_error() function. You could keep the csvloaderror string, but add a {$a} to append the reason of the failure. The $a can be passed as 4th argument of print_error(). Cheers, Fred
          Hide
          Adrian Greeve added a comment -

          Thanks Fred, that's a good idea.
          I've fixed that up. Pushing to integration.

          Show
          Adrian Greeve added a comment - Thanks Fred, that's a good idea. I've fixed that up. Pushing to 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
          Adrian Greeve added a comment -

          Rebased.

          Show
          Adrian Greeve added a comment - Rebased.
          Hide
          Sam Hemelryk added a comment -

          Thanks Adrian - this has been integrated now to 26 and master.
          After debating it with myself I settled on this being an improvement rather than a bug.
          If you'd like it backported further please let me know.

          Show
          Sam Hemelryk added a comment - Thanks Adrian - this has been integrated now to 26 and master. After debating it with myself I settled on this being an improvement rather than a bug. If you'd like it backported further please let me know.
          Hide
          Ankit Agarwal added a comment -

          Thanks, works as described.
          The filpicker generated the error as expected, I couldn't see a way of bypassing it.

          Passing the issue. cheers

          Show
          Ankit Agarwal added a comment - Thanks, works as described. The filpicker generated the error as expected, I couldn't see a way of bypassing it. Passing the issue. cheers
          Hide
          Eloy Lafuente (stronk7) added a comment -

          ...
          But still, I thank you, for you made me stronger…

          Stronger as the beast that roar in the wild
          Stronger as the storm across the ocean
          Stronger as the diamond that won’t break
          Stronger enough to take all heart aches….

          I thank you my friend, for you made me stronger…

          ---- Juneah Landicho

          Closing as fixed. Ciao

          Show
          Eloy Lafuente (stronk7) added a comment - ... But still, I thank you, for you made me stronger… Stronger as the beast that roar in the wild Stronger as the storm across the ocean Stronger as the diamond that won’t break Stronger enough to take all heart aches…. I thank you my friend, for you made me stronger… ---- Juneah Landicho Closing as fixed. Ciao

            People

            • Votes:
              0 Vote for this issue
              Watchers:
              3 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved: