Early Bird Registration for DrupalCon Portland 2024 is open! Register by 23:59 PST on 31 March 2024, to get $100 off your ticket.
I know it explicitly states that this doesn't provide anything but an api, but a default block and/or interface to add in the 4 variables would certainly make it more useful. But I understand if this gets marks "won't fix" :)
Comment | File | Size | Author |
---|---|---|---|
#11 | facebook_pull_boxes-1181842-4925942.patch | 5.08 KB | adamelleston |
#8 | boxes_improved-1181842-4925942.patch | 5.09 KB | adamelleston |
#6 | boxes-1181842-4925942.patch | 9.1 KB | adamelleston |
Comments
Comment #1
aidanlis CreditAttribution: aidanlis commentedI'll keep it open because you asked nicely, but if you'd like this functionality you'll have to implement it yourself :)
Comment #2
rickmanelius CreditAttribution: rickmanelius commentedThat's fine. I may plan on using this for my site specifically, and would gladly submit a patch in that scenario.
I'll leave this open and if I don't get to it in say a month, then I'll just close it because its clearly not a priority at that point...
Comment #3
adamelleston CreditAttribution: adamelleston commentedI am looking into creating a boxes plugin for this module once Drupalcon is over and want to implement an admin page to enter default values as well.
You could then add each box by entering the details into the extra fields rather than php which will make it easier to edit by clients etc. I will post it once its done, and I have figured out how to ;)
Comment #4
aidanlis CreditAttribution: aidanlis commentedThat'd be awesome! Let me know if you need any help.
Comment #5
adamelleston CreditAttribution: adamelleston commentedHow do you want me to do it? As a patch?
Comment #6
adamelleston CreditAttribution: adamelleston commentedRight so this is my first ever attempt at a patch so I expect I have done something wrong ;)
It basically adds an admin menu item under admin->settings->facebook pull to set default app id, app secret, graph id and limit.
It stores al the data in 4 variables.
It then add a boxes plugin so you can create your own boxes on the blocks admin page. So it then just provides the standard description and title fields with the facebook app id, secret, graph id and limit. This way you can add as many custom facebook feeds to your site as you want. It also means if you are using spaces you can then add as many to each space as you want like in open atrium.
This does mean that its now dependent on boxes and ctools but I expect ctools is running on most sites and boxes is a great replacement/upgrade for the standard blocks so I wouldn't see this as being to much of an issue.
Let me know what you think/where it can be improved etc all feedback welcome.
Adam
Comment #7
aidanlis CreditAttribution: aidanlis commentedGreat work! A few comments:
- From your patch it looks like you've removed lots of functionality, e.g. all of the notes functionality (see all of the minus signs next to the functions). What I think you've done is rolled your patch against the wrong version of the code. If you follow the git checkout instructions into a clean directory and then re-make your changes, your "git diff" should be a lot smaller (less code removed).
- You don't need to add the ctools / boxes dependencies - that's the benefit of using hooks like drupal does - they are only called if that module is enabled, otherwise they don't affect anything.
- In facebook_pull_settings, you need to prefix each of the form keys with "facebook_pull" and you should return the form with return system_settings_form($form). This eliminates the need for the _validate and _submit functions, and for the submit FAPI element.
Let me know if you get stuck on anything ...
Comment #8
adamelleston CreditAttribution: adamelleston commentedHere is a new patch with your suggestions added. I have only modified the .module file and add the plugin file in this patch so dont need boxes and ctools dependencies.
Comment #9
aidanlis CreditAttribution: aidanlis commentedHehe did you look at the patch you attached? It just removes everything from the main module file.
Comment #10
adamelleston CreditAttribution: adamelleston commentedoh dear, new patch on its way very soon :s
Comment #11
adamelleston CreditAttribution: adamelleston commentedAhh looks like I got the version numbers around the wrong way when creating the patch.
Try this one
Comment #12
adamelleston CreditAttribution: adamelleston commentedHi,
Any chance you can review this patch when you get a spare 5 minutes?
Cheers,
Adam
Comment #13
aidanlis CreditAttribution: aidanlis commentedHooray. I've applied the patch and added support for hook_block as well as the boxes support provided by @adamelleston.
@adamelleston The patch was great, there were a few small problems like missing object_type from the facebook_pull call, the default values for facebook_pull_box::options_form had been copy-pasted incorrectly, some minor other style changes like whitesapce and tabs, but other than that great work! Hope this helps with BC.