Having multiple image attachments in this version is a good feature, but to provide backward compatibility with existing installations and setups where only one image attachments was allowed, could there be an option to set/control the allowed number of image attachment per node.

As discussed here:- http://drupal.org/node/580864

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

joachim’s picture

I'm afraid this is going to be a low priority for me, and I assume the other maintainer too.
If the community provides and tests a patch, then it'll get in, but I don't think we should hold a 1.0 release back.

In the meantime -- if you're willing to get your hands dirty ;) -- you could do this with hook_form_alter in a custom module:
- set a message on the form to warn about the limit
- add a form validation function to test the number of images selected.
You could just hardcode the limit in your own code.
Parts of this approach could be turned into an eventual patch.

Josie_h’s picture

Hi joachim,

I can accept that it is probably low priority and perhaps I'm alone on this, but the lack of backward compatibility has really messed up my site configuration. However, low priority it must be I guess.

If I was any use at programming and patches etc., I would gladly contribute, but I just haven't got a clue about such things.

As per the original referenced thread, can you please tell me if there is a version I can safely go back to and where/how I can get it?

joachim’s picture

Really, unless you backed up your data, you can't. The storage has changed.
If you backed up, then check the changelog for when we moved to multiple image attachments and go to the version one before that.

As for coding, this really isn't much work, just that I'm snowed under at the moment. An hour to do the form stuff for your site. For a proper patch with a UI and everything, an extra hour perhaps. This type of small customizing with the hook system is how I got started with programming on Drupal, by the way -- the documentation is good and the help on IRC is plentiful :)

Josie_h’s picture

I'm really sorry to go on about this, I'm trying to help myself, but I'm stuck.

I've looked at the changelog as you suggested and from what I can glean it seems that multiple image attachments were implemented in 6.x-1.0-A6, so I need a version prior to this, but where/how can I get a previous version?... I didn't keep any of the old downloaded versions!

I totally realise that coding patching etc comes easy to people like yourself and my great thanks that it does, unfortunately it is totally beyond me, so my preferred option is to rebuild my entire site again (as long winded and time consuming as it may be) with the earlier Image module.

Problem is where do I get the earlier version of Image module?

joachim’s picture

Status: Active » Needs work
FileSize
1.24 KB

The old versions are available on the project page -- 'all releases'.
The problem is rather your database -- if you didn't back it up prior to upgrading then you can't go back to the old version of the code -- it won't run with the updated data!

Really, re-creating your whole site is insane! :(

Try this patch.
It's very basic and it only covers the case of limiting to one image, which is what you need on your site.

I'll leave it up to someone else to work on this further -- it needs a UI and probably some more graceful handling of various cases.

Instructions on how to patch are in the drupal docs! and people on IRC can help too :)

Josie_h’s picture

Joachim,

Thanks, I found the old versions where you suggested.

I genuinely appreciate your creating a patch for me, thank you very, very much.

