[#MDL-14233] Make bulk user picture upload handle subdirectories in zip files (with patch)

Issue Details (XML | Word | Printable)

Key:	MDL-14233
Type:	Improvement
Status:	Closed
Resolution:	Fixed
Priority:	Minor
Assignee:	Jerome Mouneyrac
Reporter:	Iñaki Arenaza
Votes:	0
Watchers:	0

Operations

Add/Edit UI Mockup to this issue

If you were logged in you would be able to see more operations.

Moodle

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

Created: 06/Apr/08 10:59 PM Updated: 30/Dec/08 12:33 AM

Return to search

Component/s:

Administration

Affects Version/s:

1.9

Fix Version/s:

1.9.2

File Attachments:

MDL-14233_update.patch (2 kB)
2.

uploaduserpicture-handle-subdirs-v2.diff (10 kB)

Issue Links:

Relates

This issue has a non-specific relationship to:

~~MDL-11752~~ Add ability to mass/bulk upload user profile images

URL:	http://moodle.org/mod/forum/discuss.php?d=94182
Participants:	Anthony Borrow, Eloy Lafuente (stronk7), Iñaki Arenaza, Jerome Mouneyrac and Petr Skoda
Security Level:	None
QA Assignee:	Anthony Borrow
Resolved date:	03/Jul/08
Affected Branches:	MOODLE_19_STABLE
Fixed Branches:	MOODLE_19_STABLE

Description

« Hide

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.

All

Comments

Change History

Version Control

Sort Order:

[ Permalink | « Hide ]

Anthony Borrow added a comment - 07/Apr/08 07:52 AM

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

[ Permalink | « Hide ]

Eloy Lafuente (stronk7) added a comment - 27/Apr/08 07:57 AM

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

[ Permalink | « Hide ]

Eloy Lafuente (stronk7) added a comment - 29/Apr/08 02:46 AM

Hi Iñaki....

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

TIA!

[ Permalink | « Hide ]

Iñaki Arenaza added a comment - 29/Apr/08 03:50 AM

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.

[ Permalink | « Hide ]

Eloy Lafuente (stronk7) added a comment - 29/Apr/08 06:01 AM

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

Eloy Lafuente (stronk7) made changes - 29/Apr/08 06:01 AM

Field	Original Value	New Value
Fix Version/s		1.9.1 [ 10240 ]

[ Permalink | « Hide ]

Iñaki Arenaza added a comment - 01/May/08 05:02 AM

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.

Iñaki Arenaza made changes - 01/May/08 05:02 AM

Attachment

uploaduserpicture-handle-subdirs-v2.diff [ 13815 ]

[ Permalink | « Hide ]

Eloy Lafuente (stronk7) added a comment - 01/May/08 06:30 AM

Thanks! Happy loooong-weekend!

Eloy Lafuente (stronk7) made changes - 08/May/08 09:36 AM

QA Assignee

jerome

Martin Dougiamas made changes - 15/May/08 03:02 PM

Fix Version/s		1.9.2 [ 10280 ]
Fix Version/s	1.9.1 [ 10240 ]

[ Permalink | « Hide ]

Iñaki Arenaza added a comment - 26/May/08 05:30 AM

Hi Eloy,

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

Saludos. Iñaki.

Eloy Lafuente (stronk7) made changes - 03/Jul/08 09:01 AM

Assignee

Eloy Lafuente (stronk7) [ stronk7 ]

Jerome Mouneyrac [ jerome ]

[ Permalink | « Hide ]

Eloy Lafuente (stronk7) added a comment - 03/Jul/08 09:01 AM

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!

[ Permalink | « Hide ]

Jerome Mouneyrac added a comment - 03/Jul/08 03:10 PM

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.

Jerome Mouneyrac committed 1 file to 'Moodle CVS' on branch 'MOODLE_19_STABLE' - 03/Jul/08 03:16 PM

~~MDL-14233~~ Make bulk user picture upload handle subdirectories in zip files (with patch)

MODIFY

admin/uploadpicture.php

Rev. 1.1.2.3

(+132 -55 lines)

Jerome Mouneyrac committed 1 file to 'Moodle CVS' - 03/Jul/08 03:35 PM

~~MDL-14233~~ Make bulk user picture upload handle subdirectories in zip files, merged from 19 (updated the code in order to use mdlib 2.0)

MODIFY

admin/uploadpicture.php

Rev. 1.9

(+134 -54 lines)

Jerome Mouneyrac committed 1 file to 'Moodle CVS' on branch 'MOODLE_19_STABLE' - 03/Jul/08 03:36 PM

~~MDL-14233~~ remove some var_dump()

MODIFY

admin/uploadpicture.php

Rev. 1.1.2.4

(+2 -3 lines)

[ Permalink | « Hide ]

Jerome Mouneyrac added a comment - 03/Jul/08 03:40 PM

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

Jerome Mouneyrac made changes - 03/Jul/08 03:40 PM

Status	Open [ 1 ]	Resolved [ 5 ]
Resolution		Fixed [ 1 ]

[ Permalink | « Hide ]

Eloy Lafuente (stronk7) added a comment - 03/Jul/08 06:51 PM

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

Jerome Mouneyrac made changes - 08/Jul/08 02:24 PM

QA Assignee

jerome

[ Permalink | « Hide ]

Petr Skoda added a comment - 09/Jul/08 06:49 PM

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

[ Permalink | « Hide ]

Anthony Borrow added a comment - 10/Jul/08 09:44 PM

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

Anthony Borrow made changes - 10/Jul/08 09:44 PM

Status	Resolved [ 5 ]	Closed [ 6 ]
QA Assignee		aborrow

[ Permalink | « Hide ]

Jerome Mouneyrac added a comment - 11/Jul/08 09:46 AM

Thanks, I have a look at that

[ Permalink | « Hide ]

Jerome Mouneyrac added a comment - 11/Jul/08 11:00 AM

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.

Jerome Mouneyrac made changes - 11/Jul/08 11:00 AM

Attachment

MDL-14233_update.patch [ 14511 ]

Iñaki Arenaza made changes - 30/Dec/08 12:33 AM

Link

This issue has a non-specific relationship to ~~MDL-11752~~ [ ~~MDL-11752~~ ]

All content on this web site is made available under the GNU General Public License, unless otherwise stated.