Moodle

Make bulk user picture upload handle subdirectories in zip files (with patch)

Details

  • Type: Improvement Improvement
  • Status: Closed Closed
  • Priority: Minor Minor
  • Resolution: Fixed
  • Affects Version/s: 1.9
  • Fix Version/s: 1.9.2
  • Component/s: Administration
  • Labels:
    None

Description

The current code for bulk importing user pictures expects the picture files not to be in subdirectories inside the zip file at all. If the zip file has subdirectories inside it, any files inside those subdirectories are ignored as we skip over directories.

The attached patch improves uploadpicture.php to recursively proccess any subdirectories in the zip file (and the files inside them), in addition to processing the files at the 'root' of the zip file.

Saludos. Iñaki.

Issue Links

This issue has a non-specific relationship to:
MDL-11752 Add ability to mass/bulk upload user profile images Minor Closed

Activity

Ascending order - Click to sort in descending order
Hide
Permalink
Anthony Borrow added a comment -

Iñaki - Great idea to process the subdirectories. That way any image file in the zip file will be processed which seems much more thorough IMO. +1 Peace - Anthony

Show
Anthony Borrow added a comment - Iñaki - Great idea to process the subdirectories. That way any image file in the zip file will be processed which seems much more thorough IMO. +1 Peace - Anthony
Hide
Permalink
Eloy Lafuente (stronk7) added a comment -

Looks interesting! +1

Anyway I've take a quick look to the implementation.... and have found some things:

  • Use of globals... err... uhm... couldn't them be passed via parameters or so? It seems that $userfields and $userfield can be easily skipped and changed to one $fieldname to be used in queries. And $usersupdated and $userserrors can be handled by returning +-1 by process_file() and acumulated in process_directory() that will return them via parameters by reference. And $overwritepicture is one simple parameter.
  • There is one "continue" that shouldn't be there IMO (return instead).

Ciao

Show
Eloy Lafuente (stronk7) added a comment - Looks interesting! +1 Anyway I've take a quick look to the implementation.... and have found some things:
  • Use of globals... err... uhm... couldn't them be passed via parameters or so? It seems that $userfields and $userfield can be easily skipped and changed to one $fieldname to be used in queries. And $usersupdated and $userserrors can be handled by returning +-1 by process_file() and acumulated in process_directory() that will return them via parameters by reference. And $overwritepicture is one simple parameter.
  • There is one "continue" that shouldn't be there IMO (return instead).
Ciao
Hide
Permalink
Eloy Lafuente (stronk7) added a comment -

Hi Iñaki....

are you going to work on this. Or do you prefer to assign it to somebody else? 100% your election!

TIA!

Show
Eloy Lafuente (stronk7) added a comment - Hi Iñaki.... are you going to work on this. Or do you prefer to assign it to somebody else? 100% your election! TIA!
Hide
Permalink
Iñaki Arenaza added a comment -

I can work on it, but it'll have to wait til Wednesday evening, as I'll be quite busy tomorrow. If it can't wait that longer, feel free to assign it to somebody else.

Saludos. Iñaki.

Show
Iñaki Arenaza added a comment - I can work on it, but it'll have to wait til Wednesday evening, as I'll be quite busy tomorrow. If it can't wait that longer, feel free to assign it to somebody else. Saludos. Iñaki.
Hide
Permalink
Eloy Lafuente (stronk7) added a comment -

Oh, that will be perfect Iñaki, I really like you to do that! Not pressure, get your time. Thanks!

Show
Eloy Lafuente (stronk7) added a comment - Oh, that will be perfect Iñaki, I really like you to do that! Not pressure, get your time. Thanks!
Hide
Permalink
Iñaki Arenaza added a comment -

Here is an updated patch that should address all of your concerns. In addition to that, I've added a bit of documentation to the internal functions.

Note that I'll be out of town (without network connectivity) until sunday at night, just in case.

Saludos. Iñaki.

