Problem/Motivation
Currently after doing a bulk upload, users need to click "Edit" on each item to assign field data to each one. This is not so bad, but in some cases it can be a problem. Our site uses organic groups, and users can't edit a file that is not in one of their groups, but because the field editing page is bypassed, the file never goes into one of their groups.
Others may just prefer that all the field edits be done on a single page, for less clicking.
Another use case is when file types have required fields, like alt text for images.
Proposed resolution
Write a patch that opens the multiform file edit page after a bulk upload.
Remaining tasks
Review patch.
User interface changes
A new page during the bulk file uploading process where the field data is set.
API changes
None.
Data model changes
None.
Comment | File | Size | Author |
---|---|---|---|
#33 | 2818683_add_configure_to_media_bulk_upload_info_file-33.patch | 488 bytes | joseph.olstad |
#29 | bulk_field_editting_after_bulk_uploading-2818683-29.patch | 2.06 KB | joseph.olstad |
|
Comments
Comment #2
brockfanning CreditAttribution: brockfanning commentedHere is my attempt at a patch. This relies on 2 other patches, one for Media and one for Multiform:
#951004-219: Allow selecting of multiple media items for a multi value media field in the same dialog
#1889652-3: Possibility to hook into multiple edit form
Comment #3
siramsay CreditAttribution: siramsay commentedthanks for this @brockfanning, works perfectly. bit beyond my understanding of media workings at present so helpful in more than one way.
I used Multiform 7.x-1.x not 7.x-1.x-dev with no problems there, so no need to use dev of that module for this use case.
to your question. Also decide if this should be configurable on/off,
Comment #4
brockfanning CreditAttribution: brockfanning commentedCool @size. You make some good points, description edited.
Comment #5
Proteo CreditAttribution: Proteo as a volunteer commented@brockfanning many thanks for the patch, working perfectly with:
Media 7.x-2.0-beta2
Multiform 7.x-1.1
Plupload 7.x-1.7
And patches for:
Media: here.
Multiform: here.
Comment #6
byronveale CreditAttribution: byronveale as a volunteer commentedI can also confirm this is working, presently with media-7.x-2.0-beta5, multiform-7.x-1.1 with patch, and plupload-7.x-1.7.
Comment #8
joseph.olstadOk, committed to 7.x-2.x dev but have not yet tagged a release for it.
Comment #9
brockfanning CreditAttribution: brockfanning commented@joseph.oldstad: I'd be a little hesitant to commit this patch yet, since it depends on another patch for a separate project. I don't know if anyone has tested it without the multiform patch. (And even if it doesn't break anything, this patch wouldn't be functional until the multiform patch gets committed.)
Comment #10
joseph.olstadcan someone please follow up with RTBC patch here: bug maintainers to commit it
#1889652: Possibility to hook into multiple edit form
As for the other patch, it was committed to 'media' a while back.
Comment #11
byronveale CreditAttribution: byronveale as a volunteer commentedFunny you should mention that Joseph…
Comment #12
joseph.olstadComment #13
joseph.olstadTo make this functionality work you will need the latest release of multiform 7.x-1.2 released a few minutes ago.
Comment #14
brockfanning CreditAttribution: brockfanning commentedGreat work @joseph.olstad, thank you!
Comment #15
joseph.olstadThanks @brockfanning, it was mostly your great work that made this possible. Really leaning on your patches.Spoken too soon, see regression :PComment #16
joseph.olstadThis commit caused a regression with plupload integration.
marking this as major priority. Lets redesign this so that it DOESN'T break plupload compatibility.
see related issue:
#2828728: plupload - Pop-up window doesn't close itself and file uploaded are showed inside
Comment #17
brockfanning CreditAttribution: brockfanning commentedYikes. Strange though, because I also use Plupload and it has been working fine with the patch in #2. I haven't updated our production setup since the recent commits to media, media_ckeditor, and multiform though, so maybe there is some bad interaction going on. I hope to have some time to look into this later in the week, and update our media setup.
Comment #18
siramsay CreditAttribution: siramsay commented@brockfanning had asked if this should be a toggle-able on/off option but I made these arguments taken from above
to your question. Also decide if this should be configurable on/off,
So the issue as per #2828728: plupload - Pop-up window doesn't close itself and file uploaded are showed inside is "by design", however , obviously we need to make this toggle-able as some use case don't need to bulk edit after upload.
Later in the #2828728 thread @interdruper says "The same in my case. Moreover the files are not saved to the File field after closing the last window, so the issue is serious" , i am am using the same set up listed with latest production version of Media and files are saved fine, so possible that is another unrelated issue.
Solution, add this as an option on the admin/config/media/browser page. I guess would need to add a new section/form to that page specific to this functionality.
Comment #19
Proteo CreditAttribution: Proteo as a volunteer commentedHey, just wanted to give my two cents. I've been using the latest beta (8) and Plupload in production in a couple of pretty busy sites, along with other sites in different stages of development without a hitch. The only "problem" is that the bulk editing form comes up without proper padding (just like in the screenshot) but that's a theming problem I guess (we use Adminimal).
Comment #20
siramsay CreditAttribution: siramsay commentedsorry unsure how to format and submit a patch, but this works below to solve the problem.
I set the new variable to true as feels this a good feature.
will look at rolling proper patch tomorrow
Comment #21
joseph.olstad@size thanks for your analysis and code.
I'm not sure though, should the default value be TRUE ? Is it safer to be defaulted to FALSE?
Comment #23
joseph.olstadhmm, something not right in the patch.
@size
in order to create a patch, its quite simple, there's a few ways to do it.
this is the standard way:
now edit the files you want, make the changes, save the files.
if you add a new file that didn't exist previously and you want that part of a patch, its slightly trickier, you'll have to do a search on doing that, I could explain it here but its widely documented on the web.
once you've created the new patch, upload it.
If you're stuck on windows and you don't know how to get these tools, simply download 'gitbash' , its a command line tool that has all these commands built in.
Comment #24
joseph.olstad@size
to apply your patch, you can do it two ways, if you have the .git repo info you can use
git apply patch_name.patch
If you do not have the .git repo you can use the 'patch' utility
wget https://www.drupal.org/files/issues/bulk_field_editting_after_bulk_uploading-2818683-21.patch
do a dry-run test, does not actually apply the patch
patch -p1 --dry-run < bulk_field_editting_after_bulk_uploading-2818683-21.patch
if dry run works, do this:
patch -p1 < bulk_field_editting_after_bulk_uploading-2818683-21.patch
to reverse the patch, simply add the
-R
flagdry-run in reverse
patch -p1 --dry-run -R < bulk_field_editting_after_bulk_uploading-2818683-21.patch
reverse patch for real
patch -p1 -R < bulk_field_editting_after_bulk_uploading-2818683-21.patch
Comment #25
siramsay CreditAttribution: siramsay commented@joseph.olstad thanks for the info, seem all pretty straight forward as I am familiar with GIT.
default true or false is debatable but as stated "on file/add page of Media Bulk Upload after clicking next it takes you to the admin/content/file/edit-multiple/XXX XXX page , this is not configurable " I always felt that it was missing from the bulk upload and when this was bought up and @brockfanning created this patch and then you commited thought it was the right direction.
I guess on install/enable we could return a configuration screen or a least a message with a link to the config page so as people know it is an option. some may also argue it should be configurable per media browser field.
I open to either way as can always toggle on or off site wide and look at ways to make it better in the future on as needed and time allows schedule.
will see if I can trouble shoot patch test fails and re-submit but info is limited on why it is failing.
Comment #26
joseph.olstadI'm trusting your advice on the true or false on this one.
Any ideas on the 6 fails for the patch?
Comment #27
joseph.olstad✗
testFilesBrowserSort
fail: [Other] Line 564 of sites/all/modules/media/tests/media.test:
✗
testFilesBrowserLibrary
fail: [Other] Line 592 of sites/all/modules/media/tests/media.test:
✗
testFilesBrowserMyFiles
fail: [Other] Line 840 of sites/all/modules/media/tests/media.test:
✗
testBrowserSettings
fail: [Other] Line 705 of sites/all/modules/media/tests/media.test:
✗
testMediaBrowserHooks
fail: [Other] Line 496 of sites/all/modules/media/tests/media.test:
✗
testMediaBrowserWebTab
fail: [Other] Line 114 of sites/all/modules/media/modules/media_internet/tests/media_internet.test:
Comment #28
brockfanning CreditAttribution: brockfanning commentedSince this is a change in behavior (an edit screen where there previously was none) it might make more sense to default to FALSE. I think that the addition of this feature is the reason for #2828728: plupload - Pop-up window doesn't close itself and file uploaded are showed inside. It's not actually a regression - just an unwanted feature.
We should probably prefix the variable with "media_" so that it's clear what module the variable belongs to - like "media_bulk_upload_edit".
I also believe there is a syntax error in the patch in this line:
(the comma should be &&)
Comment #29
joseph.olstadThanks @brockfanning
this is the new patch with those two changes made,
fixed the syntax error (, instead of &&)
and renamed bulk_upload_edit variable to media_bulk_upload_edit
Comment #31
joseph.olstadfixed in dev branch , beta11 will be comming soon, maybe tomorrow or a few days from now.
Comment #32
siramsay CreditAttribution: siramsay commentedthanks everyone, seems like a small syntax error can throw tests out and sensible suggestion on variable naming too.
as per my suggestion it does appear on install a configure link is present in Media module.info file
maybe we could add it to media bulk upload? this only works on install/enable however but I guess notes will be included with the next release too.
Comment #33
joseph.olstad@size , is this what you're suggesting? (SEE PATCH)
Comment #34
siramsay CreditAttribution: siramsay commented@joseph.olstad yes, I think that would be a good addition to the whole patch
Comment #36
epersonae2 CreditAttribution: epersonae2 commentedIs this actually fixed? Looks like it left off with some patches but nothing in even a dev version?
Comment #37
brockfanning CreditAttribution: brockfanning commentedI think everything made it in, but you do need to have the latest version of Multiform in order for it to work.
Comment #38
joseph.olstadadded last minor change to this issue,
Comment #39
joseph.olstad