| Project: | Image |
| Version: | 6.x-1.0-beta4 |
| Component: | image_attach |
| Category: | bug report |
| Priority: | critical |
| Assigned: | Unassigned |
| Status: | closed (fixed) |
Issue Summary
Hi,
I have just updated to Drupal 6.14 and also Image module 6.x-1.0-b3.
The first problem I had was with the Image module update reporting:-
'Failed: ALTER TABLE {image_attach} ADD 'weight' INT NOT NULL DEFAULT 0'
I did some searching and found a 'similar' report on an earlier issue which implied that this wasn't a problem and the update would still have been successful, so I didn't worry about the error. However, I have now found an issue that I don't know if it could be related either to that error or due to something else or even the functioning of Image has been changed.
What I'm experiencing now is:- when a user creates content where attaching images is allowed there is a scroll box of Attached Images where a user sees and can select from images previously uploaded by other users. I suppose this could be a valid feature, but I have got Attach Existing Images disabled. Is this a bug or have I missed another place to turn existing images off?
Second problem (I don't know if it's related so I wont open another issue):- before the updates I had it set to only allow 1 image to be attached to content. Since the update a user can upload/attach multiple images to a node. This might be me, but I have looked an looked but I can't find the option to limit the number of images anymore.
Thanks for any help/info.
PS: As mentioned, I don't know if these are related. Please split the message if I'm wrong.
Comments
#1
1. The weight thing can be ignored, IRRC. Check you actually have a weight column on your table!
2. Sounds like a bug, but I've just given it a spin on my test site and it all works as expected. You should see in that box the list of currently attached images only, so you can unselect some.
3. There's currently no limit on the number of images. You could file a separate feature request for that, but TBH neither of the current maintainers has much time, so it'll have to be a community effort -- if people write and test a patch it'll go in.
#2
Hi joachim,
Thanks for the reply.
1.Understood, no problem.
2.I can imagine this scroll box with existing images being present if I hadn't disabled the Attach Existing Images option, but I have disabled that option. Not sure if I was clear in my original message, but this scroll box with the list of images is visible on the entry screen for a new (fresh) content for any user. I'm not talking about a user unselecting an image from an existing node. Sorry my english is not 100%, am I making it clearer?. The main point I guess is that Attach Existing Images is disabled and users are still seeing the list and able to select those.
3. This 'might' be me. Before I upgraded the Image module I deliberately skipped a version because of problem that version had, so the last version I was using seemed to only allow 1 image to be attached... was this correct?. I was happy with that as I was using Image attach so that role permissions allowed some users to attach a single image and then using Node Images to allow another group of users to attach multiple images. Is multiple images a new feature over the last couple of versions and if so is the any way to limit the number of images that can be attached?. Seems (to me) like quite an important option really or there is no backward compatibility with previous versions and also a user could upload/attach 100's of images.
#3
Just change the title a little.
#4
Right, just tested, and yes, that's a bug.
For 3, please file a feature request. I honestly hadn't considered that as a use case, given the clamour to allow more than 1 image. You could fix this in a quick and hacky way with a validation handler in a hook_form_alter...
#5
Thanks joachim,
For 3:- My thoughts are... yes there always seemed to be a desire to have multiple images and I can see that as a valid and good feature. The problem arises, as you realise, when an existing installation is configured to use just one image and there is no way with this version to have this backward compatibility.
I'm not sure I know how to open a feature request or that my descriptions are good enough!... I'll give it a try.
I'm definitely no good at doing hacky things or patches (I've tried in the past and failed miserably). Can you tell me if there is a safe way to go back to a previous version of the Image module and if so which version would function in this way and where/how can I get it?
#6
Same problem here after updating to beta3, all images shown in the box. Could you describe more precisely how to fix it?
#7
The module has a big and complex form_alter which inserts the attaching form.
You'd need to wrap the bit that adds the existing image selection box in an if block, basically.
If you're able to provide a patch, please do :)
#8
Please could you test this patch.
#9
Hi,
I am also using Drupal 6.14 and also Image module 6.x-1.0-b3
After applying the patch, the problem stays unchanged as you can see on the attached image.
All the images previously uploaded by other users are visible to those who also want to attach an image while they create a node
Thanks
#10
Very strange. Works perfectly for me. Sure you applied the patch properly?
#11
Coming from #674782: Image attach settings are ignored; for me, the patch works well!
Thanks, Joachim, and greetings, -asb
#12
Great! Setting RTBC so I remember to commit later.
#13
Had to reroll due to this commit: http://drupal.org/cvs?commit=308056
Committed the attached patch having checked the only difference from the old patch is the string update from that earlier commit.
#580864 by joachim: Fixed nodes with no images not respecting the 'don't attach existing images' setting.
Thanks for testing!
#14
Hi,
sorry, the patch from #13 fails when applied against a (fresh!) 'image' 6.x-1.0-beta4:
$ patch < 580864-2.image_attach.fix-all-images-on-new-nodes.patchpatching file image_attach.module
Hunk #1 FAILED at 217.
1 out of 1 hunk FAILED -- saving rejects to file image_attach.module.rej
Contents of image_attach.module.rej is:
$ cat image_attach.module.rej
*************** function image_attach_form_alter(&$form,
*** 217,242 ****
);
}
}
-
- $form['image_attach']['iids'] = array(
- '#type' => 'select',
- $value => empty($node->iids) ? NULL : $node->iids,
- '#multiple' => TRUE,
- '#size' => 6,
- // Title, options and description are set just below.
- );
-
- // User may attach already existing images: show a selection box containing all images.
- if (variable_get('image_attach_existing', 1)) {
- $form['image_attach']['iids']['#title'] = t('Existing images');
- $form['image_attach']['iids']['#options'] = _image_attach_get_image_nodes();
- $form['image_attach']['iids']['#description'] = t('Choose existing images if you do not upload a new one.');
- }
- // User may only upload new images: show a selection box containing only attached images.
- else {
- $form['image_attach']['iids']['#title'] = t('Attached images');
- $form['image_attach']['iids']['#options'] = _image_attach_get_image_nodes($node->iids);
- $form['image_attach']['iids']['#description'] = t('You can remove a previously attached image by unselecting it.');
}
// User may create images, add upload form elements.
--- 217,246 ----
);
}
}
+
+ // Only show selection box of image nodes if the user may attach some,
+ // or if there are existings ones that may be removed.
+ if (variable_get('image_attach_existing', 1) || count($node->iids)) {
+ $form['image_attach']['iids'] = array(
+ '#type' => 'select',
+ $value => empty($node->iids) ? NULL : $node->iids,
+ '#multiple' => TRUE,
+ '#size' => 6,
+ // Title, options and description are set just below.
+ );
+
+ // User may attach already existing images: show a selection box containing all images.
+ if (variable_get('image_attach_existing', 1)) {
+ $form['image_attach']['iids']['#title'] = t('Existing images');
+ $form['image_attach']['iids']['#options'] = _image_attach_get_image_nodes();
+ $form['image_attach']['iids']['#description'] = t('Choose existing images if you do not upload a new one.');
+ }
+ // User may only upload new images: show a selection box containing only attached images.
+ else {
+ $form['image_attach']['iids']['#title'] = t('Attached images');
+ $form['image_attach']['iids']['#options'] = _image_attach_get_image_nodes($node->iids);
+ $form['image_attach']['iids']['#description'] = t('You can remove a previously attached image by unselecting it.');
+ }
}
// User may create images, add upload form elements.
Should this patch be applied against dev or HEAD?
Thanks & greetings, -asb
#15
Patches are always on CVS HEAD. -dev should usually be in sync with that (there may be a 5 minute delay just after CVS has been updated).
The reason it fails on beta 4 is the same reason my earlier patch fails on HEAD: this line in CHANGELOG
#662252: image_attach form text should be plural by joachim: Changed description text in image attach form to be plural.
I changed something in that part of the code for another issue. This is the problem that happens when patches lie around for too long -- I posted the patch on October 4, 2009 - 11:20 and none of the users reporting the problem tested it.
#16
As 'the' original reporter, I suppose that comment is directed at me.
Fact is joachim... this is what happens when a maintainer doesn't consider the whole thread!.
There were 3 issues in the original thread and I asked if they should be split and received no response. Later in the thread I was told to open a feature request (which I did 18th September). That feature request remains open (needs review) and precludes me from using (or testing patches on) the latest version.
Not everything is a user fault!
#17
@Josie_h:
Please do not start to beat Joachim, he is one of the most involved and helpful coders we have at d.o!
And please understand that the status "needs review" is mostly targeted at users like us - it's our job to at least test patches, if we can't help with coding, and to structure and update issues. That's the least we can do.
@joachim:
Please excuse my ignorance, but it's really hard to keep track for us non-developers which module allows patches against the latest stable release, or only against HEAD, or even only against the D7 release (like Drupal core). I'll try to improve ;)
After upgrading to image 6.x-1.x-dev (2010-Jan-06), the patch from #13 applies like a charm, indeed!
Thanks & greetings, -asb
#18
[q]Please do not start to beat Joachim, he is one of the most involved and helpful coders we have at d.o!
And please understand that the status "needs review" is mostly targeted at users like us - it's our job to at least test patches, if we can't help with coding, and to structure and update issues. That's the least we can do.[/q]
You misinterpret I'm afraid asb.
No one is trying to 'beat' joachim or anyone else, I am merely explaining why, as the original poster, I never tested the patches, which obviously needed saying as you confirm that it is us (as users) who should be doing so.
The fact that a feature request 'needs review' should not be taken out of the context, in as much as that was the direction advised to be taken.
As stated, I feel happier and safer with a previous version, but I am glad the issue is resolved for you.
#19
Sorry to be grumpy. This module has been inches from a proper 1.0 release for months, and I write patches fairly quickly after bugs are reported, only for them to sit there for ages untested. :(
@asb: I'm surprised to hear of modules that patch against a stable release. That's just weird. Would it help if I added a mention to the project page of what branch patches should be on?
#20
This is getting off topic...
People in the issue queues are patching anything that doesn't run away quickly enough; please keep in mind that we (users) having often to deal with de-facto-abandoned or not-so-well maintained modules, where dev releases do not even exist, or where ports to D6 live only in the issue queue. We have a really, well, multi-dimensional ecosystem here.
A mention on the project page would help non coders like me, but probably spam the already cluttered project page for everybody who knows already about such conventions. I'm not sure if next someone then requests a description of how to apply a patch...
And now back to getting proper 1.0 release out ;-)
Greetings, -asb
#21
Automatically closed -- issue fixed for 2 weeks with no activity.