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" :)

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

aidanlis’s picture

I'll keep it open because you asked nicely, but if you'd like this functionality you'll have to implement it yourself :)

rickmanelius’s picture

That'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...

adamelleston’s picture

I 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 ;)

aidanlis’s picture

That'd be awesome! Let me know if you need any help.

adamelleston’s picture

How do you want me to do it? As a patch?

adamelleston’s picture

Status: Active » Needs review
FileSize
9.1 KB

Right 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

aidanlis’s picture

Status: Needs review » Needs work

Great 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 ...

adamelleston’s picture

Here 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.

aidanlis’s picture

Hehe did you look at the patch you attached? It just removes everything from the main module file.

adamelleston’s picture

oh dear, new patch on its way very soon :s

adamelleston’s picture

Ahh looks like I got the version numbers around the wrong way when creating the patch.

Try this one

adamelleston’s picture

Status: Needs work » Needs review

Hi,

Any chance you can review this patch when you get a spare 5 minutes?

Cheers,
Adam

aidanlis’s picture

Status: Needs review » Fixed

Hooray. 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.

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.