I can also understand your frustration at someone like me who hasn't got a clue about coding etc, but it is really beyond my grasp, I have tried patching (after reading the docs) before and couldn't understand it. I can play guitar, keyboards, sax and rebuild engines and gearboxes and I often think these things should be easy for others to do and then see them give up :( we're all different I guess.

It eventually becomes tiresome for other people when I keep asking questions, (I didn't have the best experience when 'sun' responded to one of my previous problems in this forum), so now I try to get on with it myself. I'll try your patch, but I know I will get confused and finish up re-creating the whole site :(

Anyway, all of this is way off-topic, so if you are able to delete this thread, I would like to suggest (after re-thinking and considering other peoples use of the module):-

How about (if/when possible) "having the number of attached images selectable by role/permissions"?. This would give backward compatibility.

I don't know if that sounds logical to you, if so perhaps you could change this entire thread to this simple feature request.

Thank you again for all your help.

joachim’s picture

Changing the number of allowable images requires the same mucking about with the form, no matter what the rule is -- permission, per-type setting, etc.
Do have a go at the patch -- the documentation at http://drupal.org/patch/apply is very good, and it you get stuck, it can always be improved.

(I play the piano too by the way :)

Josie_h’s picture

Joachim,

Play me something in a minor key please A-minor will do :)... I failed and I'm depressed :( ;)

I know I come across as a complete dummy, but I still couldn't grasp the patch instructions. My only saving grace was another user at the bottom of the FAQ was in the same boat.

Anyway, I've deleted my DB and re-installed Drupal, I need to sit down and re-create everything over the weekend, but no worries I was expecting to have to do it. I just have to make sure to get the version of Image module prior to the multiple image attachments or I'l end up doing it all again.

BTW: I wasn't suggesting that my previous comment of having the number of images linked to roles was an easier workaround, I just thought it might be better to delete/modify this thread as a feature request along those lines.

Thanks again for you help, your patience and for trying to get me out of the fix the easy way, it was me that failed... but everything was sincerely appreciated.

Best wishes.

joachim’s picture

Oh well :/

According to the changelog, the change to multiple images happened for Image 6.x-1.0-ALPHA6, 2009-08-27.
So if you download alpha5 you will have the old system -- check image attaching first when you install it to be sure.
It's a shame though, as you don't get the cool gallery stuff made with views.

What did you get stuck on or find hard with the patching documentation? If you file a bug on the docs explaining what you didn't understand, that page can be improved.

Are there any Drupal meetups in your area? Informal user groups or a drupalcamp? If you go along to one of those I'm sure someone will walk you through patching step by step. I used to find patching scary and impossible, and now I'm a module maintainer!!! ;)

Best of luck with rebuilding your site.

Josie_h’s picture

Good morning Joachim,

Yes, I've downloaded the ALPHA5, haven't installed any Image module yet though, I'm working through the basic reconstruction first.

It puts me in a bit of a spot asking about where I got stuck with the patching... I should really say I crashed and burned on the first instruction in the FAQ!, which makes me feel totally stupid :(

In reality, where the FAQ starts by talking about command line instructions, I've never been able to figure out whether it is talking about issuing the commands on my local computer or on the hosting server. That is really obvious to guys like yourself I know, but to this novice at least this isn't explained. Then where it goes on to mention the patching utilities (UnxUtils and Cygwin) once again I'm not sure if I should be running these locally or if my host should have them on the server. My confusion must sound very dumb to you, but I feel that somehow the FAQ is missing some basic info or assuming more knowledge than people like me possess. Oh and I'm also running ubuntu on my PC which, as you know, supports lots of command line usage, so once again I'm probably getting confused about where to run the patching process from. I'll take some solace in you once finding patching scary and impossible... makes me feel a little better.

I don't know of Drupal meetings where I am (Spain), but if there was I wouldn't like to get involved in Spanish tech speak ;/

Hopefully one day my feature request might be considered and I can start using the Image module updates and improved features. Out of curiosity, do you think it is a valid feature request (albeit low priority) or is this a 'one user' type of issue?

joachim’s picture

I think it's a valid feature request, since CCK allows this too.

As for patching -- you patch wherever you like.
I would recommend you patch on a local development copy, so you can test stuff without breaking your site. Or you can just patch in a random folder that holds the files. Patch is just a command that says 'apply THESE changes to the files where I am'.
If you're running Ubuntu, then patching is very simple:
1. open terminal
2. cd to the module folder of the module you want to patch (you can drag a folder from Nautilus to the terminal to get its path)
3. save the patch file to this folder too
4. issue the patch command.

You can either update that documentation page with this, or file an issue requesting it be improved. It really really needs to be clear and understandable by ordinary users just like you who aren't techheads!

Josie_h’s picture

Hi Joachim,

Sorry for the dalay in replying, I was a little embarrassed as I still couldn't work out patching, but I have finally done it (a different module) :))

Just to give you an idea (or perhaps refresh your memory ;) ) of how a newbie gets confused over such things:-

1. when I clicked on a listed patch on the Drupal site, it opened in text editor (I was kind of expecting it to 'download'/'save' to my pc, so I didn't know what to do with it!). Eventually decided to save to my pc what the text editor opened to a file with a .patch extension. I then put this .patch file plus the module to be patched into the same directory as your said.

2. I ran Ubunu's Terminal and changed to the directory containing the files and then from Terminal just typed 'patch' . At this point Terminal just kept sitting there and not giving any message or returning to the command prompt. If I closed Terminal it warned that a process was still running. I dug around the web and came up with the command 'patch < filename.patch' and viola, it worked as far as I know (at least the module properties changed to reflect the new modification date and Terminal returned to the command prompt.

As mentioned this must seem so basic to you guys, but it has had me stumped many times and many of the FAQ's/doc's for Drupal seem to skip the basics and cater for an assumed level of knowledge which is really frustrating for non-techies such as me.

Anyway, thanks to you, I persevered and have (hopefully) one thing under my belt. :)

Can you tell me, as this thread as drifted way off course, is there a way to delete it and start again, so that it is not so off-topic for other users?

Thanks again.

joachim’s picture

It's fine to leave the thread as is :)

Glad to hear you've got it worked out.
I haven't time to look at documentation at the moment, but you can either edit those pages yourself to improve them as you see fit, or file an issue on the Documentation component with suggestions -- start by listing what you've posted above.
The more people are able to patch, the more potential testers we have, so improving the patching documentation is very important :D

skyredwang’s picture

Version: 6.x-1.0-beta3 » 6.x-1.x-dev
Status: Needs work » Active

I upgraded to the latest dev, and I am also requesting this feature which let the admin to set a number for attached images in the Content Type Edit mode

joachim’s picture

See my comment #1.
I am having a hard enough time getting a 1.0 stable release of this module out without looking at totally new features.
My policy on this issue is: if someone else provides a patch, and one other person tests and confirms, then I will test too and commit it. I may put more work into this if people wanting this feature help test critical patches that are holding up the 1.0 release. That, or ply me with cake or booze ;)