Show
Iñaki Arenaza added a comment - Here is an updated patch that should address all of your concerns. In addition to that, I've added a bit of documentation to the internal functions. Note that I'll be out of town (without network connectivity) until sunday at night, just in case. Saludos. Iñaki.
Hide
Permalink
Eloy Lafuente (stronk7) added a comment -

Thanks! Happy loooong-weekend!

Show
Eloy Lafuente (stronk7) added a comment - Thanks! Happy loooong-weekend!
Hide
Permalink
Iñaki Arenaza added a comment -

Hi Eloy,

any update on this? Have you had a look at the updated patch?

Saludos. Iñaki.

Show
Iñaki Arenaza added a comment - Hi Eloy, any update on this? Have you had a look at the updated patch? Saludos. Iñaki.
Hide
Permalink
Eloy Lafuente (stronk7) added a comment -

Hi Jerome, can you, please, take a look to this? It's interesting stuff to be added ASAP.

Sorry by the delay, Iñaki! And thanks!

Show
Eloy Lafuente (stronk7) added a comment - Hi Jerome, can you, please, take a look to this? It's interesting stuff to be added ASAP. Sorry by the delay, Iñaki! And thanks!
Hide
Permalink
Jerome Mouneyrac added a comment -

Iñaki's patch works fine on 1.9 and the code looks good (thanks for code documentation :o). I'll update the patch code for mdlib 2.0 support during the merge to HEAD.

Show
Jerome Mouneyrac added a comment - Iñaki's patch works fine on 1.9 and the code looks good (thanks for code documentation :o). I'll update the patch code for mdlib 2.0 support during the merge to HEAD.
Hide
Permalink
Jerome Mouneyrac added a comment -

Tested on 1.9 and HEAD (head version updated to use mdlib 2.0). Commited on 1.9 and HEAD.
Thanks Iñaki (really appreciate your code improvement and documentation :o)
Thanks Eloy as well

Show
Jerome Mouneyrac added a comment - Tested on 1.9 and HEAD (head version updated to use mdlib 2.0). Commited on 1.9 and HEAD. Thanks Iñaki (really appreciate your code improvement and documentation :o) Thanks Eloy as well
Hide
Permalink
Eloy Lafuente (stronk7) added a comment -

Great Jerome! Kisses, Iñaki! :-P :-D

Show
Eloy Lafuente (stronk7) added a comment - Great Jerome! Kisses, Iñaki! :-P :-D
Hide
Permalink
Petr Škoda (skodak) added a comment -

the $userfields might be better passed through the form contructor as $customdata parameter

Show
Petr Škoda (skodak) added a comment - the $userfields might be better passed through the form contructor as $customdata parameter
Hide
Permalink
Anthony Borrow added a comment -

Jerome - I am closing this because the patch does work; however, I wanted to call your attention to Petr comment that the $userfields might be better passed through the form contructor as $customdata parameter. Peace - Anthony

Show
Anthony Borrow added a comment - Jerome - I am closing this because the patch does work; however, I wanted to call your attention to Petr comment that the $userfields might be better passed through the form contructor as $customdata parameter. Peace - Anthony
Hide
Permalink
Jerome Mouneyrac added a comment -

Thanks, I have a look at that

Show
Jerome Mouneyrac added a comment - Thanks, I have a look at that
Hide
Permalink
Jerome Mouneyrac added a comment -

Ok I updated the code. I use $userfields as _customdata parameter during moodleform declaration.

That the first time I'm working with moodleform. I'll wait for review from Petr before commiting in order to know if it's what he expected.

Show
Jerome Mouneyrac added a comment - Ok I updated the code. I use $userfields as _customdata parameter during moodleform declaration. That the first time I'm working with moodleform. I'll wait for review from Petr before commiting in order to know if it's what he expected.

Dates

  • Created:
    Updated:
    Resolved:
Atlassian JIRA (v4.4.3#663-r165197) | Report a problem
Powered by a free Atlassian JIRA open source license for Moodle. Try JIRA - bug tracking software for your team.