Closed (fixed)
Project:
Flickr
Version:
6.x-1.x-dev
Component:
Code
Priority:
Normal
Category:
Task
Assigned:
Unassigned
Reporter:
Created:
6 Feb 2007 at 18:06 UTC
Updated:
29 Apr 2007 at 05:50 UTC
Jump to comment: Most recent file
Provide a page at /flickr to show the photos for the flickr_default_userid. I can imagine some people who could just use this module to display a set of photos from Flickr without allowing the per-user flickr display.
1) refactor function flickr_photos_get to not lookup user information. flickr_photos_get should just take a user id. move the user-based functions into flickr_photos()
2) modify flickr_photos to handle the default_userid
3) add a new menu setting for just /flickr
4) tell the user why the flickr_default_userid textbox in admin settings is disabled when no valid api key is set.
| Comment | File | Size | Author |
|---|---|---|---|
| #6 | flickr-default-user.patch_0_0.txt | 3.3 KB | andrewlevine |
| #5 | flickr-default-user.patch_0.txt | 3.3 KB | andrewlevine |
| #4 | flickr-default-user.patch.txt | 3.26 KB | andrewlevine |
| #3 | flickr-default-user_0.patch | 3.48 KB | samo |
| flickr-default-user.patch | 5.22 KB | samo |
Comments
Comment #1
andrewlevine commentedAwesome, I was planning on doing this sooner or later but now I won't have to. :)
1. I like this change a lot.
2. Good.
3. Good.
4. Great.
I'm not sure if you are aware of this but one of your other patches is include in this one as well. Could you provide a patch with just the changes for this issue? Awesome stuff though, I'm going to test this ASAP and commit after drewish/wnorrix tests as well.
Comment #2
drewish commentedI didn't have time for a deep review but looking at the patch the one thing that jumps out at me is that I'd use $uid=NULL rather than 0 as a default parameter.
Comment #3
samo commented1) I removed the information from the other patch I submitted.
2) I changed uid === NULL on drewish's suggestion.
New patch attached.
--Sam
Comment #4
andrewlevine commentedHere is Samo's patch against latest head (1.29).
by the way Samo something was up with your patch I couldn't apply it to 1.27, had to manually do it.
Comment #5
andrewlevine commentedSo this patch worked fine for me. I really like the idea of getting the 'drupal user' code out of flickr_get_photos. The only thing I added to the attached patch was a page title.
I do think we could move the default user id logic into hook_menu, but I will bring that up in another issue.
If we could get one more +1 (hopefully from drewish or wnorrix?) I will commit this.
Comment #6
andrewlevine commentedre-rolled for current head
Comment #7
drewish commentedgreat, i've committed this (with a few small changes). it moves us in the right direction.
Comment #8
(not verified) commented