jonathan1055’s picture

Version: 6.x-1.x-dev » 6.x-1.0-beta3
FileSize
4.77 KB
196.83 KB

Hello Josie_h and skyredwang,

I thought this would be a good enhancement to add, so I have taken Joachim's basic patch and added the admin settings UI functionality required and improved the validation. For admin, when editing a content type, the Image Attach fieldset now includes a drop-down list to specify the maximum number of images that can be attached to that content type. The default is unlimited, to match the current functionality of beta3, but you can select from a range of numbers I've specified - see attached screen-grab. The checking for this limit is done prior to the processing which uploads an image. If the limit is exceeded then any attempted upload is not done and a message is given and an error set in the form. Likewise, if the user chooses too many images from the list of existing uploaded images an error is set.

I had originally allowed a free-form text entry field to define the maximum number, but this required more validation to make sure the text entered was numeric and positive, and it also required an explanation to enter 0 for 'unlimited'. Then I changed it to a selection list to avoid this validation. If you think I have not chosen enough numbers or it should be a free entry field let me know.

I expect there will be a few tweaks to the patch, so give it a try and let me know what ideas you have. It is created against beta3 but you will probably get a few warnings about offsets when applying the patch, as the code line positions may have changed a bit.

Jonathan

skyredwang’s picture

Status: Active » Needs review

thanks for your work, jonathan1055.
Let's test it!

joachim’s picture

Thanks for working on this jonathan! :D

Given the eventual D7 upgrade path for image_attach will be based on CCK nodereference, we should provide the same range of numbers as CCK.

Here's an updated version with that change plus a small UI text tweak.

However, it's worth noting (and remembering for eventual migration to D7) that we are not working the same as CCK for the unlimited and single-value cases:

// CCK 6:
'#options' => array(1 => t('Unlimited'), 0 => 1) + drupal_map_assoc(range(2, 10)),
// D7 Field API
'#options' => array(FIELD_CARDINALITY_UNLIMITED => t('Unlimited')) + drupal_map_assoc(range(1, 10)),
where the FIELD_CARDINALITY_UNLIMITED constant is -1.

skyredwang’s picture

Version: 6.x-1.0-beta3 » 6.x-1.0-beta4
Status: Needs review » Reviewed & tested by the community

Tested out #18 against Beta4. It works as expected. One minor suggestion that I have is if the limit is set to 1, and then if a user decides to upload a new image, then automatically ignore/remove the selected existing image rather than telling the user that the limit is 1.

jonathan1055’s picture

Status: Reviewed & tested by the community » Needs review
FileSize
4.7 KB

Skyredwang,
Thanks for testing the patch.
Yes that is an idea, but it might lead to inconsistencies of expected behaviour (to use a fancy phrase). Eg, why should this deleting only be restricted to when the limit is set to one? But then you get the problem of if there are two images and the limit is two, and the user attempts to upload a third image, which one should be automatically deleted? The idea is worth thinking about, though.

Joachim,
Yes your improvements using drupal_map_assoc are much better, and I have also noted the other minor tweaks you made. There is just one thing I think should be changed, that when setting a maximum, only having 1 to 10 seems a bit low. I would suggest we add the numbers from 20 up to 100 by 10, ie change:

      '#options' => array(0 => t('Unlimited')) + drupal_map_assoc(range(1, 10)),

to

      '#options' => array(0 => t('Unlimited')) + drupal_map_assoc(range(1, 10)) + drupal_map_assoc(range(20, 100, 10)),

The attached patch has all the changes as in #18 but with this extra range added.

Due to both of the above, I have put the status back to 'needs review'

Jonathan

joachim’s picture

CCK nodereference only has 1-10; if we offer different numbers here then the eventual migration to CCK noderef (on Drupal 7) will result in a loss of this feature.

jonathan1055’s picture

Joachim,

