Block or filter to display photos by tag
milkshake - March 3, 2007 - 14:20
| Project: | Flickr |
| Version: | 6.x-1.x-dev |
| Component: | Code |
| Category: | feature request |
| Priority: | normal |
| Assigned: | paul.booker |
| Status: | patch (code needs work) |
Description
... would be cool, even cooler would be:
- Chooses tags based on node taxonomy, or directly from node content
- Option to limit tag search by default account, user, or no limit (all of Flickr)

#1
Hello ,
This looks to be the first instance of this kind of feature request .
So far i have add the feature to the flickr module so that you can add two flickr blocks to your site which shows public random/recent tagged content . Please see http://paulbooker.co.uk/?q=node/26 if you want this feature now.
I am going to work on generalising these changes further so that you can add any number of blocks that shows public random/recent content on flickr that's tagged with a collection of keywords this would then allows you to in conjunction with og_block_visibility to specify flickr blocks as described above for individual groups which i think will be a nice feature .
#2
Hello ,
Currently the flickr module is constructed such that you can enable one of a predefined collection of blocks with each implementing a singular feature such as a block that shows your recent photos or a block that shows random public photos with no option to assign tags anywhere.
With the above in mind i have modified the flickr module to address these two shortcomings (please login @ http://paulbooker.co.uk/?q=flickr-roadmap to download ) so that now you can create any number of flickr entries and specify whether the photos from flickr are to be from your account or from all accounts , whether to show recent or random photos and if photos should be tagged you can also specify a list of comma delimited tags.
So my proposal is that future development of the flickr module should go down the path of 1 .creating a flickr entry and then 2. implementing that entry in one of several ways ie a block,..
I would really appreciate some feedback .
Regards Paul
#3
Paul,
Could you please post a patch to this issue? Much easier for us to give feedback this way. Sounds like a cool idea.
Andrew
#4
Hi Andrew ,
I have included the patches as requested .
Please let me know if we can continue to develop the flickr module along this path .
Regards Paul
#5
i just glanced at these patches but it looks like you might want to read through the drupal coding standards to get a better idea of how to format your code to that it conforms with what's already there.
#6
Hi Drewish ,
Thanks, i read through the drupal coding standards and made the necessary changes to the patches .
Paul
#7
Hi Drewish ,
Thanks, i read through the drupal coding standards and made the necessary changes to the patches .
Paul
#8
Hi Drewish ,
Thanks, i read through the drupal coding standards and made the necessary changes to the patches .
Paul
#9
Hi Drewish ,
Thanks, i read through the drupal coding standards and made the necessary changes to the patches .
Paul
#10
Hi Drewish ,
Thanks, i read through the drupal coding standards and made the necessary changes to the patches .
Paul
#11
Paul,
I generally like the idea of being able to create as many blocks as you want instead of using pre-defined ones. A couple things from scrolling through the patch:
1. You are commenting out the if ($maycache) conditional in flickr_menu. Please don't do that :)
2. This code doesn't make sense to me, why are you creating an empty array that you don't seem to use?
<?php
if ($flickr_entries == "" ) {
$flickr_entries = array() ;
$flickr_entries[] = array() ;
}
array_push($flickr_entries , $flickr_entry) ;
variable_set('flickr_entries',$flickr_entries ) ;
?>
3. Does this lose the flickr photoset block functionality?
4. How does one delete an entry? Seems like this could get messy with everything saved in one variable.
Thanks for doing this work.
Andrew
#12
Hi Andrew ,
1. You are commenting out the if ($maycache) conditional in flickr_menu. Please don't do that :)
.. I have submitted a more recent patch that corrects that , thanks
2.This code doesn't make sense to me, why are you creating an empty array that you don't seem to use?
.. Yep , i could correct that in a further revision
3. Does this lose the flickr photoset block functionality?
.. I have never used this functionality , this would need looking at
4. How does one delete an entry? Seems like this could get messy with everything saved in one variable.
.. This needs to be added .
Let me know if you want me to provide another pair of hands to help develop the flickr module further in this direction .
Flickr module rocks!
Best Regards
Paul
#13
Some further minor changes .
#14
I like this patch. It is exactly what I needed.
Now, it needs a check for already linked photos in the random function. Pretty often I get the same picture twice in a 20 pic block of a fairly narrow tag (karasjok).
Continue this path.
#15
BTW, the way of creating new blocks for Flickr-module is great. I wish more modules would allow to create blocks just as flexibly as that. Now I can create blocks in several languages needed for the site and then make a multilanguage block using this.
For other modules I have to create views for each language, because the modules don't allow me to create the same block several times with different language settings.
Kudos to you, and I hope this novel approach will be embraced by other module builders!
#16
Some suggestions:
Possibility to AND tags (as the default search on Flickr is, but also I think I saw this ability in the Flickr API). I don't think combination of and and or is neccessary, just a toggle next to the tags to search for which will let you select either and or or for the tags written.
Where do I edit a flickr block which is already inserted? It seems Drupal doesn't allow deletion of blocks. That's not your problem, but I'd like to be able to change the blocks I've already created.
I'd like the "number of photos" drop down to be changed to a integer field. I need to use 18 pictures to fit 2 rows of 9 pictures as a banner on the top of my pages, 20 leaves two orphans on the third line, and 9 is just one row and that isn't quite enough.
All of the above aren't deal breakers. Actually I'll use two blocks with 9 pictures each, when I come to think of it, so I have no problem doing a work around.
#17
Thanks Pkej for your kind words and taking the time to test the patch and provide some valuable feedback.
I am going to work on all the major issues today/tomorrow and release another patch tomorrow
Best Regards
Paul
#18
Here are my latest flickr patches .
The only thing that is not finished is deleting a flickr entry as there is a BUG here to be fixed but i figured i would upload the patches this evening so that people can use it now and if anyone has any time over the weekend perhaps have a look at the BUG (please see below)
You can now create and edit your flickr entries so it is easy to use the flickr module (plus patches) despite the BUG.
The BUG is actually caused by how the block module handles its records in the blocks table.
Here is how to repeat the problem .
1. Set up three flickr entries and activate the blocks for these flickr entries
2. Using the flickr entries page delete the second of the flickr entries (everything still looks ok )
3. Now go the blocks administration page you will then find you lose one of your blocks. The reason is that the
core module blocks.module has regenerated the blocks in the blocks table and screwed up
the flickr entries in the blocks table
Before going to the blocks administration page it will be something like ..
module delta theme status weight region custom throttle visibility pages title
flickr 2 theme 1 0 right 0 0 0
flickr 0 theme 1 0 right 0 0 0
then after going to the blocks administration page it will be something like ..
flickr 0 theme 1 0 right 0 0 0
flickr 1 theme 0 0 left 0 0 0
It has reassigned the delta's and also deactivated the block entry with the largest delta.
Very strange !!
Paul
#19
Block patch failed on 3 of 4 hunks on a fresh flickr module 5.x-1.1
The module patch faled on 1 hunk, but a quick review showed that it failed removing something which wasn't there.
After applying the patch the entries I create wont show up in the block list.
#20
The blocks did show up after I reverted to the earlier patch level.
#21
Sorry Pkej ,
I see a problem with the patch .
Ill take a look into this and release a working patch as soon as possible.
Paul
#22
Here are the actual files .
There is a unusual problem with some lines being missed out in the created patch .
I don't have the time at the moment to figure this out so i am going to upload the complete files
Paul
#23
Hello Folks ,
Please find the attached flickr modules with all my modifications for review .
I'll try and create patches in the morning for each of the flickr modules .
Its been a real pleasure working with this module , i hope you like the
direction i have taken this module.
Regards Paul
#24
Here are the patches. Can we release everything together as a flickr-5.x-2.x-dev
#25
sorry that this has been ignored for so long. i really like the direction you're moving the module but there's a bunch of little things that need to be fixed before i'll commit this. here's some feedback on #24:
'name' => $form_values['flickr_entry_name'] ,there shouldn't be a space before comma (or the trailing spaces for that matter).array_push($flickr_entries , $flickr_entry);+ variable_set('flickr_entries' , $flickr_entries);
+ drupal_set_message("Flickr entry <em>".$form_values['flickr_entry_name'] ."</em> has been posted");
Basically you should review the Drupal coding standards
#26
Thanks Drewish .
I'll address each feedback item and submit another patch tomorrow .
Paul
#27
* I'd come up with better names than (the existing @link and) @link2 in the help text.
>> addressed
* You're not indenting correct in flickr_menu(). Make sure you're using spaces rather than tabs.
>> done
* It looks like you're removing some blank lines? It's probably best to avoid those type of minor change as they just complicate the patch and make it harder to review.
>> it will be too time consuming for me to do this
* flickr_admin_entry_create_form()'s #description seems to have some spacing and punctuation issues.
>> addressed
* 'name' => $form_values['flickr_entry_name'] , there shouldn't be a space before comma (or the trailing spaces for that matter).
* Same goes for:
array_push($flickr_entries , $flickr_entry);
+ variable_set('flickr_entries' , $flickr_entries);
+ drupal_set_message("Flickr entry ".$form_values['flickr_entry_name'] ." has been posted");
you've spaces before commas and you're not spacing around the . for concatenation.
>> addressed
* I don't think you need flickr_admin_entry_edit(), the menu system can just call drupal_get_form() directly.
>> i haven't changed this as nothing is technically broken
* theme_flickr_admin_entries_form() seems to have incorrect indenting.
>> addressed
* What's the point of flickr_admin()? Wouldn't it just be better to use flickr_admin_entries_form() rather than forcing them to follow a link?
>> please feel free to change this it seems ok to me
If this patch is going to be commited and developed upon would you mind giving thanks to my employer
ian @ glaxstar who has allowed me to work on developing this module during work time at a cost of
probably around £1000 to his company , thanks Paul
#28
I found several formatting problem with the flickr_block.module . Please find attached a further patch for this module.
Thanks Paul
#29
Paul: To me it seems counter-intuitive to select that an entry is for an individual account and then enter that flickr user ID in the block configuration options. Do you agree that the Flickr user ID textfield should be on the create/edit entry page?
Attached is a patch that is just your patch with a bunch of cleanup. I got rid of the flickr/admin/entries page and made it just flickr/admin. I also took out the functions that just called drupal_get_form. Then I took out some of the spacing that was added or removed. Last, I took out all the funky windows characters and tabs.
Thanks for the work Paul, it is appreciated. When I get time I will look at the flickr_block.module patch.
#30
paul.booker, i tried out the patch on #28 but it wouldn't apply:
amorton@minivac:~/Sites/d5/sites/all/modules/flickr/block% patch -p0 < flickr_block.module_2.patchpatching file flickr_block.module
Hunk #1 FAILED at 6.
Hunk #2 FAILED at 89.
2 out of 2 hunks FAILED -- saving rejects to file flickr_block.module.rej
i'm going to look over the patch andrew just posted.
#31
andrewlevine, the patch on #29 doesn't seem to apply cleanly:
amorton@minivac:~/Sites/d5/sites/all/modules/flickr% patch -p0 -i flickr.module_4.patchpatching file flickr.module
Hunk #2 succeeded at 36 (offset 3 lines).
Hunk #3 FAILED at 162.
Hunk #4 FAILED at 195.
Hunk #5 succeeded at 358 (offset 3 lines).
2 out of 5 hunks FAILED -- saving rejects to file flickr.module.rej
is ever one working with a recent checkout of DRUPAL-5?
#32
drewish, the patches apply to 5.x-1.1. we will have to reroll at some point
#33
Here's a reroll of the patch in #29 that applies to DRUPAL-5 (and adds a couple of very minor coder.module-type changes).
#34
And here's a reroll of the patch in #28 that applies to DRUPAL-5.
Now that I've had a chance to apply these patches and take a look, I definitely like the functionality. This is exactly what I was looking for when I started casting about for a flickr module -- a block that shows flickr photos tagged "birthday,party". I have a few concerns:
I breezily assured a client a few weeks ago that there was no need to manage photos on his site. All he had to do was get his users to tag their Flickr photos "sitename eventname" and Drupal could pull them all in. I'm very glad I don't have to start at square one getting that to happen. Thanks for all the work that's gone into this patch already.
#35
sorry to raise the bar but for this to get committed it needs to land in 6.x first and then get backported. it's kind of a pain but it ensures that functionality that people get used in the 5.x releases is there when people upgrade to 6.x.
#36
No, of course it has to go into 6.x first, that's totally fine. I'm working on a 5.x site, but if I come up with something worth committing I'll upgrade it to 6 and then they can both go in together.
I'm thinking it would make more sense for these "entries" to be nodes instead. You'd specify your criteria at node/add/flickr, so your node would show, say, the first 20 flickr photos by user "iam30" tagged "birthday,party". Then at admin/content/flickr, you get a list of flickr nodes, with checkboxes. Check the box next to your birthday party node and a block with those same criteria gets exposed at admin/build/block as "Flickr block: birthday party". If you don't want the node itself visible, just the block, you can unpublish it. Any feedback?
#37
Thanks Ksenzee ,
I think you comments in #36 are spot on . If i have some time next week illl look at rolling a patch for 6.X which includes your suggestions.
I have fixed #34,2 . I'll get back to you on that tomorrow
Best, Paul
#38
Hello ,
I have begun working on flickr HEAD . I'll let you know when i have something to show you.
Sorry for the delayed response it took me longer than expected to get the delicious module
that i am managing upgraded to Drupal 6.
Thanks for some great feedback Ksenzee , much appreciated .
Best, Paul
#39
For anyone who wants the flickr functionality that we have so far for Drupal 6 . I have the following patch against CVS branch: HEAD.
#40
For anyone who wants the flickr functionality that we have so far for Drupal 6 . I have the following patch against CVS branch: HEAD.
#41
For anyone who wants the flickr functionality that we have so far for Drupal 6 . I have the following patch against CVS branch: HEAD.
There seems to be a problem getting both of my attachments posted , apologies for the multiple posts .