In this patch:

  • New permission: "attach existing images". In addition to the global "Enable Attach Existing" setting, this permission must be for each role that should be able to attach existing images.
  • hook_update for backwards compatibility: Since there was previously no distinction between "attach images" and "attach existing images", this hook_update adds "attach existing images" permission to every role that already has "attach images". This will create an uninterrupted update experience for image_attach users.

See [#72579] for some more background.

Comments

Status: Needs review » Needs work

The last submitted patch, 72579-image_attach_existing_permission.patch, failed testing.

aaronbauman’s picture

Status: Needs work » Needs review
StatusFileSize
new5.62 KB

Now with updated tests.

Status: Needs review » Needs work

The last submitted patch, 72579-image_attach_existing_permission.patch, failed testing.

aaronbauman’s picture

Status: Needs work » Needs review
StatusFileSize
new5.62 KB

getting closer...

Status: Needs review » Needs work

The last submitted patch, 72579-image_attach_existing_permission.patch, failed testing.

joachim’s picture

I'm trying to wrap my head round the permissions this results in....

we have:

- 'attach images' -- functions like a master switch, controls access to the whole image attach fieldset
-- 'create images' -- controls access to uploading a new image
-- 'attach existing images' -- controls access to the select list of currently selected & existing image nodes

So I'm wondering whether someone might say we should have a 'attach new images' permissions instead of only relying in the one from image module, or whether that's just silly.

aaronbauman’s picture

the use case for "attach new images" would be a user who could *only* attach existing images?

joachim’s picture

Well you'd always want to check 'create images' before creating an image node.

But you might want users who can create single image nodes but can't use the upload image attach thing? Though as I think about it, really, why? You could bypass that by creating it first and then attaching it as an existing one...

Ok. So the perms are fine as the patch changes them :)

aaronbauman’s picture

Status: Needs work » Needs review
StatusFileSize
new5.64 KB

try try again...

jan_v’s picture

I've took a look at this patch, and it's so close to what i'm looking for but not in total. I'm working on a large site where upload existing images is disabled, because it would be an endless list of unclear image titles. I was looking for a functionality where unauthorized users can attach an image to a specific node type that hasn't got an attached image.

Is this possible with image_attach? Because it seems to be something really specific to search for, and i keep ending up on this issue.
I also didn't know where to post this, so my apologies if this doens't apply to this issue.

joachim’s picture

> I'm working on a large site where upload existing images is disabled, because it would be an endless list of unclear image titles. I was looking for a functionality where unauthorized users can attach an image to a specific node type that hasn't got an attached image.

I'm not entirely sure what you mean, but yes, you can disable attach existing and allow just upload of a single image per node.

jan_v’s picture

Sorry if my explanation wasn't clear :p (and my english poor).

My site has "places" on it. each "place" has an owner who can edit (thus attach images to it).
I need a functionality where a visitor (not owner, unauthorized, no editting rights) can attach an image to this "place".

An unauthorized user needs some kind of permission to ONLY attach images to the node, and nothing else.

Is that possible with image attach? Because i haven't figured that out.

joachim’s picture

Not entirely sure I understand -- could you either file a support request or better still, post on the forum where more people will see it?
I think the answer is no, as you still need to edit the node to attach.

Scott J’s picture

re #12
Jan, you need editablefields module, along with ImageField, ImageCache and other related projects.

joachim’s picture

Status: Needs review » Needs work

I've rerolled this for whitespace and fuzz, but I've spotted this problem:

+  $subject = ', attach images,';
+  $perms = db_query('SELECT * FROM {permission} WHERE perm LIKE "%%%s%%"', $subject);
+  $replacement = ', attach images, attach existing images,';

What if the 'attach images' permission is first or last in the stored list?

Core does this using regexps on the retrieved permission string -- we should follow that example.

joachim’s picture

Oops. Here's the rerolled patch.

tubby’s picture

the patch in #9 worked.

the problem is, i have allowed users to attach up to 3 images. prior to applying the patch, only 1 image was shown on og/all.

after applying the patch, all attached images show. is there any way to fix this?

joachim’s picture

I can't see how that could happen. The patch doesn't change anything to do with output. Theme functions have changed recently though, so it could be that.

Patch still needs work per my comments in #17.

tubby’s picture

well don't know what to tell you other than it now displays all 3 images on the homepage instead of limiting to one.

joachim’s picture

The patch REALLY doesn't change anything to do with output. And I've just checked it on my test site. There's something happening in your theme; as I said, the theme functions changed at the last beta.

tubby’s picture

yeah uhm...whatever. my theme is fine.

if anyone has any other suggestions that would be great.

i haven't attempted to reproduce it in other areas but right now, i'm having this issue with the organic groups module where more than one image attachment is shown on the /og page after applying the patch in #9 in drupal 6.15

sun’s picture

+++ contrib/image_attach/image_attach.install	21 Aug 2010 09:55:07 -0000
@@ -160,3 +160,27 @@ function image_attach_update_6103() {
+function image_attach_update_6104() {
+  $ret = array();
+  $subject = ', attach images,';
+  $perms = db_query('SELECT * FROM {permission} WHERE perm LIKE "%%%s%%"', $subject);
+  $replacement = ', attach images, attach existing images,';
+  while ($row = db_fetch_array($perms)) {
+    $count = 0;
+    $new_perm = str_replace($subj, $replacement, $row['perm'], $count);
+    if (!$count) {
+      continue;
+    }
+    $query = sprintf('UPDATE {permission} SET perm = "%s" WHERE pid = %d',
+                    db_escape_string($new_perm), $row['pid']);
+    $ret[] = update_sql($query);
+  }
+  return $ret;

This looks much more complex than the existing image_update_6103() -- let's use the same code and adapt it for this update.

+++ contrib/image_attach/image_attach.module	21 Aug 2010 09:55:11 -0000
@@ -200,7 +200,7 @@ function image_attach_form_alter(&$form,
-      $may_attach_existing = variable_get('image_attach_existing', 1);
+      $may_attach_existing = variable_get('image_attach_existing', 1) && user_access('attach existing images');

What's the purpose of the variable now that we have the permission?

+++ contrib/image_attach/tests/image_attach.test	21 Aug 2010 09:55:12 -0000
@@ -97,6 +99,14 @@ class ImageAttachTestCase extends Drupal
+    // Check that a user who may not attach existing images can see the "add" ¶

Trailing white-space.

Powered by Dreditor.

joachim’s picture

> What's the purpose of the variable now that we have the permission?

Turn the entire feature on or off -- if we didn't have the variable, then uid 1 would always see the existing images option even on a site where it wasn't required.

mattwmc’s picture

#9 works for me.

Thanks.