Oh, yes I see your point, image should not give more values than CCK offers. Hence I have raised a cck feature request #680546: Add a custom 'number of values' for 'multiple / multi-value' field. (e.g. nodereference, imagefield) and we'll see what develops there, then we can decide what to do. You could implement the patch in #18 anyway, then I'd be happy to make another, if and when a CCK update is done.

What do you think about Skyredwang's suggestion to automatically detach/delete the image when uploading a new one, if the limit is one. Having thought about it more, I do not like the idea. Automatically detaching or deleting anything requires more work because the user may do it accidentally. You'd need to provide a 'are you sure' pop-up and validate the response, etc. Multiple opportunities for things to go wrong. It is fine how it currently works, ie the user has to detach their one image first before uploading a new one. It is safe and easy, and does not cause too much extra work for the user.

Jonathan

joachim’s picture

Status: Needs review » Postponed

I don't like the idea of automatically removing an image when attaching a new one to remain within the count. Firstly, it's changing data without telling the user -- which was a major WTF for this module back when it was limited to one attached image per node, as you'd upload a new one and lose the old one. Secondly, like you say, it's making a special case: why not do this for 2,3, etc -- and then, how do you decide which image gets pulled? Thirdly, it's more work :p

However, I think we should warn the user about how many attach slots they have remaining on the edit form, so they don't get a nasty surprise when they try to save.

This involves delving into the tangled mess that is image_attach_form_alter(). So I am going to mark this postponed until we fix that: #680644: Tidy up nest of conditions in image attach's hook_form_alter. I've made a patch there already -- please test! :D

skyredwang’s picture

I agree with you two, and the last suggestion by Joachim was great--1.) if a user try to select too many images (greater than the limit) from the existing image list, a warning should come out. 2.) If the number of selected images is equal to the limit, but the user is trying to upload a new one, then the warning should come out, too.

joachim’s picture

Status: Postponed » Needs review
FileSize
6.29 KB

That other issue is now fixed.

So here is a new patch. Changes since #18:
- added a note on the attaching node form saying 'You may attach up to COUNT images.'
- tidied up the use of format_plural on the validation a bit.

skyredwang’s picture

Status: Needs review » Needs work

when apply #25 against the original Beta4 version:
error:

patch < 581174.image_attach.limit_number.5.patch 
patching file image_attach.module
Hunk #1 succeeded at 147 (offset -1 lines).
Hunk #2 FAILED at 215.
Hunk #3 FAILED at 227.
Hunk #4 succeeded at 312 (offset -22 lines).
2 out of 4 hunks FAILED -- saving rejects to file image_attach.module.rej

when apply #25 against the version after #18 patch
error:

patch < 581174.image_attach.limit_number.5.patch 
patching file image_attach.module
Reversed (or previously applied) patch detected!  Assume -R? [n] y
Hunk #1 succeeded at 147 (offset -1 lines).
Hunk #2 FAILED at 208.
Hunk #3 FAILED at 219.
Hunk #4 FAILED at 317.
3 out of 4 hunks FAILED -- saving rejects to file image_attach.module.rej
joachim’s picture

Status: Needs work » Needs review

You will need to apply this to CVS HEAD, not beta 4. Patches are always against CVS, not releases. Also, they are not cumulative. A patch is always assumed to be against CVS HEAD, as itself -- it's exceptions to this that would need to be stated.

The -dev release is updated fairly soon as cvs commits, so that should be ok.

skyredwang’s picture

Status: Needs review » Reviewed & tested by the community

Thanks for the tips on CVS. #25 patch is tested, works well. Please commit to the release.

On the other hand, it might be better if we could include some javascript on the node/edit to warn users that the number of attached images already hit the limit, so don't upload new ones; rather than the current workflow that for the same scenario , users upload a new image first, then receive the error msg.

jonathan1055’s picture

I also tested the patch in #25 and it is good.

I suggest Joachim commits this patch as it is, and we don't spend any more time on it. It is working well, it is simple, and we don't want to add javascript complications to what is essentially a fairly small part of the whole image module. I am not criticising your suggestion, just saying we should spend our efforts in proportion to the problem and there are bigger more important things to do.

Let's move on to focus efforts on testing things which are holding up a #486546: Image 1.0 release.

Jonathan

joachim’s picture

Status: Reviewed & tested by the community » Fixed

I agree with jonathan -- this works nicely, let's not get too fancy. I think the note about the limit that is on the form is sufficient. Users should read instructions! ;)

Committed to CVS:

#581174 by jonathan1055, joachim | skyredwang: Added ability to limit the number of attached images.

The -dev release will reroll itself shortly.

Thank you to all for your hard work on this!

joachim’s picture

I think this warrants a new beta release! :D

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.