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

Upload of Users with Custom Profile Menu fields may fail

    Details

    • Database:
      PostgreSQL
    • Testing Instructions:
      Hide
      1. Log in as admin
      2. Create custom profile field "menu" of type menu. Options "one two three four five" (Site administration -> Users -> Accounts -> User profile fields)
      3. Try uploading attached file (Site administration -> Users -> Accounts -> Upload users)
      4. You should not see any error and two new users should be uploaded, check there profile and make sure menu has same option as csv file
      5. Now edit upload file and change menu entries to "4" and "2"
      6. Try upload and you should not any way to upload file. Also, error should be visible in status.
      Show
      Log in as admin Create custom profile field "menu" of type menu. Options "one two three four five" (Site administration -> Users -> Accounts -> User profile fields) Try uploading attached file (Site administration -> Users -> Accounts -> Upload users) You should not see any error and two new users should be uploaded, check there profile and make sure menu has same option as csv file Now edit upload file and change menu entries to "4" and "2" Try upload and you should not any way to upload file. Also, error should be visible in status.
    • Workaround:
      Hide

      Create Custom Profile yearlevel as text field

      Show
      Create Custom Profile yearlevel as text field
    • Affected Branches:
      MOODLE_20_STABLE, MOODLE_21_STABLE, MOODLE_22_STABLE
    • Fixed Branches:
      MOODLE_23_STABLE
    • Pull Master Branch:
      wip-mdl-31654

      Description

      Uploading users that have a column for a Custom Profile field which is a menu may fail because the data from the CSV is directly treated as a key in the Menu Class.

      This means if you have user_profile_yearlevel = 9, it will return the value for key 9 or blank in custom profile field yearlevel which may or may not exist. If it did exist it could return the wrong value to be assigned to the yearlevel.

      Reproduction steps:

      1. Create custom profile of type menu.
      2. Assign various values to the menu.
      3. Create a csv to upload a value to the menu.
      4. Update existing users and Existing user details from the file will work.

      When the csv is processed you will see this error :-

      Error writing to database
       
      More information about this error
       
      Debug info: ERROR: null value in column "data" violates not-null constraint
      UPDATE mdl_user_info_data SET userid = $1,fieldid = $2,data = $3 WHERE id=$4
      [array (
      'userid' => '80',
      'fieldid' => '4',
      'data' => NULL,
      0 => '384',
      )]
      Stack trace:
      line 397 of /lib/dml/moodle_database.php: dml_write_exception thrown
      line 232 of /lib/dml/pgsql_native_moodle_database.php: call to moodle_database->query_end()
      line 936 of /lib/dml/pgsql_native_moodle_database.php: call to pgsql_native_moodle_database->query_end()
      line 976 of /lib/dml/pgsql_native_moodle_database.php: call to pgsql_native_moodle_database->update_record_raw()
      line 118 of /user/profile/lib.php: call to pgsql_native_moodle_database->update_record()
      line 427 of /user/profile/lib.php: call to profile_field_base->edit_save_data()
      line 572 of /admin/uploaduser.php: call to profile_save_data()

        Gliffy Diagrams

        1. user.csv
          0.2 kB
          Michael de Raadt
        2. user.csv
          0.2 kB
          Rajesh Taneja

          Issue Links

            Activity

            tlock Tim Lock created issue -
            tlock Tim Lock made changes -
            Field Original Value New Value
            Priority Minor [ 4 ] Major [ 3 ]
            Labels partner
            Hide
            tlock Tim Lock added a comment -

            Applies cleanly to 2.0, 2.1 & 2.2

            Show
            tlock Tim Lock added a comment - Applies cleanly to 2.0, 2.1 & 2.2
            Hide
            rajeshtaneja Rajesh Taneja added a comment -

            Thanks for providing the patch Tim.
            You might want to check for field.class.php file before including it, as there can be custom profile fields and this might break.

            Show
            rajeshtaneja Rajesh Taneja added a comment - Thanks for providing the patch Tim. You might want to check for field.class.php file before including it, as there can be custom profile fields and this might break.
            Hide
            salvetore Michael de Raadt added a comment -

            Thanks for reporting this.

            Raj is keen to work on that very soon.

            Show
            salvetore Michael de Raadt added a comment - Thanks for reporting this. Raj is keen to work on that very soon.
            salvetore Michael de Raadt made changes -
            Fix Version/s STABLE Sprint 18 [ 11650 ]
            Testing Instructions Create custom profile of type menu.
            Assign various values to the menu.
            Create a csv to upload a value to the menu.
             - Update existing users and Existing user details from the file will work.
            When the csv is processed you will see this error :-

            Error writing to database

            More information about this error

            Debug info: ERROR: null value in column "data" violates not-null constraint
            UPDATE mdl_user_info_data SET userid = $1,fieldid = $2,data = $3 WHERE id=$4
            [array (
            'userid' => '80',
            'fieldid' => '4',
            'data' => NULL,
            0 => '384',
            )]
            Stack trace:
            line 397 of /lib/dml/moodle_database.php: dml_write_exception thrown
            line 232 of /lib/dml/pgsql_native_moodle_database.php: call to moodle_database->query_end()
            line 936 of /lib/dml/pgsql_native_moodle_database.php: call to pgsql_native_moodle_database->query_end()
            line 976 of /lib/dml/pgsql_native_moodle_database.php: call to pgsql_native_moodle_database->update_record_raw()
            line 118 of /user/profile/lib.php: call to pgsql_native_moodle_database->update_record()
            line 427 of /user/profile/lib.php: call to profile_field_base->edit_save_data()
            line 572 of /admin/uploaduser.php: call to profile_save_data()

            Description When uploading users that have a column for a Custom Profile field which is a menu may fail because the data from the CSV is directly treated as a key in the Menu Class.

            This means if you have user_profile_yearlevel = 9, it will return the value for key 9 or blank in custom profile field yearlevel which may or may not exist. If it did exist it could return the wrong value to be assigned to the yearlevel.
            Uploading users that have a column for a Custom Profile field which is a menu may fail because the data from the CSV is directly treated as a key in the Menu Class.

            This means if you have user_profile_yearlevel = 9, it will return the value for key 9 or blank in custom profile field yearlevel which may or may not exist. If it did exist it could return the wrong value to be assigned to the yearlevel.

            *Reproduction steps:*
            # Create custom profile of type menu.
            # Assign various values to the menu.
            # Create a csv to upload a value to the menu.
            # Update existing users and Existing user details from the file will work.

            When the csv is processed you will see this error :-
            {noformat}
            Error writing to database

            More information about this error

            Debug info: ERROR: null value in column "data" violates not-null constraint
            UPDATE mdl_user_info_data SET userid = $1,fieldid = $2,data = $3 WHERE id=$4
            [array (
            'userid' => '80',
            'fieldid' => '4',
            'data' => NULL,
            0 => '384',
            )]
            Stack trace:
            line 397 of /lib/dml/moodle_database.php: dml_write_exception thrown
            line 232 of /lib/dml/pgsql_native_moodle_database.php: call to moodle_database->query_end()
            line 936 of /lib/dml/pgsql_native_moodle_database.php: call to pgsql_native_moodle_database->query_end()
            line 976 of /lib/dml/pgsql_native_moodle_database.php: call to pgsql_native_moodle_database->update_record_raw()
            line 118 of /user/profile/lib.php: call to pgsql_native_moodle_database->update_record()
            line 427 of /user/profile/lib.php: call to profile_field_base->edit_save_data()
            line 572 of /admin/uploaduser.php: call to profile_save_data()
            {noformat}
            Labels partner partner triaged
            salvetore Michael de Raadt made changes -
            Labels partner triaged partner patch triaged
            Hide
            tlock Tim Lock added a comment -
            Show
            tlock Tim Lock added a comment - I have added another patch. https://github.com/tlock/moodle/commit/97f4e51d4f4e7e4ba697880ecdc33480a3d2e691 Regards, Tim
            rajeshtaneja Rajesh Taneja made changes -
            Status Open [ 1 ] Development in progress [ 3 ]
            Hide
            rajeshtaneja Rajesh Taneja added a comment -

            Thanks for the great patch Tim,

            I have created branch with your patch, and did following modifications:

            1. In convert_csv_data function, for loop is replaced with array_search
            2. docblock added for pre_process_profile_data
            3. In 22 and master upload has been moved under tools.

            I still have one doubt about convert_csv_data returning '0' if value not found in options. Should user get error if value is not found or default value is acceptable? Will push this for peer-review to get more thoughts on this.

            Show
            rajeshtaneja Rajesh Taneja added a comment - Thanks for the great patch Tim, I have created branch with your patch, and did following modifications: In convert_csv_data function, for loop is replaced with array_search docblock added for pre_process_profile_data In 22 and master upload has been moved under tools. I still have one doubt about convert_csv_data returning '0' if value not found in options. Should user get error if value is not found or default value is acceptable? Will push this for peer-review to get more thoughts on this.
            rajeshtaneja Rajesh Taneja made changes -
            Status Development in progress [ 3 ] Waiting for peer review [ 10012 ]
            rajeshtaneja Rajesh Taneja made changes -
            Peer reviewer rwijaya
            rajeshtaneja Rajesh Taneja made changes -
            Testing Instructions # Log in as admin
            # Create custom profile field "menu" of type menu. Options "one two three four five" (Site administration -> Users -> Accounts -> User profile fields)
            # Try uploading attached file (Site administration -> Users -> Accounts -> Upload users)
            # You should not see any error and two new users should be uploaded, check there profile and make sure menu has same option as csv file
            rajeshtaneja Rajesh Taneja made changes -
            Attachment user.csv [ 27089 ]
            Hide
            skodak Petr Skoda added a comment -

            sorry, this is horrible:

            if ($field->datatype == 'menu') {

            please do not hardcode any plugin names in core! Things like this have to be implemented in plugins, not core.

            Also this is going to break backwards compatibility really badly for please that know how it works right now - files that work now would break after this change, right? Maybe numeric values could be treated as keys and non-numeric as values. In any case this must be documented in at least release notes.

            Show
            skodak Petr Skoda added a comment - sorry, this is horrible: if ($field->datatype == 'menu') { please do not hardcode any plugin names in core! Things like this have to be implemented in plugins, not core. Also this is going to break backwards compatibility really badly for please that know how it works right now - files that work now would break after this change, right? Maybe numeric values could be treated as keys and non-numeric as values. In any case this must be documented in at least release notes.
            Hide
            rajeshtaneja Rajesh Taneja added a comment - - edited

            Thanks for the feedback Petr,

            I don't see why this will break backward compatibility. Also, we can't be sure that values are non-numeric, as they can be numeric as well.

            Removed Hardcoding from core, and will update docs, if this gets integrated.

            Show
            rajeshtaneja Rajesh Taneja added a comment - - edited Thanks for the feedback Petr, I don't see why this will break backward compatibility. Also, we can't be sure that values are non-numeric, as they can be numeric as well. Removed Hardcoding from core, and will update docs, if this gets integrated.
            Hide
            skodak Petr Skoda added a comment -

            Backwards compatibility:
            1/ create file that uses numerical indexes - works at the moment 100%
            2/ apply your patch
            3/ test it - it should work, if not you have major BC problem

            Show
            skodak Petr Skoda added a comment - Backwards compatibility: 1/ create file that uses numerical indexes - works at the moment 100% 2/ apply your patch 3/ test it - it should work, if not you have major BC problem
            Hide
            rajeshtaneja Rajesh Taneja added a comment -

            Thanks Petr,
            So do you recommend to have this documented, then to have this patch?
            or shall we have this in master and not back-port it?

            Show
            rajeshtaneja Rajesh Taneja added a comment - Thanks Petr, So do you recommend to have this documented, then to have this patch? or shall we have this in master and not back-port it?
            Hide
            rajeshtaneja Rajesh Taneja added a comment -

            As discussed in devchat, this patch can go in master and release doc for 2.3 should contain this information. Hence adding master branch and doc_required tag

            Show
            rajeshtaneja Rajesh Taneja added a comment - As discussed in devchat, this patch can go in master and release doc for 2.3 should contain this information. Hence adding master branch and doc_required tag
            rajeshtaneja Rajesh Taneja made changes -
            Labels partner patch triaged docs_required partner patch triaged
            rwijaya Rossiani Wijaya made changes -
            Original Estimate 0 minutes [ 0 ]
            Remaining Estimate 0 minutes [ 0 ]
            Status Waiting for peer review [ 10012 ] Peer review in progress [ 10013 ]
            Hide
            rwijaya Rossiani Wijaya added a comment -

            Hi Raj,

            When entering in valid value for the custom profile menu, I still get the following error:

            Error writing to database
             
            More information about this error
            Debug info: Column 'data' cannot be null
            INSERT INTO mdl_user_info_data (userid,fieldid,data) VALUES(?,?,?)
            [array (
            0 => 378,
            1 => '127',
            2 => NULL,
            )]
            Stack trace:
             
                line 413 of /lib/dml/moodle_database.php: dml_write_exception thrown
                line 902 of /lib/dml/mysqli_native_moodle_database.php: call to moodle_database->query_end()
                line 944 of /lib/dml/mysqli_native_moodle_database.php: call to mysqli_native_moodle_database->insert_record_raw()
                line 120 of /user/profile/lib.php: call to mysqli_native_moodle_database->insert_record()
                line 427 of /user/profile/lib.php: call to profile_field_base->edit_save_data()
                line 747 of /admin/tool/uploaduser/index.php: call to profile_save_data()

            Isn't the patch should prevent displaying the above error?

            Show
            rwijaya Rossiani Wijaya added a comment - Hi Raj, When entering in valid value for the custom profile menu, I still get the following error: Error writing to database   More information about this error Debug info: Column 'data' cannot be null INSERT INTO mdl_user_info_data (userid,fieldid,data) VALUES(?,?,?) [array ( 0 => 378, 1 => '127', 2 => NULL, )] Stack trace:   line 413 of /lib/dml/moodle_database.php: dml_write_exception thrown line 902 of /lib/dml/mysqli_native_moodle_database.php: call to moodle_database->query_end() line 944 of /lib/dml/mysqli_native_moodle_database.php: call to mysqli_native_moodle_database->insert_record_raw() line 120 of /user/profile/lib.php: call to mysqli_native_moodle_database->insert_record() line 427 of /user/profile/lib.php: call to profile_field_base->edit_save_data() line 747 of /admin/tool/uploaduser/index.php: call to profile_save_data() Isn't the patch should prevent displaying the above error?
            rwijaya Rossiani Wijaya made changes -
            Status Peer review in progress [ 10013 ] Development in progress [ 3 ]
            Hide
            rajeshtaneja Rajesh Taneja added a comment -

            I have added another commit to handle wrong data for custom menu profile field. I am not sure if this should be part of this bug as we never did any checks on custom profile fields and probably needs to be improved for all the required custom_profile_fields.

            Show
            rajeshtaneja Rajesh Taneja added a comment - I have added another commit to handle wrong data for custom menu profile field. I am not sure if this should be part of this bug as we never did any checks on custom profile fields and probably needs to be improved for all the required custom_profile_fields.
            rajeshtaneja Rajesh Taneja made changes -
            Status Development in progress [ 3 ] Waiting for peer review [ 10012 ]
            rwijaya Rossiani Wijaya made changes -
            Status Waiting for peer review [ 10012 ] Peer review in progress [ 10013 ]
            Hide
            rwijaya Rossiani Wijaya added a comment -

            Hi Raj,

            Some comments regarding the patch.

            1. On user/profile/field/menu/field.class.php line 97, you might want to change -1 to null (online comment).

            2. It would probably better to move function pre_process_profile_data() to user/profile/lib.php.

            3. On admin/tool/uploaduser/index.php, the content on line 1039 (to 1058) and pre_process_profile_data() look similar. The function could probably be modified to handle both cases.

            Show
            rwijaya Rossiani Wijaya added a comment - Hi Raj, Some comments regarding the patch. 1. On user/profile/field/menu/field.class.php line 97, you might want to change -1 to null (online comment). 2. It would probably better to move function pre_process_profile_data() to user/profile/lib.php. 3. On admin/tool/uploaduser/index.php, the content on line 1039 (to 1058) and pre_process_profile_data() look similar. The function could probably be modified to handle both cases.
            rwijaya Rossiani Wijaya made changes -
            Status Peer review in progress [ 10013 ] Development in progress [ 3 ]
            Hide
            rajeshtaneja Rajesh Taneja added a comment - - edited

            Thanks Rossie,

            pre_process_profile_data can't be moved to uer/profile/lib.php, but can be moved to admin/tools/uploaduser/localib.php. As it is only related to user upload feature only. As per code-replication, in first case it's stdClass and updates the value. In second case it's an array and sets error message.

            Not sure if we should combine the two and make it complicated and update data-type.

            Show
            rajeshtaneja Rajesh Taneja added a comment - - edited Thanks Rossie, pre_process_profile_data can't be moved to uer/profile/lib.php, but can be moved to admin/tools/uploaduser/localib.php. As it is only related to user upload feature only. As per code-replication, in first case it's stdClass and updates the value. In second case it's an array and sets error message. Not sure if we should combine the two and make it complicated and update data-type.
            Hide
            rajeshtaneja Rajesh Taneja added a comment -

            Hello Rossie,

            As suggested, I have combined two loops.
            Can you please review it again.

            Show
            rajeshtaneja Rajesh Taneja added a comment - Hello Rossie, As suggested, I have combined two loops. Can you please review it again.
            rajeshtaneja Rajesh Taneja made changes -
            Status Development in progress [ 3 ] Waiting for peer review [ 10012 ]
            rwijaya Rossiani Wijaya made changes -
            Status Waiting for peer review [ 10012 ] Peer review in progress [ 10013 ]
            Hide
            rwijaya Rossiani Wijaya added a comment -

            It looks good Raj.

            Thanks for fixing.

            Show
            rwijaya Rossiani Wijaya added a comment - It looks good Raj. Thanks for fixing.
            rwijaya Rossiani Wijaya made changes -
            Status Peer review in progress [ 10013 ] Development in progress [ 3 ]
            rajeshtaneja Rajesh Taneja made changes -
            Testing Instructions # Log in as admin
            # Create custom profile field "menu" of type menu. Options "one two three four five" (Site administration -> Users -> Accounts -> User profile fields)
            # Try uploading attached file (Site administration -> Users -> Accounts -> Upload users)
            # You should not see any error and two new users should be uploaded, check there profile and make sure menu has same option as csv file
            # Log in as admin
            # Create custom profile field "menu" of type menu. Options "one two three four five" (Site administration -> Users -> Accounts -> User profile fields)
            # Try uploading attached file (Site administration -> Users -> Accounts -> Upload users)
            # You should not see any error and two new users should be uploaded, check there profile and make sure menu has same option as csv file
            # Now edit upload file and change menu entries to "4" and "2"
            # Try upload and you should not any way to upload file. Also, error should be visible in status.
            Pull 2.1 Branch wip-mdl-31654-m21
            Hide
            rajeshtaneja Rajesh Taneja added a comment -

            Thanks Rossie,

            pushing it for integration review.

            Show
            rajeshtaneja Rajesh Taneja added a comment - Thanks Rossie, pushing it for integration review.
            rajeshtaneja Rajesh Taneja 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 [ 10041 ]
            Hide
            rajeshtaneja Rajesh Taneja added a comment -

            Branches re-based.

            Show
            rajeshtaneja Rajesh Taneja added a comment - Branches re-based.
            Hide
            skodak Petr Skoda added a comment -

            to integrators: I still believe this might be viewed as regression if original CSV file with numeric menu indexes does not work, this would create hard to detect problems if menu uses numeric values. Change like this would need to be announced and documented.

            Show
            skodak Petr Skoda added a comment - to integrators: I still believe this might be viewed as regression if original CSV file with numeric menu indexes does not work, this would create hard to detect problems if menu uses numeric values. Change like this would need to be announced and documented.
            nebgor Aparup Banerjee made changes -
            Status Waiting for integration review [ 10010 ] Integration review in progress [ 10004 ]
            Integrator nebgor
            Hide
            nebgor Aparup Banerjee added a comment -

            Hi Raj,

            I'm sending this back for further discussion.
            I feel the decision to affect systems out there to the extent that they might have to change other systems just to be able to upgrade to 2.3 should deserve a little more discussion.

            I've noted this in dev chat : http://moodle.org/local/chatlogs/index.php?conversationid=9615

            I think we should separate the solution (and its effects) from a design decision here too. Do we want to support numerical indexes? yes/no ? (we have for so long - what changed the design? )

            if yes - how?
            if no - you're solution seems fine but we need to plan this change so that future uptakes of the version can be planned for by others.

            also note :
            admin/tool/uploaduser/locallib.php:414: trailing whitespace.
            if (method_exists($formfield, 'convert_external_data') &&

            ps: Also noting that in a stable (fixes) sprint - we may not have given this enough design/supportability thought too.

            so simply - please discuss this change in support with more views/people.

            Show
            nebgor Aparup Banerjee added a comment - Hi Raj, I'm sending this back for further discussion. I feel the decision to affect systems out there to the extent that they might have to change other systems just to be able to upgrade to 2.3 should deserve a little more discussion. I've noted this in dev chat : http://moodle.org/local/chatlogs/index.php?conversationid=9615 I think we should separate the solution (and its effects) from a design decision here too. Do we want to support numerical indexes? yes/no ? (we have for so long - what changed the design? ) if yes - how? if no - you're solution seems fine but we need to plan this change so that future uptakes of the version can be planned for by others. also note : admin/tool/uploaduser/locallib.php:414: trailing whitespace. if (method_exists($formfield, 'convert_external_data') && ps: Also noting that in a stable (fixes) sprint - we may not have given this enough design/supportability thought too. so simply - please discuss this change in support with more views/people.
            nebgor Aparup Banerjee made changes -
            Status Integration review in progress [ 10004 ] Reopened [ 4 ]
            Hide
            rajeshtaneja Rajesh Taneja added a comment -

            Thanks for the feedback Aparup,

            This was discussed in Devchat. Tim, Eloy agreed about having this in 2.3
            Not sure, about more discussion. As per rest of the fields, they expect value but with menu it expects index.
            You can see there is one duplicate for this issue, so seems it is a problem.

            As per stable, as this is change in functionality, I haven't back-ported this.

            @Petr: If this gets integrated, I will update the docs.

            Show
            rajeshtaneja Rajesh Taneja added a comment - Thanks for the feedback Aparup, This was discussed in Devchat. Tim, Eloy agreed about having this in 2.3 Not sure, about more discussion. As per rest of the fields, they expect value but with menu it expects index. You can see there is one duplicate for this issue, so seems it is a problem. As per stable, as this is change in functionality, I haven't back-ported this. @Petr: If this gets integrated, I will update the docs.
            Hide
            skodak Petr Skoda added a comment - - edited

            Thanks everybody! I somehow missed the discussion in dev chat, sorry, sometimes it is better to copy paste any decisions from there into tracker.

            Show
            skodak Petr Skoda added a comment - - edited Thanks everybody! I somehow missed the discussion in dev chat, sorry, sometimes it is better to copy paste any decisions from there into tracker.
            Hide
            rajeshtaneja Rajesh Taneja added a comment -

            Thanks Petr,

            My Bad, I should have copied the discussion. Will remember to paste it next time

            As per the whole conversation, it is a bug. Unfortunately, we have been living with it. So, 2.3 is a good option to correct it.

            Show
            rajeshtaneja Rajesh Taneja added a comment - Thanks Petr, My Bad, I should have copied the discussion. Will remember to paste it next time As per the whole conversation, it is a bug. Unfortunately, we have been living with it. So, 2.3 is a good option to correct it.
            Hide
            rajeshtaneja Rajesh Taneja added a comment -

            I have added Martin, Michael, Sam and Eloy as watchers to have there opinion on this.
            @Petr: If you think patch makes sense then please give +1 for it.

            Show
            rajeshtaneja Rajesh Taneja added a comment - I have added Martin, Michael, Sam and Eloy as watchers to have there opinion on this. @Petr: If you think patch makes sense then please give +1 for it.
            Hide
            rajeshtaneja Rajesh Taneja added a comment - - edited

            As mentioned by Rossie, about user upload showing exception. This is just a status message and will let user proceed. Updating user profile, doesn't mention about wrong fields if uploaded will not let you proceed (which is included in this patch). So doc update is required.

            Show
            rajeshtaneja Rajesh Taneja added a comment - - edited As mentioned by Rossie, about user upload showing exception. This is just a status message and will let user proceed. Updating user profile , doesn't mention about wrong fields if uploaded will not let you proceed (which is included in this patch). So doc update is required.
            rajeshtaneja Rajesh Taneja made changes -
            Labels docs_required partner patch triaged docs_required partner patch triaged ui_change
            Hide
            salvetore Michael de Raadt added a comment -

            I don't see a problem with this if we note this as a behavioural change in the release notes and update the user docs accordingly. The current labels are set correctly to prompt this.

            Show
            salvetore Michael de Raadt added a comment - I don't see a problem with this if we note this as a behavioural change in the release notes and update the user docs accordingly. The current labels are set correctly to prompt this.
            samhemelryk Sam Hemelryk made changes -
            Currently in integration Yes [ 10041 ]
            Hide
            rajeshtaneja Rajesh Taneja added a comment -

            Branches re-based.
            Hoping will get more +1's, by the time integrator reviews it

            Show
            rajeshtaneja Rajesh Taneja added a comment - Branches re-based. Hoping will get more +1's, by the time integrator reviews it
            rajeshtaneja Rajesh Taneja made changes -
            Status Reopened [ 4 ] Waiting for integration review [ 10010 ]
            samhemelryk Sam Hemelryk made changes -
            Currently in integration Yes [ 10041 ]
            nebgor Aparup Banerjee made changes -
            Status Waiting for integration review [ 10010 ] Integration review in progress [ 10004 ]
            Hide
            nebgor Aparup Banerjee added a comment -

            ok i suppose enough time has passed and hopefully more people see this coming.
            Thanks for the work here, its been integrated now and is ready for testing in master.

            This change should be highlighted release.

            ps: Rajesh, as spoken this was only master, futher to that i take it that there is no fix possible for stable branches.

            Show
            nebgor Aparup Banerjee added a comment - ok i suppose enough time has passed and hopefully more people see this coming. Thanks for the work here, its been integrated now and is ready for testing in master. This change should be highlighted release. ps: Rajesh, as spoken this was only master, futher to that i take it that there is no fix possible for stable branches.
            nebgor Aparup Banerjee made changes -
            Status Integration review in progress [ 10004 ] Waiting for testing [ 10005 ]
            Affects Version/s 2.2.2 [ 11552 ]
            Affects Version/s 2.1.5 [ 11553 ]
            Affects Version/s 2.0.8 [ 11554 ]
            Affects Version/s 2.0.7 [ 11451 ]
            Affects Version/s 2.1.4 [ 11452 ]
            Affects Version/s 2.2.1 [ 11456 ]
            Fix Version/s 2.3 [ 10657 ]
            salvetore Michael de Raadt made changes -
            Tester salvetore
            Hide
            salvetore Michael de Raadt added a comment -

            I've added a corrected CSV without quotes.

            Show
            salvetore Michael de Raadt added a comment - I've added a corrected CSV without quotes.
            salvetore Michael de Raadt made changes -
            Attachment user.csv [ 27703 ]
            salvetore Michael de Raadt made changes -
            Status Waiting for testing [ 10005 ] Testing in progress [ 10011 ]
            Hide
            salvetore Michael de Raadt added a comment -

            Test result: Success.

            Tested in master with MySQL, PostgreSQL and MS SQL.

            Thanks to all involved.

            Show
            salvetore Michael de Raadt added a comment - Test result: Success. Tested in master with MySQL, PostgreSQL and MS SQL. Thanks to all involved.
            salvetore Michael de Raadt made changes -
            Status Testing in progress [ 10011 ] Tested [ 10006 ]
            Hide
            stronk7 Eloy Lafuente (stronk7) added a comment -

            And this has landed upstream, finally! Yay!

            תודה רבה && شكرا جزيلا



            Closing, ciao

            Show
            stronk7 Eloy Lafuente (stronk7) added a comment - And this has landed upstream, finally! Yay! תודה רבה && شكرا جزيلا Closing, ciao
            stronk7 Eloy Lafuente (stronk7) made changes -
            Status Tested [ 10006 ] Closed [ 6 ]
            Resolution Fixed [ 1 ]
            Currently in integration Yes [ 10041 ]
            Integration date 29/Mar/12
            salvetore Michael de Raadt made changes -
            Link This issue will help resolve MDL-26653 [ MDL-26653 ]
            Hide
            tsala Helen Foster added a comment -

            Please could anyone advise what needs to be added to the user docs for this issue, or should the label be dev_docs? (I notice it's mentioned in http://docs.moodle.org/dev/Moodle_2.3_release_notes )

            Show
            tsala Helen Foster added a comment - Please could anyone advise what needs to be added to the user docs for this issue, or should the label be dev_docs? (I notice it's mentioned in http://docs.moodle.org/dev/Moodle_2.3_release_notes )
            Hide
            rajeshtaneja Rajesh Taneja added a comment -

            Hello Helen,

            For custom profile fields that are a menu, it was mentioned to use corresponding menu number (index), and now it's the value itself.
            http://docs.moodle.org/22/en/Upload_users#Fields_that_can_be_included - Custom profile field names (first example, second line)

            Show
            rajeshtaneja Rajesh Taneja added a comment - Hello Helen, For custom profile fields that are a menu, it was mentioned to use corresponding menu number (index), and now it's the value itself. http://docs.moodle.org/22/en/Upload_users#Fields_that_can_be_included - Custom profile field names (first example, second line)
            tsala Helen Foster made changes -
            Labels docs_required partner patch triaged ui_change partner patch triaged ui_change
            Hide
            tsala Helen Foster added a comment -

            Thanks Raj, http://docs.moodle.org/23/en/Upload_users#Fields_that_can_be_included now states:

            For custom profile fields that are a menu, use the corresponding values (new in Moodle 2.3 onwards).

            Example: if your custom field is department and there are three choices - HR, Marketing,Training- then use HR, Marketing and Training.

            Hope I've understood it correctly - please shout if not!

            Show
            tsala Helen Foster added a comment - Thanks Raj, http://docs.moodle.org/23/en/Upload_users#Fields_that_can_be_included now states: For custom profile fields that are a menu, use the corresponding values (new in Moodle 2.3 onwards). Example: if your custom field is department and there are three choices - HR, Marketing,Training- then use HR, Marketing and Training. Hope I've understood it correctly - please shout if not!
            Hide
            rajeshtaneja Rajesh Taneja added a comment -

            Thanks Helen,

            This is perfect.

            Show
            rajeshtaneja Rajesh Taneja added a comment - Thanks Helen, This is perfect.
            Hide
            dougiamas Martin Dougiamas added a comment - - edited

            "HR, Marketing and Training"

            The code will parse that "and"?

            Shouldn't it be something like:

            Example: A custom field 'Department' with one of three values 'HR', 'Marketing' or 'Training'. Just insert one of those three words (eg 'Training') as the value for that field.

            Show
            dougiamas Martin Dougiamas added a comment - - edited "HR, Marketing and Training" The code will parse that "and"? Shouldn't it be something like: Example: A custom field 'Department' with one of three values 'HR', 'Marketing' or 'Training'. Just insert one of those three words (eg 'Training') as the value for that field.
            Hide
            tsala Helen Foster added a comment -

            Thanks Martin, I have amended the example as suggested.

            Show
            tsala Helen Foster added a comment - Thanks Martin, I have amended the example as suggested.
            salvetore Michael de Raadt made changes -
            Link This issue caused a regression MDL-41744 [ MDL-41744 ]
            stronk7 Eloy Lafuente (stronk7) made changes -
            Fix Version/s STABLE Sprint 18 [ 11650 ]

              People

              • Votes:
                5 Vote for this issue
                Watchers:
                6 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved:
                  Fix Release Date:
                  25/Jun/12