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

Wrong validation in upload course - part 2

XMLWordPrintable

    • MOODLE_404_STABLE
    • MOODLE_404_STABLE
    • MDL-81365-main
    • Hide

      CiBoT will cover mostly. Also copied from MDL-43056 (that tracker also has needed csv attached):

      Testing scenario

      1. Login as admin
      2. Create a new category (Home ► Site administration ► Courses ► Add a category). It should have the id = 2, if not, please change the category ids in the provided test files (csv).
      3. Go to upload courses (Home ► Site administration ► Courses ► Upload courses) and upload the first file (upload_test.csv) - Note: change separator to ';' for the upload to work correctly.
      4. Complete the upload process and verify all went correct.
      5. Assign a second user the manager system role (Home ► Site administration ► Users ► Permissions ► Assign system roles).
      6. Login with this account.
      7. Go again to the Upload courses page and upload the second file (upload_test2.csv) - Note: change separator to ';' for the upload to work correctly.
      8. Complete the upload process and verify all went correct.
      9. Change the permission for the 'tool/uploadcourse:use' capability on the newly created category. (Home ► Site administration ► Courses ► Manage courses and categories). Set the permission to 'Prohibit' for the role Manager.
      10. Try again to upload the course file (upload_test3.csv) - Note: change separator to ';' for the upload to work correctly.
      11. Verify there should be an error on the creation of the course in the second category, but complete the upload courses process anyway.
      12. Please do the rollback done on step 9 and give the permissions back on this course category for the role Manager.
      13. Repeat step 10, but providing another file (upload_test4.csv) Note: change separator to ';' for the upload to work correctly.
      14. Finish the Upload process and verify that both courses were created without throwing an error.
         
      Show
      CiBoT will cover mostly. Also copied from MDL-43056 (that tracker also has needed csv attached): Testing scenario Login as admin Create a new category (Home ► Site administration ► Courses ► Add a category). It should have the id = 2, if not, please change the category ids in the provided test files (csv). Go to upload courses (Home ► Site administration ► Courses ► Upload courses) and upload the first file (upload_test.csv) -  Note: change separator to ';' for the upload to work correctly. Complete the upload process and  verify  all went correct. Assign a second user the manager system role (Home ► Site administration ► Users ► Permissions ► Assign system roles). Login with this account. Go again to the Upload courses page and upload the second file (upload_test2.csv) -  Note: change separator to ';' for the upload to work correctly. Complete the upload process and  verify  all went correct. Change the permission for the  'tool/uploadcourse:use'  capability on the newly created category. (Home ► Site administration ► Courses ► Manage courses and categories). Set the permission to 'Prohibit' for the role Manager. Try again to upload the course file (upload_test3.csv) -  Note: change separator to ';' for the upload to work correctly. Verify  there should be an error on the creation of the course in the second category, but complete the upload courses process anyway. Please do the rollback done on step 9 and give the permissions back on this course category for the role Manager. Repeat step 10, but providing another file (upload_test4.csv)  Note: change separator to ';' for the upload to work correctly. Finish the Upload process and  verify  that both courses were created without throwing an error.  

      1. Consider the method validate_plugin_data_context() - it calls  edit_instance_validation() see https://github.com/moodle/moodle/blob/main/lib/enrollib.php#L3572
      2. This is wrong since the method was designed to do context related validation only (see implementation of this function in cohort enrol plugin as example), while edit_instance_validation does many other checks so 
        • It is bad from code maintainability
        • It creates problem like MDL-80875 and MDL-80599 - there is no form when we upload courses. Its wrong to do form validation for data from csv because some data migth use default values or accept empty and have special workflow for that

      So I think that call must be removed and function should become what it was before MDL-43056 which caused this regression

       

        1. MDL-81365.png
          MDL-81365.png
          349 kB
        2. upload_test.csv
          0.1 kB
        3. upload_test2.csv
          0.1 kB
        4. upload_test3.csv
          0.1 kB
        5. upload_test4.csv
          0.1 kB

            ilyatregubov Ilya Tregubov
            ilyatregubov Ilya Tregubov
            Sara Arjona (@sarjona) Sara Arjona (@sarjona)
            Huong Nguyen Huong Nguyen
            Ron Carl Alfon Yu Ron Carl Alfon Yu
            Votes:
            0 Vote for this issue
            Watchers:
            4 Start watching this issue

              Created:
              Updated:
              Resolved:

                Estimated:
                Original Estimate - Not Specified
                Not Specified
                Remaining:
                Remaining Estimate - 0 minutes
                0m
                Logged:
                Time Spent - 1 hour, 18 minutes
                1h 18m

                  Error rendering 'clockify-timesheets-time-tracking-reports:timer-sidebar'. Please contact your Jira administrators.