I could have sworn you said on IRC that there was an issue for this but search isn't turning up anything. I would like to have a set of statuses attached to a group rather than a person. Each status would be posted by an actual user, but the list itself would be on the group. So it would be like a group wall rather than a profile wall. This would be a handy way for group members to have a quick chat without cluttering up their own walls. I'd prefer to use FBSS over some other chat module for this because then it would have a consistent interface. Plus, the upcoming plans for Publisher and such would be great to have.

I totally understand if you say this is out of scope, but keep in mind that Facebook itself allows groups to have walls and you did name this Facebook-style... ;)

So, what do you think? Is this doable? I'd be happy to help test. Maybe even help code it if you can point me in the right direction.

Thanks,

Michelle

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

IceCreamYou’s picture

Status: Active » Postponed

This is on my long-term to-do list, up there in the nether with comments. Technically speaking, the core implementation is actually pretty easy--the status update form has to run some logic to figure out that it's being shown on a group, and then when it saves a status, there would have to be another column on the {facebook_status} table to hold the group ID. The complicated part is writing the Views integration, mainly because there would need to be filters to exclude statuses posted to private groups as well as groups in general. OG integration should also have a separate API, but I don't really care about that.

So if and when I do this, it will probably be as a submodule. If you get something rolling, the whole process will move a lot faster. The core functionality would be in an implementation of hook_facebook_status_save(). You can ignore pretty much everything else for now, although Views integration is pretty crucial at some point obviously.

Michelle’s picture

Thanks for the pointers. I was thinking of this in another direction... Basically:

type: node or user
eid: (entity id) This is the ID of what the status is attached to, either node ID or user ID, depending on type.
sid: uid of the person sending the status
rid: uid of the person (if any) receiving the status

Then you can have a view of all statuses, all node statuses, all user statuses, or filter statuses to a particular type/id combination.

This would disconnect streams from users a bit and make it so you could potentially attach a stream to anything, much like fields are getting disconnected from nodes. :) You could even add taxonomy in there and have a stream per taxonomy term.

I haven't actually tried to code any of this. I see your API is well documented, though, which is great. I'll have to give it some more thought but I worry that my ideas may be too radical for you to fold back into the module itself. :(

Michelle

IceCreamYou’s picture

That makes sense--I was essentially thinking the same thing except I wasn't thinking of abstracting it to another level. There would still have to be at least one filter written explicitly for OG integration because of the complication of having private groups though.

FWIW, the current database schema is here. Essentially from what you're proposing UID would change to EID and a "type" column would be added. That's completely feasible although it would require much more sweeping changes because the API signatures would all change and all the DB queries in them would change as well.

I actually really like this idea, although it will be months before I can even think about committing the amount of time this would require. If Author Pane had views integration and this proposal got in to FBSS, FBSS could potentially even be a complete replacement for the comment system in addition to all its other uses.

Michelle’s picture

Sounds like postponed is a good status. It doesn't make sense for me to hack together something when this really needs some re-architecting to make it work. It's one of those "would be nice" things on my site but I can get along without it. Why don't we talk more when you have time?

Michelle

IceCreamYou’s picture

Works for me. I may just throw in a few final fixes to the 2.x branch, release a full 2.0, and then slate this as the first feature to go into 3.x. That will hopefully make the transition easier for most people and keep from overhauling the API when we're already at RC2 for this branch, and will give me some direction for 3.x because I still haven't decided on a Publisher implementation.

Michelle’s picture

That sounds like a good plan. :)

I wish we could fast forward sometimes... I think separating the stream from users will open up a lot of possibilities, especially once comments are added for a full conversational experience. Microblogging on an organic group is awesome if the folks want to get together for a quick chat. I'm sure there are other uses for it as well. Maybe for wiki pages to have a "discussion" tab? This has a ton of potential.

Michelle

IceCreamYou’s picture

Title: Attach statuses to an organic group » Abstraction of statuses from users

I completely agree. Technology rocks. :D I've reworked the roadmap for 3.x, by the way--you may want to take a look.

Michelle’s picture

Sounds great! Can we just skip 2.x and have 3.x come out tomorrow, please? ;)

Michelle

carlos73’s picture

It would be great to have a "full Facebook-style Publisher" but for DRUPL 6 not for DRUPL 7.
I think that drupal 7 takes lot of time before it becomes release and you have to wait that all the others modules (OG, notification, etc..) will be developed for Drupal 7.

I suggest you the develpoement for Drupal 6....first of all for those features:

- pubblish photo
- Userpoints integration
- Comments on statuses

Carlos

IceCreamYou’s picture

This issue is unrelated to plans for a Publisher; please keep it on-topic. (And anyway, if D7CX continues strong and test coverage does as much for bug-squashing in core as some seem to think it will, it's fairly likely that I won't even get around to starting a Publisher implementation until after D7 and many major contributed modules are already out and stable.)

IceCreamYou’s picture

Note to self: abstracting statuses from users also implies the use of the type_options property in hook_activity_info.

blup’s picture

Any update on this? I'd really like to use this throughout the site rather than just for user 'walls'. :)

IceCreamYou’s picture

I still really want this as well, but I'm working on two major extensions to FBSS at the moment and won't be able to get to this until those are more mature. My estimates of how long it will be before I get to start on an issue have been dramatically wrong over the past few months, so I don't want to make any promises. This issue requires rewriting hundreds of lines of code so even once I start it could take awhile.

On the other hand, if you really need this, you could contribute to speed development or hire me to work on it.

bflora’s picture

IceCreamYou - I'm interested in speeding development on this through supporting you. What's the best way to learn more? Full FB-style publisher available on user profiles, a home page, and on groups is my goal as well.

IceCreamYou’s picture

bflora, I appreciate your interest. Please contact me through my contact form here on drupal.org.

Michelle’s picture

Status: Postponed » Active

Setting this active just to ask a question. Feel free to set it back to postponed afterwards. I understand my urgency is not your urgency, especially since I'm not paying. ;)

Basically, I would really like to have statuses attached to nodes right now, even if it's not done "right". Do you have any tips for band-aiding this for now? I thought about making a user for each node that I want to have a stream but that's really messy. Any ideas that are less hacky than that? I normally hate hacking modules but I'm willing to do that if needed. Just looking for pointers to where I could stick in some custom code and fool FBSS into thinking a node is a user. Or am I dreaming and there's simply no way to hack this?

Thanks,

Michelle

IceCreamYou’s picture

Michelle, hacking this in is not going to be pretty. Practically every function in the module relies on the assumption that a status will be posted onto a user profile. You don't have to change everything to get the primary functionality to work, but you'll be giving up things like the Conversation view and integrations with other modules.

You'll need to add a column to the facebook_status table, probably called "type." Then you'll need to modify all the core functions: facebook_status_save_status(), theme_facebook_status_item(), facebook_status_box() and friends [this is the form callback], theme_facebook_status_form_display(), facebook_status_block() if you need it, facebook_status_user() if you need it, and _facebook_status_get_status_fast(). I'd do it in that order. In doing this, you will probably replace or cut out many of the helper functions.

Once you do that, integration with everything will be broken, including (most importantly) Views. If I were you and I needed this hacked together fast, I would forget Views and write custom queries and form_alter them in.

To completely abstract FBSS away from users the "right" way, I estimate 60-100 hours. To do it the way I described above, it would take me probably 15-25 hours. bflora has offered to help those 60-100 hours come closer, but we're still talking months (I'd love to have it done by the end of May, with funding, if I started in early April -- but I wouldn't be surprised if it pushed into June. Without funding, it's unlikely to be done before August, but I have no idea what my availability will be that far into the future.)

Michelle’s picture

Status: Active » Postponed

Ugh... If it would take you 15-25 hours, it would take me much longer. And it doesn't really make sense to invest that kind of time into hacking together something that barely works.

I wish I could fund you but I have no income so that's not happening. :( I guess I need to either wait or see if maybe I can adapt shoutbox easier. Thanks for trying. I appreciate you taking the time to explain.

Thanks,

Michelle

BenK’s picture

Subscribing to this thread and will look at documentation to see how I could get involved... also need the ability to have status updates on a node (so that a node can have a "wall").

--Ben

IceCreamYou’s picture

@BenK, I'm glad you're interested in getting involved. Completing this issue requires rewriting a significant amount of logic in almost every function in almost every file in the module. Patches are welcome and will be reviewed, of course, but unless you're willing to dive in very deep, the best thing you can do for now is likely going to be helping me find the time to work on this. That can be done either by helping out with other issues or by funding me. (I have a list of things I want to add/remove from/change in the module that aren't in the issue queue, so if you want to help on that front and you don't see an issue you can help with, please contact me and I'll give you something to do. ;-))

BenK’s picture

@IceCreamYou -- Are you going to be a DrupalCon San Francisco? I'm already here. If you're coming, perhaps we could touch base in person...

--Ben

IceCreamYou’s picture

I'm not, although I wish I was. If you want we can set up a call or meet on IRC sometime after tomorrow.

BenK’s picture

Status: Postponed » Active

Hey Isaac,

I saw your blog post on Drupal Planet recently, so I thought I'd stop by to see how abstraction efforts were coming along...

Is this still planned for D6 or now moved to D7?

Hope all is well,
Ben

IceCreamYou’s picture

I was thinking for awhile that I would move this to Drupal 7, but as the release candidate for Drupal 7 gets farther and farther away, I'm thinking this will probably be in a 6.x-3.x branch. I'm going to write another blog post on it soon, and I'm hoping to actually get the ball rolling sometime next week.

Since Mediacurrent has been so kind as to sponsor some of my efforts toward this issue, my goal is to get a beta release out with this functionality by the end of august. We'll see how busy I am, but I think it is not unreasonable. (And the fact that I can get it out by the end of August is the main reason I'm not going to wait for D7 on this, since it looks increasingly like D7-RC1 will be in early September.)

Michelle’s picture

Yay for D6! I know it reaches a point where maintainers are increasingly going to focus on D7 but that puts those of us still on D6 in a bit of a bind so I am very happy to hear this. :)

Michelle

IceCreamYou’s picture

I've been planning for this transition today. I started with the database structure:

mid | sender | recipient | type | created | message

Since statuses are now able to show up anywhere, they're no longer "status updates" -- they're "messages." I've actually been transitioning to that terminology recently by calling self updates "status updates" and cross-posting "status messages."

However this brings up a big question: how is the module going to detect what type of recipient we're dealing with? It was easy when it was just users -- we just assumed that if we were on the user path then arg(1) was the recipient, otherwise the current user was the recipient. However now we have all kinds of complicated things -- like if you're looking at a page that belongs to an organic group, is the recipient the group or the page or the current user or the page's creator?

There are a couple of solutions to this problem.

At the most basic level, the module will work as it always has. If you're on a user page, the recipient is the owner of that page. If you're somewhere else, you are the recipient.

However, that behavior will get overridden if other modules exist that can elaborate more on that dubious haze of "the rest of the site." There are three modules that can do that, and I've done some research into the APIs and configuration of each of these already.

Rules
Because Rules has conditions, it should be possible to use it to set the recipient for FBSS. For example someone could create a rule like IF [on OG page] AND [current user belongs to group] THEN [set recipient of status updates to OG]. Rules would directly invoke a FBSS function that could then set the recipient, presumably as a global variable. There are some limitations here though -- Rules is typically invoked when an action is taken, not when viewing a particular page.
Context
In a lot of ways, Context and Rules are very similar. We could do almost the same thing with Context as with Rules -- I would have to create a Context Reaction plugin that would set the recipient for the current page. Context is more passive than Rules, but it's better than Rules at detecting the context of the page. Also Context is easy to use for people who have used Drupal for awhile, but it's prohibitively complex for people who don't know what they're doing (mainly due to terminology, which is not its fault). One of the goals of FBSS is to be as dead-simple as possible.
Spaces
Spaces is an interesting beast. It's basically like Context (and in fact relies on Context for a lot of its functionality) but only one Space can be active at one time. That eliminates some of the guesswork from the other two options since both Rules and Context could easily set the recipient more than once in a single page load. However, as Ken Rickard put it, "anything that uses custom_url_rewrite_outbound scares the hell out of me." In other words, Spaces rewrites all URLs so that the user stays in the same space when they go to another page (unless that page assigns a new space). So if you looked at a group, you would be in that group's space, and then if you clicked a link to let's say a wiki node, you might still be in a group space even though you're not looking at that group specifically any more. Another problem with using Spaces is that FBSS would have to create its own interface (or else rely on Context) for assigning recipients based on the space. In addition, Spaces has more dependencies than the others.

From a technical perspective, there's actually no reason why we couldn't use all three of these options. But there is some question as to which one to recommend, whether any extra interface would be needed, etc. My inclination right now is to go with Context, as that seems to be the most effective and versatile solution. But I'm interested to hear what other people think. The only thing I'm ruling out at this stage is doing everything natively -- there's no reason to re-architect a system for detecting context when several already exist.

This brings up some even more complicated scenarios, like what if we want to have two status update forms on the page -- one for the current user and one for the group? I don't think we can handle things like this. There are probably ways to do it but they are far too complex and convoluted.

Anyway, other planning details:

  • I'm not going to rewrite the module from scratch, as tempting as it is.
  • I am going to move certain functions to include files, break certain integrations out into their own modules, and rewrite the actual content of every single function except possibly facebook_status_load().
  • I am going to actually use a folder structure this time instead of dumping all 50+ files into the same top-level directory. :-P
  • I am going to break practically everything in the process. There is almost nothing in the current module that is compatible with what the module will be after this issue is completed.
  • I am not going to provide an upgrade path during development until the first alpha, and I am not going to provide an upgrade path between dev versions until the first beta.
  • Here is where I want to be in terms of stable releases:
    • Alpha 1 -- Nothing crashes and burns, and the core of the module uses the newly abstracted database model.
    • Alpha 2 -- Context integration is working so FBSS will finally be usable with OG. Views integration is comprehensive.
    • Alpha 3 -- Services integration (or whatever) is completed so that third-party applications can now CRUD status messages remotely. Other integrations are more or less working.
    • Beta 1 -- Integrations are ironed out, basic functionality is pretty stable.
    • Beta 2 -- I'm sure there will be something else I suddenly realize I want to add along the way. This will go in here. Also finalize API and start upgrading related modules.
    • Beta 3 -- I am likely going to have to force myself to create a Beta 3 at some point. Even though it won't be completely stable, I would rather have something that's mostly stable or there will be too long an interim between Beta 2 and RC1.
    • RC1 -- API is final. Documentation is complete. Integrations with all 20+ modules are working at least as well as they are in the 2.x branch.
    • RCx -- As many release candidates as are necessary until I feel that the module is sufficiently stable.
    • 1.0 -- Status messages can be posted anywhere on a site with correctly assigned context. All integrations work well. All submodules are working. Few bugs, features, and tasks of "Normal" importance are left in the queue, unless postponed. No issues of "Critical" importance are left in the queue.

This is a great time to begin development on 3.x because this is the only non-fixed issue in the queue right now and I have nothing left on my personal to-do list except for four not-very-useful changes to Views handlers.

...I think that's it for now. Will update as I go. Feedback welcome.

Michelle’s picture

I've read this a few times over the last couple of days and it still makes my head spin. This sounds way more complicated than I was envisioning. Maybe it needs to be to cover everyone's use cases... For mine, it's pretty simple:

* When you create a new wall, you give it a type: User, node, or term.
* You give the wall the ID of the thing it's attached to.
* The ID makes sense in the context of the type. If it's a User wall, the ID is the user's ID.

I don't understand all this about context and spaces and how that figures in but that's probably because I'm looking at it from my narrow use case. I just want to add walls to users, nodes, and terms. :)

Michelle

IceCreamYou’s picture

Michelle, the problem is that you don't "create a new wall." You just put the status update block somewhere. So somehow the module has to figure out a way to determine whether the wall belongs to a user, node, or term. The administrator can't specify it in the block's settings, for example, because then that setting would apply everywhere the block appeared. And if that block appeared on both group and profile pages, that wouldn't make any sense.

So all this context stuff that sounds complicated is to make your life simple. The idea is to satisfy your use case with minimal configuration. You won't need to understand what goes into the back end.

And the basic functionality will work without Context/etc. so you won't even have to deal with that.

Michelle’s picture

Ah, I guess I figured that abstraction from users would mean making the wall its own entity. Trying to guess who's wall it is from where on the site it is sounds a lot harder. I guess I'm not much help, sorry. :(

Michelle

IceCreamYou’s picture

I guess I figured that abstraction from users would mean making the wall its own entity.

You'd have a 1:1 correlation between the stream and the entity it would be attached to, which would also make for a terribly awkward workflow if you had to manually create a stream for every user, node, or term you added. Think about it: when you create a group on Facebook, for example, a wall is just "there." You don't have to create it separately. (And if you don't want a stream on a certain group, you can just disable the block [or CTools content type] there.)

That's not to say I'm sold on Context integration, I just think it makes sense to have the context magically detected instead of requiring the user to manually input the data.

And don't be sorry, I appreciate the feedback.

Michelle’s picture

That's not what I meant... I was thinking more like you'd create a "group wall" and give it the characteristics you want and then place it where you want and pass in the group ID as an argument.

Michelle

IceCreamYou’s picture

Well you'd still end up with the same problem. You'd still have to have a way to collect the argument. Might as well just assume a stream can show up anywhere and automatically detect the argument to start with, skipping the whole step of "creating a stream."

In effect though, you're sort of getting at what I was trying to explain in #26, except instead of "creating a stream" you would be "creating a new context" which you can think of as an argument.

jcisio’s picture

Just know about this issue through the FBSMP module. Why don't simply limit the entities? I see there are 3 kinds of entities:
- Users
- Containers: groups, forums
- Contents: nodes

If we ignore the content (there is however a "wall" for nodes: the core comment.module), it will be straight away. And when a node belongs to many groups, do the way OG do to display blocks.

IceCreamYou’s picture

jcisio, I don't understand what you're trying to say, and you very clearly don't understand the complications here. I appreciate your enthusiasm, but please, let the developers do the developing.

jcisio’s picture

I might don't know the FBSS module well, on the contrary, I think I understand well the problem here. When a page displays the wall, you want to know to which entity that wall belongs. Thing I proposed solves that. And I also proposed to not compete node wall to the node comment, even they could be different (well, but I think in Facebook, is there anything that have wall AND comments?), even that the core comment.module is (sometimes considered) crappy.

I give my feedback here because I like the idea. I appreciate your straight speaking, but please don't criticize users because of their suggestion.

pribeh’s picture

subscribing.

IceCreamYou’s picture

jcisio, you may understand the problem, but you don't understand the code. There is absolutely no way to use FBSS with anything other than users unless I rewrite thousands of lines of code.

blup’s picture

IceCreamYou, with all due respect as I greatly appreciate your module, there is no need to shut down ideas like jcisio's. I understand if you don't want to work on this feature request, but there is also no need for exaggeration. I was working on this a couple of months ago, but halfway through I gave up as I had another project at hand it would've taken me the rest of the weekend. You insist on thousands of lines of code, but there is barely 1800 in the module, out of which 600 are module integration code. That leaves you with 1200, which I don't believe you'll rewrite entirely. Of course, there is also the views integration code, but as you said, "You can ignore pretty much everything else for now, although Views integration is pretty crucial at some point obviously".

Back to the problem at hand, my approach was the one stated by Michelle in comment #2, changing UID to EID, and adding a 'type' column (user or node). Thinking back, i believe the RID might not be necessary as it could get overcomplicated with multiple/variable receivers. Instead, I would propose a column for the 'category' of receivers (with different levels of privacy - see below). The information on this column could serve to inform context/rules about the receivers it should search for. I haven't had the time to check out Spaces, but setting the receivers through rules/contexts seems very logical, and you wouldn't have to re-invent the wheel. To cover the use-cases:

-You message a user, the user gets notified
-Your status updates will notify your followers or friends (flag/UR/etc)
-You post on a node, the node's author (and followers i guess) will get notified
-In the case the node is an OG, then an OG submodule will deal with the recipients (everyone/admins only, etc)

Privacy levels could then refine these. For example, "private" would notify the user/author/admins of group, whereas "public" would notify everyone possible (within the context of course). I don't think I'm missing anything, but please correct me if I'm wrong. I might have some free time coming up, so let me know if I can contribute with anything. I'll be happy to help get this rolling along.

IceCreamYou’s picture

IceCreamYou, with all due respect as I greatly appreciate your module, there is no need to shut down ideas like jcisio's.

My intention is not to be mean, and I apologize if my response came across as harsh. From what I can tell, jcisio's idea was basically to hack in support for certain entities and not fully abstract the module to work with any type of entity. That doesn't make sense though, because that hack would take nearly as much work as fully abstracting the module, and then to go back later and fully abstract it would take that same amount of work a second time.

I understand if you don't want to work on this feature request, but there is also no need for exaggeration. I was working on this a couple of months ago, but halfway through I gave up as I had another project at hand it would've taken me the rest of the weekend. You insist on thousands of lines of code, but there is barely 1800 in the module, out of which 600 are module integration code.

I don't know where you got those numbers. There are 6790 lines of code in the module not counting the default views (1755 lines), CSS (65 lines) and JS (262 lines) for a grand total of 8872 lines. The facebook_status.module file alone is 2567 lines.

I don't believe you'll rewrite entirely.

Not entirely, but I will have to change almost every single significant function. Most functions have logic in them that assumes that a status is posted to a user's profile. Even those that don't, often call other functions that do make that assumption, and the parameters to those functions would need to change.

Of course, there is also the views integration code, but as you said, "You can ignore pretty much everything else for now, although Views integration is pretty crucial at some point obviously".

You're taking that comment out of context. I made that statement to advise Michelle on the most rapid way to possibly add support for groups. That is not my goal. I am the maintainer of this module and there is absolutely no way I'm going to release anything that half-baked. I won't release a module that has huge parts of it broken. I will not sacrifice stability in favor of speed.

Back to the problem at hand, my approach...

I'm not sure I follow what you mean by "category" here. It seems kind of like you think there should be a second table to associate recipient types with privacy categories?

Privacy levels could then refine these. For example, "private" would notify the user/author/admins of group, whereas "public" would notify everyone possible (within the context of course).

I think you're going somewhat out of scope here. It's true as you seem to imply that due to the huge changes that the topic of this issue requires, I'm creating a new branch of FBSS that is, for now, open to new features. I'll be doing a lot of reorganization and optimization, mainly. However as far as I'm aware there hasn't been any discussion of privacy or notifications, and right now FBSS doesn't support either in the sense that you're using them. Privacy is such an intensely complex problem that in my opinion it requires an entirely separate module; I've been writing a blog post on the topic that I'll publish to Planet Drupal next week. Notifications has been on the roadmap for awhile but that framework is always opt-in, whereas you seem to imply that it would be opt-out. Maybe I'm misunderstanding you though -- can you clarify?

I might have some free time coming up, so let me know if I can contribute with anything. I'll be happy to help get this rolling along.

Thank you for offering to help -- I appreciate it. I don't need any help coding yet, but as soon as I get the new branch to a point where it doesn't blow up when it's enabled, I would love to have help testing and nailing down all the bugs that I'm sure will be introduced. Also, if you're interested in building some sort of privacy framework, or if you would like to volunteer to work on integration with the Notifications module, I would love to discuss that with you either by email or in another issue.

blup’s picture

I don't know where you got those numbers. There are 6790 lines of code in the module not counting the default views (1755 lines), CSS (65 lines) and JS (262 lines) for a grand total of 8872 lines. The facebook_status.module file alone is 2567 lines.

Strip the comments and the module integration code from your main module and you get 1200 lines of code.

You're taking that comment out of context. I made that statement to advise Michelle on the most rapid way to possibly add support for groups. That is not my goal. I am the maintainer of this module and there is absolutely no way I'm going to release anything that half-baked. I won't release a module that has huge parts of it broken. I will not sacrifice stability in favor of speed.

I understand, and I didn't mean that you'll neglect the views integration, but once it's all well established in the main module, the rest will flow.

I think you're going somewhat out of scope here.

You're right, I kind of mixed up two ideas I had for the module. The privacy aspect, as you said, should be covered in another module/thread.

IceCreamYou’s picture

Strip the comments and the module integration code from your main module and you get 1200 lines of code.

I don't have time to verify that, but comments are part of the code too and also need to be changed when the logic changes...

Once it's all well established in the main module, the rest will flow.

Maybe, but that "flowing" takes a lot more time than it sounds.

Anyway, I appreciate your feedback.

And just as an update... I am actually working on this in addition to talking about it. I'm not particularly close to a state where the module will run again, but I'm making progress.

bflora’s picture

Keep up the great work, Ice! Your "can do" attitude is a breath of fresh air.

oxford-dev’s picture

I also think this is an important step in the right direction, I need this functionality but for a different reason.

For each comment I also display an avatar of the user (a common feature I would have thought). However, currently because the uid of the post is the user who was posted to, this means that if user A writes something on user B's wall, user B's avatar is displayed with the post and not user A.

Another reason why this new approach is necessary.

IceCreamYou’s picture

@oxford-dev, the User: Picture field is for the recipient, but FBSS provides its own fields that let you show the poster's avatar.

oxford-dev’s picture

I'm not technically using the users avatar, its an image from the content profile.

However, if you say that similar functionality exists then maybe a bit of tinkering and I could swap out the avatar for my profile image.

Oh actually, i still dont think this is possible because the fbss feeds are being displayed through an activity feed.

guillaumev’s picture

FileSize
16.14 KB

Hi,

I'm providing here a module/hack that will allow you to use facebook status with organic groups. The module creates an og_fbss view that is displayed in a tab "Statuses" under the group page. Note that this module completely deactivates the facebook status per-user page (if you need it activated, comment out the "og_fbss_profile_alter" function). This module is definitely to be considered as a temporary solution for people like me who want to implement a quick and dirty per-group status page. Feel free to use the code and ask questions if you need to.

Also, please note that this module/hack was developed by myself and Christian Fasanando (http://drupal.org/user/799920).

IceCreamYou’s picture

Note also that the module in #47 will not work for users with JavaScript disabled or on sites that have AJAX updating disabled, and might also not work on sites with certain pathauto rules. (That's because it uses referrer_uri() to look for the group's node ID when it should really either add a custom hidden ('#type' => 'value') property to the form or make use of the $form_state variable like FBSS itself does.)

Other than that, from briefly looking at the code it looks like the module should work as advertised if you don't need user streams.

Offlein’s picture

Subscribing. And clap, clap for IceCreamYou. You're a serious developer! Keep up the good work.
Edit: I just realized that making "subscribe" posts must be really obnoxious for developers. I won't do this anymore.

IceCreamYou’s picture

Hahahaha well perhaps you would be interested in death to +1 subscribe... until that gets the remaining 90% of the funding it needs though there's no good alternative.

momper’s picture

+1

Offlein’s picture

IceCreamYou, any way we can get an update on progress of this? I know that you had been paid for development and were expecting to have a new iteration of the module (albeit possibly buggy) by the end of the month. Does that still look likely and is there anything I can do ($$?) to ensure that still goes as planned?

IceCreamYou’s picture

The goal was to get an alpha out by the end of August because my schedule became very volatile starting September 1st. August ended and the code I have for the 3.x isn't ready for release (it just causes WSODs). Finishing this is still a priority for me, but it will get done in increments on days when I find myself with available time, because right now I'm not able to schedule regular time to work on it. Since I don't know how much time I'll have available, I can't estimate when I'll be done. However I recognize that lots of people would love to have the improvements that will be in 3.x, so I will try to get this done as soon as I can (i.e. I'm really, really hoping for an alpha by mid-October, but no promises).

Also, this obviously pushes the whole release schedule from #26 back. Probably what I'll do is release one alpha ASAP, one beta after 1-2 months of testing, and then port it to D7. The 3.x release will probably get 1-2 beta releases in D7 and then everything will be moved to a new namespace and combined with the Micropublisher for a more complete experience with less of the "Facebook" stigma. There will probably be one branch at the new namespace that is architected similarly to the FBSS 7.x-3.x branch, and one branch that will do more interesting experimental things with the new Entity and Field APIs.

Offlein’s picture

Understood, IceCreamYou. I think what I'm going to do (for anyone that cares) is stick with Heartbeat's "Shout" form til this is out, and until that time, implement some kludge of a shortened "Node Insert" form to add, say, a Forum post that defaults to the present Organic Group.

That is - I want specifically to implement a Facebook-style wall on Group pages, using Heartbeat and FBSS. But I will make a Forum node insert form pre-set to the active group, which looks just like the Heartbeat "shout" form and tab the two - so you can say "Post a status" or "Post to this group". When you create a forum node, Heartbeat will log a message and update, so you're -- in effect -- making a shout (or FBSS) only to that group.

jcisio’s picture

The Shoutbox project provides technically a Facebook-style wall for sidewide usage, for each OG and for each user. What do you think about it?

IceCreamYou’s picture

Shoutbox is trying to be FBSS but it can't even come close to competing in terms of features right now. And the only reason it's usable at all is that Acquia took over maintaining it. The only reason to use it is that it supports OG, which FBSS will do shortly.

When FBSS is ported to D7 and rebranded, I'm going to consider providing an upgrade path from Shoutbox, mainly to make it easier to convince Acquia to switch when Drupal Commons upgrades to 7.

Michelle’s picture

I wish Acquia would have put its resources to supporting you. I'm going to wait for you. FBSS rocks! :)

Michelle

jcisio’s picture

Or Acquia just wants a lightweight solution, they don't want tons of hooks or module integration?

In my case, I've been looking for a "wall" module, I tried FBSS a half year ago, I've just found Shoutbox today and decided to use it on my site (1 million unique a month).

That said, I'm willing to switch to FBSS, especially there will be an upgrade path ;) if the new 3.x branch is more horizontal than vertical (integration, micropublishing...)

IceCreamYou’s picture

@Michelle - thanks. :D

@jcisio:

Well, Acquia needed OG integration, and Shoutbox already had that.

I think FBSS 2.x is actually lighter-weight than Shoutbox because Shoutbox is partially node-based, although I don't have numbers to back that up.

FBSS 3.x will be a lot faster than 2.x as well because it's going to be much better at only loading the things you need. There are two reasons for this: (1) most of the integrations are going to be split out into separate modules so you will be able to choose which ones you want, and (2) the humongous module file is being split up into a set of much smaller include files so only the relevant code will get loaded.

I'm not sure what you mean about horizontal vs. vertical, but you will have better control over which features you want in 3.x.

jcisio’s picture

The horizontal thing is what are being said in this issue: FBSS should support OG, user based wall... Even if it is faster and feature-rich, it might be not the chosen if it does not support the minimal feature set we want.

drupalok’s picture

+1 FBSS rocks!
+1 FBSS changing name
+1 who'd love to see this module work on GROUP nodes

Liliplanet’s picture

Hi,

The og-fbss is fabulous :) and definitely a major enhancement to organic groups.

I'm unable to display Facebook-style Micropublisher: Themed attachment if it has been added to a fb status. Only shows status text.

Am I correct to think that the themed attachments from micro-publisher is not as yet available in og_fbss?

Look most forward to any reply, and thank you.
Lilian

bensnyder’s picture

Shoutbox? FBSS? Just a little rant...

I'm currently customizing a Drupal Commons install which includes Shoutbox by default. I'm running Shoutbox for OG features until FBSS delivers the solution... but overall am frustrated that both projects provide vastly similar functionality. It would be so nice if there was a merging of efforts...

[This brings back memories of about 3.5 years ago when I first discovered Drupal. So many modules with similar functionality at the time....]

Looking at usage statistics, FBSS reports 2695 installs and Shoutbox reports 2289. After about 10 hours of research and looking through code... I'm left with more frustration. My gut is telling me "build off of FBSS" but for now I'm forced to mix the two. Plus, I've got to consider whether or not Commons will continue to push ahead with Shoutbox...

+1 for #56

Thoughts?

dmadruga’s picture

+1

IceCreamYou’s picture

Alright, ladies and gentlemen. I've just committed version 6.x-3.x of FBSS to the CVS repository.

THIS CODE IS NOT READY FOR PRODUCTION, OR EVEN STAGING, OR ANYTHING REALLY.

Here's the thing: I haven't tested this code at all. I have not copied the files to anything resembling a server and clicked "enable" at admin/build/modules. So I would not be the least bit surprised if enabling the module causes a WSOD right now.

Testing

Because of this, I haven't created a release node. If you want to look at the files, grab them from CVS. The most interesting files are facebook_status.module (of course) and most of the files in /includes/utility. (For some reason the online CVS browser is showing some files that my own client says are not part of the 3.x branch. I have no idea why that is or how to fix it. I think the only offending files are facebook_status_view.css, facebook_status_views_handler_field_latest_update.inc, facebook_status_views_handler_field_latest_update_date.inc, facebook_status_views_handler_relationship.inc, and the "ctools" directory and its contents.)

Improvements

However, I'm excited about this, and you should be too, because this branch:

  • Has full recipient abstraction, meaning it works with Organic Groups. Or anything else. A simple 20-40 line handler is all that's necessary to get status streams on any kind of content. See /includes/utility/facebook_status.contexts.inc for the default implementations (user, node, and OG).
  • Is standardized. Wording should now be clearer and more consistent, and APIs should be more straightforward to use. Additionally, the module's files are organized logically instead of all 50+ of them grouped together in a single directory.
  • Is easier to theme. At least, that's the idea. I switched to using a template file for individual status updates on the premise that those are easier for non-coders to understand. Since I haven't run the code yet, I don't know what the consequences are.
  • Is faster. Again, I don't have benchmarks for this, but I moved all of the non-essential code (all 1491 lines of it, or a staggering 58% of the 6.x-2.x branch) into submodules and include files. That means only the code you need should ever be loaded at any given time, which equates to increased speed.
  • Has comments. That's right, the Facebook-style Statuses Comments module, previously its own project, is now included in this branch. (However, I haven't upgraded it to the 3.x API yet, and it has some bugs of its own.)

For the record, since it seems to have been a point of contention: I changed all but 9 functions in the module. The structural changes are every bit as pervasive as I said they'd be. That's one reason it took so long to get this version out; the other is that as of September 1st I have been in college (I'm a freshman at the Wharton School at the University of Pennsylvania) and so it's taken me awhile to get adjusted and find the time to work on this. I had fewer distractions in high school when I was writing the 6.x-2.x branch.

Roadmap

My goal right now is to get a stable release (i.e. an alpha) of FBSS 3.x out as soon as possible, with the minimum set of functionality. That basically means I want Views integration and all the submodules upgraded to the 3.x API and usable without getting a WSOD. I stripped out some edge-case/legacy/cruft features in this branch, but otherwise I want as few regressions as possible.

Unfortunately, if I'm the only one working on this, "as soon as possible" is not very soon. I have exams coming up, so I pretty much won't be working on this until I go on break on the 22nd, and then I'm going on a week-long cruise that may or may not have internet. That's why I'm making the code available now; the module is at least structurally finished, and I won't have any more time to work on it for around a month.

As soon as I get an alpha of 6.x-3.x released, I will immediately switch to porting the 6.x-2.x branch to 7.x (because it's very stable, and that's more important at first). As soon as I get a 7.x-2.x branch running, I will work on porting the 6.x-3.x branch to 7.x. It will not be a direct port, because I am going to incorporate the Facebook-style Micropublisher module into it. This branch, effectively 7.x-4.x, will be released under a different (as-yet-unannounced) namespace that doesn't have the word "Facebook" in it. Upgrade paths will be provided at each stage; however it will not be possible to upgrade from 6.x-3.x to 7.x-2.x, so keep that in mind.

Once I get the 7.x-4.x branch (at the new namespace) to a full, 1.0, stable release, I am going to work on building a social networking distribution. Effectively, I want to build Facebook-in-a-box, so that people can just download the distro, click the install button, and get a full-featured, pre-configured Drupal site that works essentially like Facebook. That is a very long-term vision; at this stage, almost none of the components of such a distribution are at the level of features, stability, usability, and polish that I would like them to be.

Help with development!

These are long-term goals right now, because I simply don't have enough free time to make this happen quickly. If you are able, please help with development! There are some to-do items listed at the top of facebook_status.module that are a good place to start. In particular, FBSS 3.x depends on Views, but I have not even started upgrading the Views integration. I also haven't ported the facebook_status_tags submodule to be compatible with the FBSS 3.x API yet.

I would love to work with others to make this happen, and several people have expressed interest to me over email. If you would like to help work on the FBSS suite, the best way to do so is simply to post patches in the queue. Mark them against the 2.x branch until I enable the 3.x branch, but tag them with "FBSS3."

I would welcome a co-maintainer, but my standards are high. If you are interested in becoming a co-maintainer, please contact me.

Other things still on my to-do list

(but not at the top of facebook_status.module):

  • Fix that on user deletion, statuses are deleted directly from the database instead of calling the API function
  • Support Modal Frame API
  • Add a "view" token
  • For statuses posted to Twitter that are over 140 characters, shorten them and add in a link to the status page (use Shorten)
  • Real Twitter-style re-post system
  • Automatically create hashtags vocabulary
  • Support Flag 2.x
  • Support Twitter 3.x
  • Update code to match even-stricter Doxygen standards
  • Add Services integration
  • Notifications/Subscriptions integration
  • Simpletest integration

I would be surprised if most of these make it in. Of course I'd also love to clear out the issue queue.

Let's go

I know that non-developers will be disappointed that testing is not really an option yet. I'm sorry. However, for those who are able, please help get the ball rolling so we can get to a stage where people can test things and find bugs. Thanks!

pribeh’s picture

Great news Ice.

Michelle’s picture

Glad to hear you've made progress on this. Can't wait until it gets to a point where it's usable. Hope someone steps up to help with the development. :)

Michelle

bensnyder’s picture

SO COOL!

tugis’s picture

First of all congrats for the module ;)

I'm looking forward for the stable functionality of having the status also for the abstraction of statuses from users. Do you have any idea about when it will be ready?
I have a suggestion: why not configuring this module as a CCK field? This way you would be able to achieve the abstraction from users.

IceCreamYou’s picture

Do you have any idea about when it will be ready?

No more than what I've said above.

I am not going to explain again why FBSS does not, and never will use nodes.

jcisio’s picture

Maybe #69 gives a good reason for 7.x version: each status is a field (you have unlimited field) that can be attached to any fieldable entity.

IceCreamYou’s picture

7.x fieldable entities is a different (and much better) question. I haven't decided whether I want to use them by default, although I think it probably makes sense to at least make statuses fieldable. It all depends on a number of factors that I will look at when I get there.

bensnyder’s picture

FileSize
1.64 KB

Understanding modules is usually not that hard for me but this one is friggin' rough. I'm going to need some time to pour over this I think.

Anyway, here are two small, small fixes. Hoping to wrap my head around the OOP stuff.

IceCreamYou’s picture

Yeah. I put a lot of effort into making this as elegant as possible, with good coding standards, clear separation of various subsystems, and as few monolithic code blocks as possible. But the nature of having to figure out context is just a tough problem. I think the code itself is not so difficult to read, it's just that the flow of logic to the various subsystems is a little hard to follow. Here's an attempt at an overview:

  1. There is a call to display a status update form on the page through one of 5 entry points: the block, the statuses/share page, a conversation page, a user profile, or the CTools content type.
  2. If we're entering through the block or CTools content type, we first need to determine the context in order to determine which form to display (i.e. are we looking at a user, a standard node, a group node...?). We do that by calling facebook_status_determine_context().
    1. facebook_status_determine_context() first calls facebook_status_all_contexts() to get a list of all possible contexts. If contexts exist in the database (which occurs if a user has saved the configuration form at admin/settings/facebook_status/contexts), that configuration overrides the default one provided in code.
    2. It then walks through the list of contexts, loads any relevant resources, and finds the one that applies in the current situation by calling the context handler's is_applicable() function. Each context handler is an object that extends the facebook_status_object class (or one of its children). These classes are defined in includes/utility/facebook_status.contexts.inc.
    3. Note that the list of all contexts comes in already sorted by weight, so only the "most relevant" context is chosen.
  3. Once we have the context, we can call theme('facebook_status_form_display'), which actually does the work of displaying the form. It is defined in includes/utility/facebook_status.form.inc.
  4. In theme_facebook_status_form_display(), we first check access to the form using facebook_status_user_access(), which in turn calls the relevant access function as defined in includes/utility/facebook_status.access.inc.
  5. Then we display the form by calling drupal_get_form('facebook_status_box') (defined further down in facebook_status.form.inc).
  6. facebook_status_box() builds the form and adds the relevant JavaScript and CSS resources.
  7. When the form is submitted, we call facebook_status_save_status() to save the status message (defined in facebook_status.module).

So each $context is an array of properties (defined by implementations of hook_facebook_status_context_info() or overridden by the settings in the {facebook_status_contexts} table in the database) that define a type of "recipient" for status messages. (facebook_status_facebook_status_context_info() is in facebook_status.module.) The default context types are "user," "node," and "og." One of the properties of a $context array is the "handler," which is an object (a derivative of the facebook_status_context class) with methods used to find out more about the relevant recipient. (The facebook_status_context class is well-documented in includes/utility/facebook_status.contexts.inc.)

To give a broader picture, in order to display a status update form, we just need a recipient for the status update message as well as what that recipient is (a user, node, or group). The whole context system is just a way to figure out to which recipient we're sending a message.

Once you understand all this, it becomes clearer why all the context classes are in facebook_status.contexts.inc, the access methods are in facebook_status.access.inc, the form handling methods are in facebook_status.form.inc, etc.

I hope that helps (and doesn't confuse you more)!

bensnyder’s picture

@IceCreamYou - That was an extremely informative, much appreciated post.

Im running "tail -f /var/log/apache2/error.log" on a secondary monitor and refreshing the browser to spot php errors. Are there other techniques I can use?

1) The first error I see is:
PHP Fatal error: Call to a member function access_add() on a non-object in /var/www/[site]/sites/all/modules/facebook_status/includes/utility/facebook_status.access.inc on line 30

That line calls return $context['handler']->access_add($recipient, $sender);

I have no idea where to start. I have an idea in the back of my head though that is telling me maybe a constructor wasn't properly initiated? Idk!

Line 29 though is: $context = facebook_status_determine_context($type);

If the function's default parameters are facebook_status_user_access_add($recipient = NULL, $type = 'user', $sender = NULL)" - why is it that nowhere in the facebook_status_determine_context() function can I find code that deals with $type = 'user'?

2) After commenting out the above code, and temporarily bypassing the error, I get:
PHP Fatal error: Can't use function return value in write context in /var/www/[site]/sites/all/modules/facebook_status/includes/utility/facebook_status.contexts.inc on line 213

I don't even have OG installed. Should this code be run in the first place?

IceCreamYou’s picture

Are there other techniques I can use?

At this point, just attempting to install and seeing what happens is about all you can do... as you can tell, I still haven't done this myself yet.

The first error I see is...

I have an idea in the back of my head though that is telling me maybe a constructor wasn't properly initiated?

Yeah, I forgot to initialize the context handler when $type is specified in facebook_status_determine_context(). Committed a fix

If the function's default parameters are facebook_status_user_access_add($recipient = NULL, $type = 'user', $sender = NULL)" - why is it that nowhere in the facebook_status_determine_context() function can I find code that deals with $type = 'user'?

facebook_status_determine_context() takes the parameter $type. It doesn't care what the $type is, it just loads whatever context type is specified. If no $type is specified (i.e. $type is NULL, per the signature) then it determines the appropriate one automatically by calling the is_applicable() method on each context handler until it finds one that works.

facebook_status_user_access_add(), however, requires that the $type not be NULL, because it has to know what type of $recipient it's dealing with. "user" was an arbitrary default choice. I could just have easily made $recipient and $type required; instead I made the default be the "user is updating its own status" case, where $recipient is $GLOBALS['user'] and $type is 'user'.

I don't even have OG installed. Should this code be run in the first place?

I'm not sure it matters whether that code is run for that error to occur. It certainly shouldn't run. The group context should be removed from the list returned by facebook_status_all_contexts() when it checks dependencies. Regardless, easy fix committed

torvall’s picture

Hi IceCreamYou,

I installed the 6.x-3.x branch of the module, applied the fixes mentioned on #73 and #76 (and fixed a few others errors I bumped into), but when I saved the settings on the contexts admin form, my site stopped working (only WSOD's at any page). I had to delete the module folder and clear the cache to get the site to work again (and then copy back the module folder to uninstall it).

The error is:

PHP Fatal error:  Class name must be a valid object or a string in /var/wwwlib/drupal-6/sites/all/modules/facebook_status/facebook_status.module on line 587

And the offending line reads:

$context['handler'] = new $context['handler']();

Anyway, here's a patch with the fixes I done to the code. They are all just minor fixes, but at least they clear most errors one gets after installing the module.

IceCreamYou’s picture

Thanks for the patch. I'll try to take a look this weekend -- my guess is the culprit is lines 614-620 -- most likely the module-defined context array definitions don't get correctly merged. This might be better:

$result = db_query("SELECT * FROM {facebook_status_contexts}");
$contexts = module_invoke_all('facebook_status_context_info');
while ($c = db_fetch_array($result)) {
  $contexts[$c['type']]['in_db'] = TRUE;
  $contexts[$c['type']]['weight'] = $c['weight'];
  $contexts[$c['type']]['view'] = $c['view'];
  $contexts[$c['type']]['selectors'] = $c['selectors'];
}
torvall’s picture

That code did the trick, but I ran into another blocker.

Now I get this error:

warning: call_user_func_array() expects parameter 1 to be a valid callback, function '_facebook_status_access_js' not found or invalid function name in /var/wwwlib/drupal-6/includes/menu.inc on line 452.

I did before, but I copied the "_facebook_status_access_js" function to the main module file just to get rid of the message temporarily. However, when I try to post a status now, I get a javascript popup with the message: "An HTTP error 403 occurred. /facebook_status/js". This is without the function at facebook_status.module.

I also had to comment out the lines 130-133 and 138-141 at /includes/utility/facebook_status.admin.inc that refer to the vars "facebook_status_size_long" and "facebook_status_flood_user" that are no longer in the advanced configuration form.

bensnyder’s picture

FileSize
4.28 KB

This patch includes:
#73
#77 (which, among other things, fixes an error I had in #73) [thx torvall!]
#78

..and does not do anything from #79.

Let's keep this issue moving along.. thank you everyone for all your work!

bensnyder’s picture

FileSize
6.1 KB

Ok... after posting the last one I decided to include #79 as well with clear comments.

torvall’s picture

Issue tags: +FBSS3
FileSize
70.96 KB

Well, here's a first almost working version.

I don't post as patch, because I had to move the views-related files to the module's folder root to get them recognized by Drupal.

The major modification I did, was moving the views code to the new data model. Although I think I might have mixed up the "sender" (old facebook_status.uid field) and the "recipient" (facebook_status.pid) fields in some cases. The change between owner and poster to sender and recipient implies some changes in the way those fields were handled before, but I don't know this module well enough to do it without some guidance from the author. IceCreamYou, can you throw a few hints on how to proceed in that regard?

Anyways, at least it is possible to post status on the users' profile page and see them on the "Status" tab. :)

bensnyder’s picture

Issue tags: -FBSS3

@torvall,

Help me understand if you will. (I'm pro themer but an weekend module developer, if you follow me).

facebook_status_menu()

<?php
$items['facebook_status/js'] = array(
    'title' => 'Save status update form',
    'page callback' => 'facebook_status_save_js',
    'access callback' => '_facebook_status_access_js',
    'access arguments' => FALSE,
    'type' => MENU_CALLBACK,
    'file' => 'includes/utility/facebook_status.ahah.inc',
  );
?>

Why would you have to move the access callback _facebook_status_save_js into facebook_status.module if it is defined in the file includes/utility/facebook_status.ahah.inc?

It would seem that the error message:

Warning: call_user_func_array() expects parameter 1 to be a valid callback, function '_facebook_status_access_js' not found or invalid function name in _menu_check_access() (line 452 of /var/www/jttl.snydertechnologies.com/includes/menu.inc).

...which I too am getting is indicative of 403 error. I'm guessing the menu system is not registering this path? Odd.

When I move _facebook_status_save_js into facebook_status.module the error goes away and the path is accessible. No 403. Hmm.

Funny though. It doesn't seem to be showing the status updates I post. Could this because it's not running the page callback which saves it? Could the facebook_status.ahah.inc file be problematic?

...looking into this.

bensnyder’s picture

Looks like you posted ahead of me.

IceCreamYou, can you please commit the patches we've been working on and #82 so we can continue to develop this?

Much appreciated.

torvall’s picture

Btw, as a workaround to the "_facebook_status_access_js" issue, I simply disabled the "Use AHAH to refresh the status update form without refreshing the page" option in the advanced settings form.

bensnyder’s picture

FileSize
172.95 KB

#85 thanks... but why should we have top work around it in the first place? Why couldn't it find the function in the file in the first place? Was the file not loaded?

#82
Also, attached is a patch that I produced after replacing everything in the module with the tar you posted.

bensnyder’s picture

...oops

#82 (and consequently the patch I made in #86) are showing the

Warning: call_user_func_array() expects parameter 1 to be a valid callback, function '_facebook_status_access_js' not found or invalid function name in _menu_check_access() (line 452 of /var/www/jttl.snydertechnologies.com/includes/menu.inc).

error and 403 message.

Did you package it wrong? I did a fresh install of the module as you posted it in #82

torvall’s picture

You need to go to the advanced options form, and disable the "Use AHAH to refresh the status update form without refreshing the page" option for now. I really don't know why the function isn't being found, since others in different files in the same folder are being correctly used.

It could be related to the same issue that caused the Views functions and handlers not being loaded as well. For instance, according to hook_views_default_views() documentation: "This hook should be placed in MODULENAME.views_default.inc and it will be auto-loaded. This must either be in the same directory as the .module file or in a subdirectory named 'includes'." (Same thing for all other hooks.) However, moving all related Views files to the includes folder didn't work as well, and that's why I moved them all to the root.

IceCreamYou’s picture

Thanks for working on this!

Now I get this error:
warning: call_user_func_array() expects parameter 1 to be a valid callback, function '_facebook_status_access_js' not found or invalid function name in /var/wwwlib/drupal-6/includes/menu.inc on line 452.

I don't know why that error is occurring (my immediate guess would be a cache issue) but it should be possible to remove _facebook_status_access_js() altogether by simply specifying 'access callback' => TRUE (because that's all that _facebook_status_access_js() really is).

I also had to comment out the lines 130-133 and 138-141 at /includes/utility/facebook_status.admin.inc that refer to the vars "facebook_status_size_long" and "facebook_status_flood_user" that are no longer in the advanced configuration form.

Don't comment stuff out -- just remove it. We don't need cruft sitting around.

The major modification I did, was moving the views code to the new data model...

I have not yet reviewed this code, but updating it is a big task, so thank you for starting on it. From a very brief glance, I'd just like to note that all of the functions you added back to the module were intentionally removed, and should not have been added back.

It could be related to the same issue that caused the Views functions and handlers not being loaded as well.

No. Views does it's own thing.

According to hook_views_default_views() documentation: "This hook should be placed in MODULENAME.views_default.inc and it will be auto-loaded. This must either be in the same directory as the .module file or in a subdirectory named 'includes'." (Same thing for all other hooks.) However, moving all related Views files to the includes folder didn't work as well, and that's why I moved them all to the root.

The Views handlers should be where I put them, not in the root. The change needed to fix this is to add 'path' => 'includes/views' to the hook_views() implementation in facebook_status.module and then change the path defined in facebook_status_views_handlers() in views.inc to drupal_get_path('module', 'facebook_status') .'/includes/views/handlers'.

IceCreamYou, can you please commit the patches we've been working on and #82 so we can continue to develop this?

Committed #81.

IceCreamYou’s picture

Also committed the fix I suggested in #89 for the _facebook_status_access_js() warning.

bensnyder’s picture

@torvall

The major modification I did, was moving the views code to the new data model...

@IceCreamYou

... I'd just like to note that all of the functions you added back to the module were intentionally removed, and should not have been added back.

IceCreamYou, please clarify a little more so I better understand what you mean. Also, thank you for the commit!

I have a project waiting for Alpha-1 release of this module as denoted in #26 and I think torvall almost got us there in #82. I say almost because I ran into a fatal php error in facebook_status_views_handler_field_respond.inc which I'm sure we'll have to deal with soon along with a few others.

So the question is - what do we have to do to get there? I'm going to dissect #82 and see if I can make any headway and post a patch. Looks like I'm going to be reading some views documentation too... yippeekayay.

IceCreamYou’s picture

IceCreamYou, please clarify a little more so I better understand what you mean.

torvall's changes to enable Views support added facebook_status_item_components(), _facebook_status_can_edit(), and _facebook_status_exclude() back to facebook_status.module. Presumably this was because some of the custom Views handlers use those functions, but they are obsolete. The custom Views handlers should be modified to work with the 3.x equivalents.

I have a project waiting for Alpha-1 release of this module as denoted in #26

FWIW, the release of Drupal 7 means that schedule is going to be somewhat compressed (and obviously I implemented a new context system instead of using one of the solutions I suggested in #26).

So the question is - what do we have to do to get there?

I mean, we basically have to keep testing and fixing bugs, plus complete Views integration... I will try to finally get some time today to work on the module as well.

I'm going to dissect #82 and see if I can make any headway and post a patch.

That would be great, thanks.

torvall’s picture

Issue tags: +FBSS3
FileSize
2.13 KB

@IceCreamYou:

Don't comment stuff out -- just remove it. We don't need cruft sitting around.

Sorry for that, won't happen again. :)

From a very brief glance, I'd just like to note that all of the functions you added back to the module were intentionally removed, and should not have been added back.

I see that you already removed those functions, I'll focus now on updating the Views handlers to 3.x.

The Views handlers should be where I put them, not in the root. The change needed to fix this is to add 'path' => 'includes/views' to the hook_views() implementation in facebook_status.module and then change the path defined in facebook_status_views_handlers() in views.inc to drupal_get_path('module', 'facebook_status') .'/includes/views/handlers'.

Done. Patch attached.

On a side note, there are 2 copies of "facebook_status.author-pane.inc" (in the root folder and at the includes folder). From looking at both, I guess the obsolete is the one at the root, but I'm only guessing. :)

@bensnyder:

I have a project waiting for Alpha-1 release of this module...

Same here. But I think there's still some work to be done to get this module to alpha status. I also need the Rules integration (and some other submodules as well) for my project. It isn't just the main module we need to work on, but also all of the submodules. ;)

torvall’s picture

Issue tags: -FBSS3

IceCreamYou, you said in #26:

If you're on a user page, the recipient is the owner of that page. If you're somewhere else, you are the recipient.

I was going through the handlers and am thinking that shouldn't the recipient be the user/group/node being posted to in all cases? Depending on "type", we can allow access to the node owner, group manager or target user. In that case, we would re-implement _facebook_status_can_edit() and _facebook_status_exclude() using that info.

What do you have in mind for this, or how do you think this should be implemented?

IceCreamYou’s picture

Sorry for that, won't happen again. :)

No problem, that's why we have reviews. :)

Done. Patch attached.

Committed.

On a side note, there are 2 copies of "facebook_status.author-pane.inc" (in the root folder and at the includes folder). From looking at both, I guess the obsolete is the one at the root, but I'm only guessing. :)

Oops. :-P Actually, Author Pane (unlike Views, for example) only auto-discovers that file if it's in the module's root. I probably put it in the includes directory originally with all the other include files, but then remembered that it has to go in the root and copied it back. The files are different -- I remember being confused the last time I looked at one of them because I knew I had previously updated it -- but the one in root is better. I just removed the one in includes.

I was going through the handlers and am thinking that shouldn't the recipient be the user/group/node being posted to in all cases?

Keep reading... right after that, I said, "However, that behavior will get overridden if other modules exist that can elaborate more on that dubious haze of 'the rest of the site.'" In other words, the recipient is usually the "thing" you're looking at, and if you're not looking at anything, you are the recipient. (The system is actually smarter than that. Administrators can order the contexts, so for example you could make it so the current user is always the recipient, or you could make it so groups are never the recipient... etc.)

Depending on "type", we can allow access to the node owner, group manager or target user. In that case, we would re-implement _facebook_status_can_edit() and _facebook_status_exclude() using that info.

Those functions don't exist in the 3.x branch. If you're trying to understand how access works, start at facebook_status_user_access(), which is fully context-aware.

torvall’s picture

Ok, I thing I got it (duh!). Here is the patch with a new version. I hope it is correct.

Also: I checked all sender/poster instances and believe that I got them all right; and I changed all ocurrences of "uid", "pid", "status_date" and "status" (where related to the message field) to "sender", "recipient", "created" and "message". I renamed "facebook_status_views_handler_field_status.inc" to "facebook_status_views_handler_field_message.inc", so you'll need to remove that file from the repository by hand. I haven't figured out how to remove files through the patch yet, just add them. :)

I commented out most of the code in the message field handler for now, and it is only outputting the status message text. Why was facebook_status_item_components() being used and how to replace it?

Also, I noticed that the fields output is being re-written through the "created" field. Why is that necessary?

bensnyder’s picture

I haven't figured out how to remove files through the patch yet, just add them

torvall, what IDE do you use? I highly recommend you look at Aptana 3.

Thanks for continuing to post patches!

torvall’s picture

I forgot: apparently, the AHAH issue isn't fixed yet. If the "Use AHAH..." option is checked, when submitting a status, the form disappears and the message doesn't get saved to the database. Although with your change to the menu callback the error is gone.

torvall’s picture

torvall, what IDE do you use? I highly recommend you look at Aptana 3.

Well... actually, I don't use an IDE, just a text editor (gEdit) with a few plugins and syntax highlighting. I tried a few for PHP development, but usually they are either resource hogs, buggy or simply clumsy. Aptana looks good, though. I will take a look, thanks!

bensnyder’s picture

got this from the /user page after applying your patch

PHP Fatal error:  Call to undefined function _facebook_status_exclude() in /var/www/[site]/sites/all/modules/views/plugins/views_plugin_argument_validate_php.inc(43) : eval()'d code on line 28
bensnyder’s picture

Well... actually, I don't use an IDE, just a text editor (gEdit) with a few plugins and syntax highlighting. I tried a few for PHP development, but usually they are either resource hogs, buggy or simply clumsy. Aptana looks good, though. I will take a look, thanks!

Aptana was simply a fresh breath of air for me and I think you'll find the same result. They just recently rolled all pro version features (such as sftp support) into the Open Source release and simply provide support now.

torvall’s picture

got this from the /user page after applying your patch
PHP Fatal error: Call to undefined function _facebook_status_exclude() in /var/www/[site]/sites/all/modules/views/plugins/views_plugin_argument_validate_php.inc(43) : eval()'d code on line 28

That seems to be a cache issue, since that function is not in the code anymore. Try clearing your cache (especially the views cache at "/admin/build/views/tools"). If that doesn't solve it, uninstall the module completely (not just disabling it) then go to the views list and delete any "facebook_status*" view that may be left. Then reinstall the module and try again. Remember you'll need to deactivate the "Use AHAH..." option to be able to insert statuses.

bensnyder’s picture

FileSize
8.48 KB

duh.... thanks.

Adding the pane "Facebook-style Statuses Latest Status Updates" in Panels throws the php log message:
PHP Fatal error: Call to undefined function facebook_status_user_access_edit() in /var/www/[site]/sites/all/modules/facebook_status/includes/views/handlers/facebook_status_views_handler_field_edit.inc on line 23, referer: [site]/admin/build/pages/nojs/operation/page-front/handlers/page_front_panel_context/content

and browser popup: (attached)

edit: looks like you added the offending line in #96

torvall’s picture

Issue tags: +FBSS3
FileSize
39.2 KB

Actually, it looks like I messed up and used the wrong functions. Try this patch (instead of #96). I tested the view you used, and it works on my setup.

IceCreamYou, please disregard the previous patch. ;)

bensnyder’s picture

I flushed everything, uninstalled, reinstalled, etc so I'm sure of this one.

Posts are successful, but for every post rendered in the view, I receive the following:

Warning: Missing argument 1 for facebook_status_user_access_edit() in facebook_status_user_access_edit() (line 87 of /var/www/[site]/sites/all/modules/facebook_status/includes/utility/facebook_status.access.inc).
Warning: Missing argument 1 for facebook_status_user_access_delete() in facebook_status_user_access_delete() (line 70 of /var/www/[site]/sites/all/modules/facebook_status/includes/utility/facebook_status.access.inc).

So I'm left to conclude that the view that is rendering the posts and subsequently calling these functions (because I'm the admin and I have permissions) isn't providing an argument (obviously).

Bear with me I'm an amateur module developer :)

so it would appear that either the first arguments in:

line 70: function facebook_status_user_access_delete($status, $account = NULL)
line 87: function facebook_status_user_access_edit($status, $account = NULL)

need default argument values or the view that is calling the functions need to be fixed.

Taking a stab at it. Please screw my head on right if I'm wrong..

bensnyder’s picture

grep -r "user_access_edit" *
includes/utility/facebook_status.access.inc:function facebook_status_user_access_edit($status, $account = NULL)

Shouldn't I be able to find more than just the function definition? Is this function not used at all?

bensnyder’s picture

http://api.drupal.org/api/drupal/modules--user--user.module/function/use...

grep -r "user_access" *
api.php: * @see facebook_status_user_access()
api.php:function hook_facebook_status_user_access_alter(&$allow, $op, $args) {
facebook_status.author-pane.inc:  if (!facebook_status_user_access('view', $status)) {
facebook_status.module:    'access callback' => 'facebook_status_user_access',
facebook_status.module:    'access callback' => 'facebook_status_user_access',
facebook_status.module:    'access callback' => 'facebook_status_user_access',
facebook_status.module:    'access callback' => 'facebook_status_user_access',
facebook_status.module:    'access callback' => 'facebook_status_user_access',
facebook_status.module:  if (facebook_status_user_access('edit', $status)) {
facebook_status.module:  if (facebook_status_user_access('delete', $status)) {
facebook_status.module:    if (facebook_status_user_access('add', facebook_status_user_load($status->sender), 'user', $user)) {
facebook_status.module:  if (facebook_status_user_access('add', $user, 'user', $user)) {
facebook_status.module:function facebook_status_user_access($op) {
facebook_status.module:  $allow = call_user_func_array("facebook_status_user_access_$op", $args);
facebook_status.module:  drupal_alter('facebook_status_user_access', $allow, $op, $args);
facebook_status.module:  if (facebook_status_user_access('edit', $status)) {
facebook_status.module:  if (facebook_status_user_access('delete', $status)) {
includes/views/handlers/facebook_status_views_handler_field_repost.inc:    if ((user_access('edit own status') || user_access('edit all statuses')) && $values->{$this->aliases['recipient']} != $GLOBALS['user']->uid) {
.....

..answered my own dumb question.

bensnyder’s picture

Here are changes in the #104 patch that might be of interest in solving the warning messages in #105. Not sure if I'm even on the right track to anything so sorry for all the posting if thats the case.

Index: includes/views/handlers/facebook_status_views_handler_field_delete.inc
-    if (_facebook_status_can_edit($status, TRUE)) {
-      drupal_add_css(drupal_get_path('module', 'facebook_status') .'/facebook_status.css');
+    if (facebook_status_user_access('delete')) {
+      drupal_add_css(drupal_get_path('module', 'facebook_status') .'/resources/facebook_status.css');

Index: includes/views/handlers/facebook_status_views_handler_field_edit.inc
-    if (_facebook_status_can_edit($status)) {
-      drupal_add_css(drupal_get_path('module', 'facebook_status') .'/facebook_status.css');
+    if (facebook_status_user_access('edit')) {
+      drupal_add_css(drupal_get_path('module', 'facebook_status') .'/resources/facebook_status.css');

Index: includes/views/handlers/facebook_status_views_handler_field_repost.inc
-    if ((user_access('edit own status') || user_access('edit all statuses')) && $values->{$this->aliases['pid']} != $GLOBALS['user']->uid) {
-      drupal_add_css(drupal_get_path('module', 'facebook_status') .'/facebook_status.css');
+    if ((user_access('edit own status') || user_access('edit all statuses')) && $values->{$this->aliases['recipient']} != $GLOBALS['user']->uid) {
+      drupal_add_css(drupal_get_path('module', 'facebook_status') .'/resources/facebook_status.css');
bensnyder’s picture

Just caught this:

grep -r "_facebook_status_can_edit" *
submodules/fbss_rules/fbss_rules.rules.inc:  _facebook_status_can_edit($status);
submodules/fbss_rules/fbss_rules.rules.inc:  _facebook_status_can_edit($status, TRUE);

That probably needs to be changed, right? If I was confident enough in what to do I'd provide a patch but I thought I'd at least point it out as that function was changed/removed in many places in #104

torvall’s picture

The module builds dynamically the name of the access function to call in facebook_status_user_access($op) (line 457 of facebook_status.module), and that's why you cant find those functions being used anywhere else (I did used them before, and that is the fix in the last patch.

In the attached patch, the calls are correct (and I also had to include the "type" field in the edit and delete handlers because it was missing from the $status that was being passed as a parameter). Some of the things you point out in #108 were included in the previous patch (I think).

Btw, I noticed that it is necessary to post the contexts form in the administration to populate the "facebook_status_contexts" table.

Currently, the "type" column isn't being properly saved (but I'm on it :).

That probably needs to be changed, right?

Yep, all submodules will have to be modified as well. I think that in the case of "_facebook_status_can_*", it has to be replaced with "facebook_status_user_access" with the parameters operation ('view', 'edit', etc.) and the $status object (it's not on the function declaration, but the parameter will get through (note the "func_get_args()" on the second line).

IceCreamYou’s picture

Thanks for working on this guys! I had less time this weekend than I thought but I'll try to keep up... ;-)

Why was facebook_status_item_components() being used and how to replace it?

facebook_status_item_components() was a sort of poor man's drupal_render(). It allowed adding various things to themed statuses. In terms of the status message itself, it basically ran the selected input filter over the text and applied nl2br() if necessary. It also allowed other modules to run their own operations over the status (for example the Tags submodule uses hook_facebook_status_render_components_alter() to link-ify #hashtags and @mentions).

As far as replacing it goes -- well, the input filter needs to be applied, and nl2br() needs to be run if necessary -- I think that other modules that want to modify the status in Views can just do so through Views handler/hook overrides. The themed status in FBSS 3.x is handled by templates so it can be modified using preprocessing.

Well... actually, I don't use an IDE, just a text editor (gEdit) with a few plugins and syntax highlighting. I tried a few for PHP development, but usually they are either resource hogs, buggy or simply clumsy. Aptana looks good, though. I will take a look, thanks!

I use Aptana.

Btw, I noticed that it is necessary to post the contexts form in the administration to populate the "facebook_status_contexts" table.

Right. The default implementation of all contexts is specified by modules, but the version in the database can override the module-supplied default settings if the user has changed the order in which contexts are checked for example.

Yep, all submodules will have to be modified as well.

Some of them have been updated, some have been partially updated, some have not been updated at all...

----

Patch review:

I've committed pretty most of #110 (thanks!) with some very minor modifications. I didn't commit the changes to facebook_status.views_default.inc because I intend to completely redesign the default Views. The biggest thing I committed that's different than the patch is that I based the JOIN on the {users} table on the sender instead of the recipient. JOIN-ing on the recipient is a legacy thing from way back before FBSS supported posting messages to other users. Changing it should remove the need for some ugly work-arounds (although it could potentially introduce others).

I also haven't committed anything farther down than the changes to facebook_status_views_handler_field_cross.inc because I'm tired (I apply patches to my own modules by hand). I'll finish the rest later.

torvall’s picture

As far as replacing it goes -- well, the input filter needs to be applied, and nl2br() needs to be run if necessary -- I think that other modules that want to modify the status in Views can just do so through Views handler/hook overrides. The themed status in FBSS 3.x is handled by templates so it can be modified using preprocessing.

Is this it? Is seems to works fine, but I'm not sure if the order in which the input filter and line break converter are applied is correct.

/**
 * Field handler to provide the most recent status update.
 */
class facebook_status_views_handler_field_message extends views_handler_field {
  function render($values) {
    //Run input filter on status text.
    $message = _facebook_status_run_filter($values->{$this->field_alias});
    //Apply nl2br.
    if (variable_get('facebook_status_nl2br', 0)) {
      $message = nl2br($message);
    }
    return $message;
  }
}
The default implementation of all contexts is specified by modules, but the version in the database can override the module-supplied default settings if the user has changed the order in which contexts are checked for example.

Currently, the recipient and type aren't being saved to the database, and at the time I thought that that may have something to do with it (though I didn't have much success fixing that yet).

I didn't commit the changes to facebook_status.views_default.inc because I intend to completely redesign the default Views.

They really need a redesign. :) And I must confess that I'm not very confident that the changes I did to that file are all correct. I may have confused "sender" with "recipient" here and there. By the way, won't we need to create a context-aware recipient argument handler?

After these last developments, the variable names, interface labels, comments, etc. have become sort of "desynchronized". Maybe the next step should be to go through all files and consolidate the work done so far, to avoid any future pitfalls and also to make sure it is as "polished" as can be. I admit that I could have done better in that sense, but I still didn't have a clear understanding of the mechanics of the module (and it is easier for me to mess up with code than with comments; in my experience, a mistake in a comment may be far more catastrophic than in code).

So, what do you say about changing all instances of "uid", "pid", "owner", "poster", etc. to simply "sender" and "recipient" (with the obvious documentation support to communicate clearly the concepts)?

bensnyder’s picture

So, what do you say about changing all instances of "uid", "pid", "owner", "poster", etc. to simply "sender" and "recipient" (with the obvious documentation support to communicate clearly the concepts)?

The documentation is my biggest issue. I'd like to see inline comments on what's happening, especially in contextual situations, at least every other line or so. I've been doing my best.. Slowly but surely I'm familiarizing myself with this module.

Thanks again to the both of you for your efforts!

IceCreamYou’s picture

Currently, the recipient and type aren't being saved to the database, and at the time I thought that that may have something to do with it (though I didn't have much success fixing that yet).

Wait, you mean the recipient and type aren't being saved to the {facebook_status} table? If that's the case it probably has something to do with the is_applicable() property of context handlers not working.

I must confess that I'm not very confident that the changes I did to that file are all correct. I may have confused "sender" with "recipient" here and there.

I apply all patches to my modules by hand, so I checked what you did and it looked right to me.

By the way, won't we need to create a context-aware recipient /argument/ handler?

Well actually, all the fields that used to be for the poster should now be for the recipient. (In the past, because the recipient ["owner"/uid] was JOINed to the {users} table, the default User handlers sufficed for the recipient.) So -- yes.

After these last developments, the variable names, interface labels, comments, etc. have become sort of "desynchronized". Maybe the next step should be to go through all files and consolidate the work done so far, to avoid any future pitfalls and also to make sure it is as "polished" as can be.

I'm not sure exactly what instances of de-synchronization you're referring to, but yes, of course.

So, what do you say about changing all instances of "uid", "pid", "owner", "poster", etc. to simply "sender" and "recipient" (with the obvious documentation support to communicate clearly the concepts)?

There shouldn't be any places remaining (in code that has been updated to match the 3.x API/standards) that still use uid/pid/owner/poster, but if there are, they should be standardized. (I updated those terms as I was applying the Views patch from #110 this morning -- yet another reason why I apply patches by hand.)

The documentation is my biggest issue. I'd like to see inline comments on what's happening, especially in contextual situations, at least every other line or so.

In my experience, the difficulty of understanding other modules is not so much reading the code at any specific location but rather understanding how it all fits together -- that is, understanding the flow. And I think each individual function is explained reasonably well by its corresponding docblock, and the parts of the code that do approximately the same thing are grouped pretty well together. However, I will happily accept any additional code comments that would make the module clearer / easier to understand -- and I think it might be a good idea to write a DEVELOPERS.txt explaining general concepts, standards, data structures, and back-end workflows. I'll put that on my to-do list.

IceCreamYou’s picture

I must confess that I'm not very confident that the changes I did to that file are all correct. I may have confused "sender" with "recipient" here and there.

I apply all patches to my modules by hand, so I checked what you did and it looked right to me.

Actually, it was all right up to where I stopped in #111... after that, almost every single instance was wrong. ;-) Plus there's more to it than just changing uid to recipient and poster to sender: access needs to be updated, the type is relevant in some places, and the assumption that the recipient is always a user needs to be removed.

Anyway, finished committing Views changes. Here are some remaining Views tasks:

  • Add the new "type" column (field/filter/argument/sort)
  • Redesign default views (and assign them to the default contexts)

...there may be others.

I actually still haven't tried installing the code, as weird as that probably sounds. I'm going to be pretty busy this week but I'll try to actually run through it and make sure things work as expected. If things do work as expected and the two Views tasks above are completed, I'll create a dev release. (I'll release an alpha or beta once the submodules, JS, and CSS are updated, depending on how stable I think everything is at that point. I'd also like to get Services integration in, along with a few usability-related changes I've had on my to-do list for awhile, but we'll see.)

IceCreamYou’s picture

I just committed a fix that I think will solve the problem with updating statuses via AHAH. Basically, the form stuff (facebook_status.form.inc) wasn't available on the AHAH callback page (facebook_status.ahah.inc) so I just included it.

bensnyder’s picture

I just committed a fix that I think will solve the problem with updating statuses via AHAH. Basically, the form stuff (facebook_status.form.inc) wasn't available on the AHAH callback page (facebook_status.ahah.inc) so I just included it.

aaaaawwessome thanks..!

IceCreamYou’s picture

Alright, here's my current, tentative to-do list.

Before dev:

  • Add "type" column to Views integration
  • Finish creating default views
    facebook_status_user_stream, facebook_status_node_stream, facebook_status_conversation
  • Update JS and CSS to match new markup

Before alpha:

  • Write a DEVELOPERS.txt
    general concepts, standards, data structures, and back-end workflows
  • Upgrade submodules (this is a big one obviously, and involves a lot of sub-tasks)
    They don't have to be perfect, they just shouldn't be broken.

Before beta:

  • Polish submodules
  • Test thoroughly
  • On user deletion, call API delete function instead of doing a direct query
  • Allow having no character limit
  • Validate that chosen #hashtag vocabulary is free-tagging
  • Automatically create new #hashtag vocabulary
  • Don't accept numeric hashtags like #1
  • Check that the relevant view exists before trying to show it
  • Run the module through Coder

Would love to have before RC (but these won't hold back a release):

Depending on how quickly these tasks happen, they could shift up or down. I'm also going to want to start working on the D7 branch at some point (D7 AJAX stuff is soooooo much nicer) which might affect this list as well. Anyone can work on anything in this list, although I should probably be the one to design the default Views.

Edit: And obviously it would be nice to make sure all the relevant issue queues got cleaned up before RC. And an upgrade path needs to be written, ideally before beta.

torvall’s picture

Wait, you mean the recipient and type aren't being saved to the {facebook_status} table? If that's the case it probably has something to do with the is_applicable() property of context handlers not working.

Only when the context type is not 'user'. I've been trying to solve this one, but I'm getting nowhere. is_applicable() looks fine, my guess is that there's something wrong between theme_facebook_status_form_display() and facebook_status_save_status(). This one is a biggie for me, because without this fixed, I'm sort of stuck. Help!

Well actually, all the fields that used to be for the poster should now be for the recipient. (In the past, because the recipient ["owner"/uid] was JOINed to the {users} table, the default User handlers sufficed for the recipient.) So -- yes.

I implemented a facebook_status_views_handler_argument_recipient, but can't test it without proper data in the DB. Anyway, for now, I'll start on the type field.

Finish creating default views
facebook_status_user_stream, facebook_status_node_stream, facebook_status_conversation

Aren't facebook_status_user_stream and facebook_status_node_stream redundant (when using the recipient argument handler)?

Actually, it was all right up to where I stopped in #111... after that, almost every single instance was wrong. ;-)

I looked at the work you did, and I saw your pain. Again, I'm sorry and I really appreciate your patience. Some other person might not be so tolerant... ;)

I just committed a fix that I think will solve the problem with updating statuses via AHAH. Basically, the form stuff (facebook_status.form.inc) wasn't available on the AHAH callback page (facebook_status.ahah.inc) so I just included it.

I confirm that this is fixed.

IceCreamYou’s picture

...my guess is that there's something wrong between theme_facebook_status_form_display() and facebook_status_save_status(). This one is a biggie for me, because without this fixed, I'm sort of stuck. Help!

Usually the thing to do in that situation would be to put drupal_set_message('function_name: '. $type) pretty much everywhere between those two places then, and see what the result is. I'll look into it.

I implemented a facebook_status_views_handler_argument_recipient...

What exactly are you trying to do there? Because what the handler you wrote does is take the recipient ID as an argument and apply the current page's context. If you are looking at a page or a feed, that means the context will always be "user" by default (because you're not looking at a node). If you are looking at a block, then the current page's context might have nothing to do with the view.

It seems to me that what you're really looking for is one of these two things:

  • Two arguments -- one for the recipient ID and one for the recipient type, such that you would end up with URLs like statuses/node/37 to show all statuses posted to node 37.
  • A filter which automatically determines the current context and filters to only status updates that match the context.

Aren't facebook_status_user_stream and facebook_status_node_stream redundant (when using the recipient argument handler)?

Probably. I might combine them.

I looked at the work you did, and I saw your pain. Again, I'm sorry and I really appreciate your patience. Some other person might not be so tolerant... ;)

Heh. Well, you got it fixed one way or another!

IceCreamYou’s picture

Okay, the wrong context applying should be fixed.

IceCreamYou’s picture

I updated the CSS/JS and fixed a few important bugs earlier today.

Regarding DEVELOPERS.txt -- what would you like to see in it, specifically, keeping in mind that the API will be documented online (as they are currently for 6.x-2.x)? I mean, I'll probably include #74 in the online API docs -- is there anything else that would be helpful to include?

IceCreamYou’s picture

Just added the type and autotype fields. Already created the views facebook_status_all and facebook_status_stream, now just need facebook_status_conversation and then I'll create a node for the dev build. Going to sleep now though.

bensnyder’s picture

wow! great work! Sorry for being quite lately, I was busy getting Panels 960gs ready for release. Check it out and let me know what you think!

IceCreamYou’s picture

Committed default Views and created a development release. Now I need help updating the submodules and writing an upgrade path. Please help work on patches! Testers are also welcome at this point but keep in mind that the main Facebook-style Statuses module is the only one likely to work at this stage.

I'm going to keep this issue open until the submodules are ported to the 3.x branch because until that happens they will not have "abstracted statuses from users" which is what this issue is technically supposed to be about. :-P

bensnyder’s picture

I will certainly spend time on the submodules! Should be easier now that the main module is an example to follow. IceCreamYou we all really appreciate your effort on this! 4:53am!! Now THATS commitment!

momper’s picture

Title: Abstraction of statuses from users » FBSS: Abstraction of statuses from users
torvall’s picture

This patch fixes (most) of the status links, though the Reply/Respond/View conversation link (facebook_status_views_handler_field_respond) needs some work. Outside a user context (node or og), how should "conversations" be handled?

There is also an issue with the redirect after submitting the edit form; I think it may be related to the use of AHAH, but am not sure.

Looks like the facebook-status-item.tpl.php file needs to be in the module's root as well. I tried to append the path to the template declaration in hook_theme(), but it didn't work.

Sorry for being away lately, had an unexpected surge in work last week, though I kept tracking this issue and all the commits.

bensnyder’s picture

torvall’s picture

FileSize
13.52 KB

Here's an initial (working) version of the fbssc submodule. The patch is relative to submodules/fbssc.

I changed 'uid' and 'comment_time' columns to 'sender' and 'comment_created' to match the names used in the base module. I guess that this means that a specific upgrade function will be needed for this submodule as well. Shouldn't we take the chance to standardize modules names, tables, fields, etc.? If yes, what are the general guidelines?

torvall’s picture

Version: 6.x-2.x-dev » 6.x-3.x-dev
bensnyder’s picture

I had to uninstall/reinstall but I can confirm #130. I'm looking at the fbss_flag at the moment, can't find anything obvious so far but something isnt right.

IceCreamYou’s picture

Title: FBSS: Abstraction of statuses from users » Abstraction of statuses from users
Version: 6.x-3.x-dev » 6.x-2.x-dev

Re #128: The respond link already works the way it should...? It only shows up in the user context, as should be expected. I will review/apply the patch and look into the edit form problem as soon as I get a chance... It should work to use the correct path to the template file in hook_theme(), my guess is that you probably just didn't refresh your theme cache after doing it.

Re #129: This is the conversation of interest: #988196: Merge Shoutbox and Facebook-style Statuses (Microblog) [Drupal 7] -- basically Shoutbox and FBSS are likely to merge at some point in D7 (meaning mainly that Shoutbox is likely to have an upgrade path to FBSS). However I want to finish 6.x-3.x before starting on the D7 branch...

Re #130: "uid" can stay UID in FBSSC because there is only a sender with no recipient. UID is actually a more descriptive name when there is only one user. But in general it is a good thing to standardize to the conventions used by the main module (similarly "comment_created" would be better as "created"). And yes, we should definitely take the chance to standarize everything -- the reason I didn't change the name of FBSSC to fbss_comments to match the other submodules is that FBSSC is a legacy module and I think it would be difficult to upgrade if the name changed. However I'm thinking now that it should just be standarized and the current FBSSC module (which is a separate project) can just get a "migrate to fbss_comments" submodule or something. (This is also the reason I didn't rename FBSST... I'm not sure if there is a good solution there.) Anyway, I will test/apply the patch as soon as I get a chance...

Re #132: I'm not sure what there is to confirm about #130? As you've probably noticed I did update fbss_flag already but I haven't tested it. The best approach to fixing it if there are problems is probably to just try using it (make sure the default flag is created correctly, try adding the fields to a FBSS view, try clicking the flags to see if they store the clicks correctly). If there are errors, note the line where the error was thrown, then go look it up in the code and see if you can figure out what caused the error based on the message. Also note that it currently only works with Flag 6.x-1.x right now, although I'd love to have Flag 6.x-2.x support.

pribeh’s picture

Bump for Flag 2.x support. Flag 1.x is archaic ;)

bensnyder’s picture

I did an install from fresh with Flag 6.x-1.x and no default or configurable flag in views appears.

torvall’s picture

Version: 6.x-2.x-dev » 6.x-3.x-dev
FileSize
11.16 KB

This version of fbssc has the fields properly "standardized".

In relation to changing modules names, etc., maybe that could be solved by including all the upgrade functions in the base module .install file itself. If any of the sub modules is present, a warning could be presented to the users who don't read the docs to disable/delete old modules. I guess that's more practical for users than having to install a separate module.

torvall’s picture

Version: 6.x-3.x-dev » 6.x-2.x-dev

Ugh! $form_state, sorry. :)

IceCreamYou’s picture

Version: 6.x-2.x-dev » 6.x-3.x-dev

I did an install from fresh with Flag 6.x-1.x and no default or configurable flag in views appears.

Hmm.

In relation to changing modules names, etc., maybe that could be solved by including all the upgrade functions in the base module .install file itself. If any of the sub modules is present, a warning could be presented to the users who don't read the docs to disable/delete old modules. I guess that's more practical for users than having to install a separate module.

Well actually the fbss_comment module's hook_install() implementation could do the upgrade if the old fbssc table was present, as long as FBSSC was disabled first (either by the user or programmatically). It would also have to modify the fbssc record in the {system} table so that the module could be uninstalled correctly.

Ugh! $form_state, sorry. :)

That was actually my bad. This issue should probably be against the 3.x branch -- I didn't realize I set it back to 2.x earlier.

IceCreamYou’s picture

The flag thing turned out to be a pretty stupid error. The module's name is fbss_flag, and the hooks it has to implement start with hook_flag_ETC, so the names of the implementations of those hooks had to be fbss_flag_flag_ETC, but I had fbss_flag_ETC (forgetting one _flag). Fixed now.

bensnyder’s picture

The flag thing turned out to be a pretty stupid error. The module's name is fbss_flag, and the hooks it has to implement start with hook_flag_ETC, so the names of the implementations of those hooks had to be fbss_flag_flag_ETC, but I had fbss_flag_ETC (forgetting one _flag). Fixed now.

lol that's the kind of error you could spend hours beating your head on a desk while trying to find it.

torvall’s picture

Version: 6.x-3.x-dev » 6.x-2.x-dev
FileSize
4.42 KB

This patch is for facebook_status_tags. It's still missing the page handlers and the views, but already saves and presents tags correctly.

For this to work, I had to add a function to facebook_status.module:

/**
 * Invokes all <modulename>_facebook_status_alter_message to allow other modules
 * to modify the message (like facebook_status_tags).
 */
function facebook_status_display_message($message) {
  return module_invoke_all('facebook_status_alter_message', $message);
}

...and modify the message handler to:

/**
 * Field handler to provide the most recent status update.
 */
class facebook_status_views_handler_field_message extends views_handler_field {
  function render($values) {
    $message = _facebook_status_run_filter($values->{$this->field_alias});
    if (variable_get('facebook_status_nl2br', 0)) {
      $message = nl2br($message);
    }
    return implode(facebook_status_display_message($message));
  }
}

Since there is no longer the "render_components" thing, I figured this was the best approach.

torvall’s picture

Version: 6.x-2.x-dev » 6.x-3.x-dev

Hehehehe... :)

IceCreamYou’s picture

Since there is no longer the "render_components" thing, I figured this was the best approach.

Actually, it's handled by the theme system. What you should do is implement moduleName_preprocess_hook() and modify $vars['message']. For the Views field, the best approach is to override the relevant handler using hook_views_data_alter():

facebook_status_tags_views_data_alter(&$data) {
  $data['facebook_status']['message']['field']['handler'] = 'facebook_status_tags_views_handler_field_message';
}

A very quick glance at the patch in #141:

-    $sql .= implode(' OR ', array_fill(0, count($pid), 'fb.pid = %d'));
+    $sql .= implode(' OR ', array_fill(0, count($pid), 'fb.recipient = %d'));

There are a bunch of these -- the PID corresponds with the sender ("p" meaning "poster" as in the creator of the status message) and the UID corresponds with the recipient (owner).

I'm going to attempt to more thoroughly review the patches in #128, #136, and #141 tonight.

jcisio’s picture

FYI there is a new wave with Entity API that is built from scratch and for D7 http://drupal.org/project/micro

IceCreamYou’s picture

#128: The "sid" parameter is correct, and should not be changed to "rsid." Explanation here. Good job catching a bunch of little things I missed though.

#136: The main module has standardized on using hyphens instead of underscores in classes and ids -- it would be nice to do that in FBSSC too.
#136: The main module also standardized on putting a space after the two slashes that begin one-line comments, e.g. // This is a comment instead of //This is a comment. I went ahead and did that for FBSSC and FBSST (at least, all the instances that I saw) but it's a good thing to keep in mind.
#136: I dropped Appbar support -- it's supposed to be done through Rules in Appbar 2.x anyway.
#136: $view->tag == 'facebook_status' is correct, because the relevant hook only affects default views.

#141: I implemented the preprocess hook as I described in #143, but did not do the Views part.

Committed most of the changes from all 3 patches. Thanks!

torvall’s picture

Made many small fixes and renamed fbssc to 'facebook_status_comments' to match the main module and the already renamed 'facebook_status_tags'. IMHO, having all modules under the 'facebook_status_*' namespace is more convenient, but this is your project and if you prefer to change all submodules to 'fbss_*', I'll do it! :)

How should 'statuses/%sid' be handled? There is a page handler that references 'includes/utility/facebook_status.status-page.inc', bus this file does not exist.

The views on 'facebook_status_tags' still need a lot of work, but they should already display something.

And what about integrating comments and tags, is that on the roadmap, or not for now?

IceCreamYou’s picture

IMHO, having all modules under the 'facebook_status_*' namespace is more convenient, but this is your project and if you prefer to change all submodules to 'fbss_*', I'll do it! :)

I like "fbss" a little better, mainly because it's shorter. It doesn't matter very much to me as long as everything (other than the main module...) is standardized.

How should 'statuses/%sid' be handled? There is a page handler that references 'includes/utility/facebook_status.status-page.inc', bus this file does not exist.

Take a look at its definition in hook_menu(). I don't remember what facebook_status.status-page.inc was supposed to do but the handling for the individual status pages is in the main module file so that reference can be removed.

And what about integrating comments and tags, is that on the roadmap, or not for now?

Eventually. It's not a priority until D7 because it requires significantly restructuring FBSST. (The current DB schema is only set up to work for statuses; the ideal scenario is that the module would just provide an input filter so it could work anywhere.)

torvall’s picture

Here's a more polished (tested and revised) version of the previous patch.

torvall’s picture

I like "fbss" a little better, mainly because it's shorter. It doesn't matter very much to me as long as everything (other than the main module...) is standardized.

I just think that is clearer (although much more verbose) this way. But again, the last word is yours. It's nothing that can't be quickly fixed with replace all.

Take a look at its definition in hook_menu(). I don't remember what facebook_status.status-page.inc was supposed to do but the handling for the individual status pages is in the main module file so that reference can be removed.

Yup, simply removing that reference fixes it.

Eventually. It's not a priority until D7 because it requires significantly restructuring FBSST.

Ok, maybe it's better to focus on getting current features stable and then think about it.

IceCreamYou’s picture

Any chance you could make the patch from #148 without renaming the files? I just want to see the changes line-by-line because I apply all the changes by hand.

torvall’s picture

Here it is, with the same filenames and with the Activity integration module also updated.

torvall’s picture

...and Pathauto.

torvall’s picture

Some hooks aren't working properly, don't know why.

Namely hook_hook_info() (that I had to redeclare in fbss_activity) and hook_pathauto_bulkupdate() (had to rename it to facebook_status_pathauto_bulkupdate() in fbss_pathauto).

In the case of hook_hook_info(), it causes duplicate sets of triggers to appear in the Trigger module (although only the one declared in fbss_activity is used when setting up activity templates).

It probably is some basic hook thing that I'm not aware of, like the correct naming or something of that sort. I'm hoping it is something obvious to you. :)

IceCreamYou’s picture

Thanks torvall. I'm really busy this week but I will look into this as soon as I can.

IceCreamYou’s picture

Committed! Comments:

-      $selectors[] = '.view-id-facebook_status_conversation';
+      $selectors[] = '.view-id-facebook-status-conversation';

I don't think we have any control over this one. The machine name of the view is facebook_status_conversation, so Views spits it out with underscores included. Views doesn't allow hyphens in machine names.

-  $view->tag = 'Facebook-style Statuses';
+  $view->tag = 'facebook_status';

I like the English version. I'm not aware of any particular standard on this.

+/**
+ * Override message handler to convert tags to links.
+ */
+function facebook_status_tags_views_data_alter(&$data) {
+  $data['facebook_status']['message'] = array(
+    'title' => t('Status message (tags linked)'),
+    'help' => t('The text of the status update with tags converted to links.'),
+    'field' => array(
+      'handler' => 'facebook_status_tags_views_handler_field_message',
+      'click sortable' => TRUE,
+    ),
+    'filter' => array(
+      'handler' => 'views_handler_filter_string',
+    ),
+  );
 }

I don't really see a need to change the title/description/etc... all that needs to be changed is the handler, e.g. $data['facebook_status']['message']['field']['handler'] = 'facebook_status_tags_views_handler_field_message';

diff -uprN -x CVS facebook_status-DRUPAL-6--3/submodules/fbss_activity/fbss_activity.module facebook_status/submodules/fbss_activity/fbss_activity.module
--- facebook_status-DRUPAL-6--3/submodules/fbss_activity/fbss_activity.module	2010-11-30 07:20:45.000000000 +0000
+++ facebook_status/submodules/fbss_activity/fbss_activity.module	2011-01-30 23:33:42.000000000 +0000
@@ -21,6 +21,16 @@ function fbss_activity_facebook_status_d
 }
 
 /**
+ * Major hack. This shouldn't be needed.
+ */
+function fbss_activity_hook_info() {
+  $info = facebook_status_hook_info();
+  $info['fbss_activity'] = $info['facebook_status'];
+  unset($info['facebook_status']);
+  return $info;
+}
+
+/**
  * Implementation of hook_activity_info().
  */
 function fbss_activity_activity_info() {
@@ -29,9 +39,18 @@ function fbss_activity_activity_info() {
   $info->name = 'facebook_status';
   $info->object_type = 'facebook_status';
   $info->eid_field = 'sid';
-  $info->objects = array('Owner' => 'facebook_status', 'Poster' => 'poster');
-  $info->hooks = array('facebook_status' => array('fbss_deleted', 'fbss_edited', 'fbss_submitted', 'fbss_submitted_other'));
-  $info->realms = array('facebook_status_poster' => 'Facebook-style Statuses Poster', 'facebook_status_owner' => 'Facebook-style Statuses Owner');
+  $info->objects = array('Recipient' => 'facebook_status', 'Sender' => 'sender');
+  $hooks = array('fbss_deleted', 'fbss_edited', 'fbss_submitted', 'fbss_submitted_other');
+  foreach (facebook_status_all_contexts() as $type => $details) {
+    if ($type == 'user') {
+      array_push($hooks, 'fbss_submitted_user_self', 'fbss_submitted_user_other');
+    }
+    else {
+      array_push($hooks, 'fbss_submitted_'. $type);
+    }
+  }
+  $info->hooks = array('facebook_status' => $hooks);
+  $info->realms = array('facebook_status_sender' => 'Facebook-style Statuses Sender', 'facebook_status_recipient' => 'Facebook-style Statuses Recipient');
   return $info;
 }
 
@@ -41,9 +60,9 @@ function fbss_activity_activity_info() {
 function fbss_activity_activity_grants($activity) {
   $realms = array();
   if ($activity->type == 'facebook_status') {
-    $realms['facebook_status_owner'] = array($activity->uid);
-    $result = db_fetch_object(db_query("SELECT pid FROM {facebook_status} WHERE sid = %d", $activity->eid));
-    $realms['facebook_status_poster'] = array($result->pid);
+    $realms['facebook_status_sender'] = array($activity->uid);
+    $result = db_fetch_object(db_query("SELECT recipient FROM {facebook_status} WHERE sid = %d", $activity->eid));
+    $realms['facebook_status_recipient'] = array($result->recipient);
   }
   return $realms;
 }
@@ -53,8 +72,8 @@ function fbss_activity_activity_grants($
  */
 function fbss_activity_activity_access_grants($account) {
   return array(
-    'facebook_status_owner' => array($account->uid),
-    'facebook_status_poster' => array($account->uid),
+    'facebook_status_recipient' => array($account->uid),
+    'facebook_status_sender' => array($account->uid),
   );
 }

Skipping the Activity stuff for now. (Will look at it separately, preferably in its own patch...) Note that it's usually better to use e.g. $hooks[] = 'item'; instead of array_push($hooks, 'item');

-function fbss_pathauto_pathauto_bulkupdate() {
+function facebook_status_pathauto_bulkupdate() {

Why? I don't understand this change.

FBSSC's hook_facebook_status_render_components() no longer works, since that hook no longer exists. It needs to be replaced with a preprocess solution -- I'm not sure of the best way to do it right now since simply adding a variable would require the user to change their template. It may be best to actually override the template FBSS uses and provide our own.

-      $view->display['default']->display_options['fields']['nothing']['alter']['text'] .= $t;
+      $view->display['default']->display_options['fields']['nothing']['alter']['text'] = $t;

I don't think this is correct...

torvall’s picture

I don't think we have any control over this one. The machine name of the view is facebook_status_conversation, so Views spits it out with underscores included. Views doesn't allow hyphens in machine names.

You're right. Incidentally, that was also causing a PHP warning. It was my mistake, in the process of replacing underscores in HTML element's classes names.

I like the English version. I'm not aware of any particular standard on this.

It was just because of the if ($view->tag == 'facebook_status') in fbssc_views_default_views_alter(). I guess there's no reason to not check for the English version there.

Skipping the Activity stuff for now. (Will look at it separately, preferably in its own patch...)

Ok.

Why? I don't understand this change.

Well, Drupal simply won't even try to load fbss_pathauto_pathauto_bulkupdate(). Same thing with Activity and facebook_status_hook_info() (although in that case, it may be strictly related to Activity).

I don't think this is correct...

The field has the previous text and the concatenation is causing the items to appear in duplicate (without and with the 'comment' link). Although the field is unset() to insert the comment-box field, it is immediately reset with the stored data in the next line. So, the text needs to be replaced.

IceCreamYou’s picture

Well, Drupal simply won't even try to load fbss_pathauto_pathauto_bulkupdate(). Same thing with Activity and facebook_status_hook_info() (although in that case, it may be strictly related to Activity).

My best guess on this, without having looked at or played with the relevant code, is that both Pathauto and Activity manually invoke those hooks using the type of object they're dealing with instead of the name of the module. So simply changing the object names to fbss_pathauto and fbss_activity, respectively, would probably fix this.

The field has the previous text and the concatenation is causing the items to appear in duplicate (without and with the 'comment' link). Although the field is unset() to insert the comment-box field, it is immediately reset with the stored data in the next line. So, the text needs to be replaced.

Actually, the original version was inconsistent, and your patch switched which lines were inconsistent, but it should be fixed now.

IceCreamYou’s picture

Just committed some cleanup and improvements... here's the remaining tasks to beta right now, as documented at the top of facebook_status.module:

 *   - Write update path (including for submodules)
 *   - The character counter (and possibly other JS stuff) isn't working.
 *   - Finish upgrading the submodules
 *     o Complete: facebook_status_tags, fbss_comments, fbss_flag
 *     o Incomplete: fbss_activity, fbss_mollom, fbss_pathauto, fbss_rules, fbss_twitter, fbss_userpoints
 *   - Add a context configuration page that lets admin change the pages on
 *     which the context applies. Note that this also requires changing
 *     facebook_status_determine_context() to handle the case where no contexts
 *     apply. Additionally, let contexts modify this page to add extra settings.
 *   - FBSSC's hook_facebook_status_render_components() needs to be replaced with a preprocess solution
 *   - Allow having no character limit

Upgrading the remaining submodules is the only task remaining before I'll release an alpha. I don't care much about the context configuration page task. For RC, in addition to the tasks noted in #118, I would also like to have the issue queue cleaned up.

IceCreamYou’s picture

Just committed a fix for the character counter and the FBSSC problem (plus some other FBSSC fixes). In the process, discovered that comments don't show up.

IceCreamYou’s picture

Holding back alpha: finish upgrading submodules, specifically fbss_activity, fbss_mollom, fbss_pathauto, fbss_rules, fbss_twitter, fbss_userpoints

Holding back beta: write upgrade path, and get fbss_comments to actually display saved comments

Want to have (but not required) for RC:

Actually, if the submodules get upgraded and an upgrade path gets written relatively quickly, I might just go ahead and release an RC and start working on the D7 port.

torvall’s picture

Here's the fix for the fbss_comments module (make the comments show up in views).

torvall’s picture

This is the update for the fbss_activity module. Still includes "fbss_activity_hook_info". Without that, FBSS triggers don't get exposed to Activity.

torvall’s picture

FileSize
9.15 KB

And for the fbss_rules module. Still didn't test all of the rules, but in the UI it works well, and the events are being fired. Can't get token data, though.

torvall’s picture

The fbss_pathauto module is working as is, although to get Pathauto's bulk update working, 'fbss_pathauto_pathauto_bulkupdate' must be renamed to 'facebook_status_pathauto_bulkupdate'. I did look into your suggestions and tried some hook names (including the object type) but without any success.

IceCreamYou’s picture

Fixed and committed. (Details for people interested in the code: the Activity hooks had to be renamed to facebook_status_hookName() because the fbss_activity module is implementing them on behalf of facebook_status, just like the Activity module implements those hooks for the node, user, and comment modules. No fbss_activity_hook_info() was necessary. The pathauto integration requires specifying the module on behalf of which you're implementing the integration in its settings hook, so I just had to change that (and a few other places that referenced it). I also fixed a few things in the Rules integration patch.)

I also updated Mollom integration.

Only fbss_twitter and fbss_userpoints still need to be updated before an alpha release. Ideally though I would like to skip the alpha, just get the upgrade path written and release a beta -- otherwise the alpha and beta will get released very close to each other.

Update: I updated Userpoints integration too.

torvall’s picture

The RSS feed isn't showing recipients properly. Here's the patch for it.

torvall’s picture

The attached patch has the update function for facebook_status.

For fbss_comments, I think this is all that's required:

/**
 * Update database to version 3.x.
 */
function fbss_comments_update_6300() {
  $ret = array();
  // Drop primary key and indexes to rename table.
  db_drop_primary_key($ret, 'file_revisions');
  db_drop_index($ret, 'fbssc', 'cid');
  db_drop_index($ret, 'fbssc', 'sid');
  db_drop_index($ret, 'fbssc', 'uid');
  db_drop_index($ret, 'fbssc', 'comment_time');
  // Rename the created field.
  db_change_field($ret, 'fbssc', 'comment_time', 'created', array('type' => 'int', 'unsigned' => TRUE, 'not null' => TRUE, 'default' => 0));
  // Rename the table.
  db_rename_table($ret, 'fbssc', 'fbss_comments');
  // Recreate primary key and indexes.
  db_add_primary_key($ret, 'fbss_comments', array('cid'));
  db_add_index($ret, 'fbss_comments', 'cid', array('cid'));
  db_add_index($ret, 'fbss_comments', 'sid', array('sid'));
  db_add_index($ret, 'fbss_comments', 'uid', array('uid'));
  db_add_index($ret, 'fbss_comments', 'created', array('created'));

  return $ret;
}
torvall’s picture

Oops! Forgot a "$ret = array();" at facebook_status_update_6300() in the patch.

IceCreamYou’s picture

Committed the RSS patch, thanks -- will commit the upgrade patch later, gotta run right now -- but here's a quick review:

-      '@recipient' => (module_exists('realname') && !empty($recipient->realname)) ? $recipient->realname : $recipient->name,
+      '@recipient' => ($status->type == 'user' && module_exists('realname') && !empty($recipient->realname)) ? $recipient->realname : $context['handler']->recipient_name($recipient),

Just $context['handler']->recipient_name($recipient) is fine here. If we need to handle the realname module differently (and I'm not convinced that we do) then that should probably be done in the user handler's recipient_name().

+  // Drop current indexes.
+  db_drop_index($ret, 'facebook_status', 'uid');
+  db_drop_index($ret, 'facebook_status', 'pid');
+  db_drop_index($ret, 'facebook_status', 'status_time');

Is it necessary to drop the indexes on these columns before changing them? (I don't know the answer to that question, and I don't see anything wrong with doing it -- I'm just curious whether most SQL drivers will change the indexes automatically.)

+  db_add_field($ret, 'facebook_status', 'type', array('type' => 'varchar', 'length' => 255, 'not null' => TRUE, 'default' => '', 'description' => 'The stream context type.'));

This creates the "type" column with empty values. However, all old status updates will really be of type "user." We will need to run a command like UPDATE {facebook_status} SET type = 'user' after inserting the column.

torvall’s picture

Is it necessary to drop the indexes on these columns before changing them? (I don't know the answer to that question, and I don't see anything wrong with doing it -- I'm just curious whether most SQL drivers will change the indexes automatically.)

I followed as example the implementation system_update_6022 of hook_update_N from Drupal's modules/system/system.install. Inside that function, it reads in the comments:

  // Rename the file_revisions table to upload then add nid column. Since we're
  // changing the table name we need to drop and re-add the indexes and
  // the primary key so both mysql and pgsql end up with the correct index
  // names.

I just took their word for it. ;)

This creates the "type" column with empty values. However, all old status updates will really be of type "user." We will need to run a command like UPDATE {facebook_status} SET type = 'user' after inserting the column.

Yes, and it's in the patch:

+  $ret[] = update_sql("UPDATE {facebook_status} SET type = 'user'");
IceCreamYou’s picture

I followed as example the implementation system_update_6022 of hook_update_N from Drupal's modules/system/system.install... I just took their word for it.

Good enough for me! :)

Yes, and it's in the patch:

Whoops, my bad. :-P

Committed the upgrade patch for facebook_status.install, with just one comment:

+  drupal_uninstall_schema('facebook_status_contexts');

drupal_uninstall_schema() takes the module name as a parameter, not the table name.

The FBSSC upgrade function from #167 would be mostly correct except that it would have to be run manually from update.php, which can only happen after the new fbss_comments module is installed, in which case there will be a collision of table names from converting the old {fbssc} table to the new {fbss_comments} table. I committed a version that just moves the data from the old table to the new one. Here's the upgrade process for FBSSC now:

  • Disable the old Facebook-style Statuses Comments module (fbssc).
  • Upgrade the core Facebook-style Statuses module.
    This needs its own explanation...
  • Enable the new Facebook-style Statuses Comments module (fbss_comments).
    Potential problem -- these have the same name in the UI at admin/build/modules.
  • Run update.php. On the page where it asks you which updates to run, choose 6300 for the Facebook-style Statuses Comments module. Run the update.
  • If the update is successful, uninstall the old Facebook-style Statuses Comments module.

Instructions for upgrading the main Facebook-style Statuses module are more complicated than just replacing the files and running update.php because all the Views and markup has changed, some settings have been removed, and the integrations have been moved to submodules. Basically, custom FBSS views need to be backed up and then deleted; all FBSS-related modules need to be disabled (including the core module); FBSS needs to be upgraded normally; the relevant FBSS submodules need to be enabled; then custom Views need to be re-created, styles need to be adjusted, and settings need to be reviewed.

EVERYONE: as noted in #160, the only remaining task before releasing a beta is upgrading the fbss_twitter module. That means we are ready to start testing! Please try the new 3.x branch and report any bugs you find. Reporting 3.x bugs in this issue is fine until I close this issue.

torvall’s picture

Since I dislike loose ends, here's a patch for fbss_twitter. I did not test it, but I tried to be as thorough as possible and checked if all calls and handlers are correct. As far as I can tell, it should work. :)

About fbss_comments' update, shouldn't we be using hook_install (see: system_install()) instead of hook_update_N and check there if the table ("fbssc") doesn't already exist? And if it does, update it (and inform the user and give him further instructions), or create a new schema if not. The way it is now seems very user unfriendly and may lead to some confusion, in my opinion. And would the system present the module's update in update.php being a fresh install? It is a different module with a different "system" name, after all.

IceCreamYou’s picture

About fbss_comments' update, shouldn't we be using hook_install (see: system_install()) instead of hook_update_N and check there if the table ("fbssc") doesn't already exist?

That would be nice, except that there's no cross-database-compatible way to do that.

The way it is now seems very user unfriendly and may lead to some confusion, in my opinion.

Yes. That's why I was initially resistant to changing the module's name. The only alternative that I can think of is to create a new release of the old FBSSC module whose sole purpose is to port old data to the new module. However, the steps will be basically the same -- the old FBSSC needs to be disabled but not uninstalled before the update to the new module, then the new module has to be enabled, then some upgrade process needs to be manually selected, and then the old module should be uninstalled.

And would the system present the module's update in update.php being a fresh install?

As I noted, users will have to manually choose update 6300 when they run update.php. It won't run unless users choose it manually since it is a fresh install.

Will review the patch when I get a chance, thanks --

torvall’s picture

I've been digging around to find out how people that had the issue with renaming submodules managed this. I found #591538: Upgrade path for new submodule names is broken (Nodewords module) that is similar, in a sense, to the issue with FBSS. They handle, in the main module's hook_update_N, any changes that are needed to update submodules (in their case, enabling submodules and do some tinkering in the {system} table). Maybe something in the same line can be done here like, when updating the main module, create the new table for FBSSC (if that module is enabled) and copy the data, disable the old "fbssc" module and enable the new one. Then notify the user that a copy was made and that he should uninstall the old module to remove its data. I'm not sure if a check in fbss_comments_install isn't needed to insure Drupal doesn't try to create a new schema (and possibly "overwrite" the newly created table).

bensnyder’s picture

Reference the bottom of: http://drupal.org/node/576278#comment-3769186
and: http://commons.acquia.com/discussion/modules-development-upgrade-status-...

For notifications, Is the path forward going to be settings up custom rules or using the Notifications module? (I know it's not at the top of the TODO list :)

Also, can we keep track of TODO lists on the project page? Reference #118 and #160

IceCreamYou’s picture

The old FBSSC module will throw errors if it is enabled at the same time as the FBSS 3.x branch -- possibly even fatal errors. It must be disabled (but not uninstalled, obviously, or the data would be lost) before upgrading to FBSS 3.x.

There is no way to detect if the old FBSSC module was installed. It does not have any of its own system variables and it doesn't store any data anywhere outside of the {fbssc} table, so the best we can do is check the {system} table to see if FBSSC was at one point enabled. That doesn't get us anywhere because it doesn't tell us if the {fbssc} table is still around.

Also, I don't know of any way to change another module's schema. I think that if the {fbss_comments} table is created anywhere other than in fbss_comments_install(), Drupal's schema-related functions won't recognize it when called within fbss_comments. (That doesn't mean it's not possible -- I just don't know how to do it.)

There is still the option of creating different tests depending on which database driver is in use to check whether the {fbssc} table exists. If this was done in the main module's upgrade function, it could automatically enable fbss_comments as well if appropriate. However, the current solution would still have to be available as a fallback for people using unsupported drivers.

torvall’s picture

It must be disabled (but not uninstalled, obviously, or the data would be lost) before upgrading to FBSS 3.x.

Being an update between major versions, it seems reasonable to leave that task to the user.

That doesn't get us anywhere because it doesn't tell us if the {fbssc} table is still around.

There is a db_table_exists() function in the API (available at least for mysql, mysqli and pgsql). Maybe we can use that inside fbss_comments_install()?

IceCreamYou’s picture

There is a db_table_exists() function in the API (available at least for mysql, mysqli and pgsql). Maybe we can use that inside fbss_comments_install()?

Nice! I completely forgot about that function. Yeah, that would do it.

For notifications, Is the path forward going to be settings up custom rules or using the Notifications module?

There is no Notifications integration right now -- that's why it's on the "it would be nice to get it in but it's not a show-stopper" list in #160.

Also, can we keep track of TODO lists on the project page? Reference #118 and #160

#118 is outdated. The RC list for #160 is correct, everything for alpha/beta will be done once the patch from #172 is in. I'll move the to-do list to the project page once I release the beta and close this issue.

bensnyder’s picture

ice, you da' man
torvall, thank you for your contributions to this effort!

torvall’s picture

FBSST's view argument doesn't validate the term if the correct vocabulary isn't selected (or none of them is). An easy fix is to unset all vocabularies (to search for the tag in all of them); I tested having the same tag in different vocabularies and apparently works fine, although I don't know if there are other implications.

Also, the $view->tag in that view needs to be changed to 'Facebook-style Statuses' to have the comments show up.

AntiNSA’s picture

subscrube

torvall’s picture

This patch has the "fix" I mentioned in #180 and a minor fix for facebook_status_token_list().

torvall’s picture

Meh. Forgot the patch. :)

torvall’s picture

Just realized that tokens need to be updated in Pathauto as well (at least). So I added that to the update function in facebook_status_update_6300() (it's the best place to do it, right?).

IceCreamYou’s picture

I have just committed the upgrade to Twitter module integration (with some minor modifications from #172) as well as improved the FBSSC upgrade process (the old module is now detected, disabled if necessary, upgraded to the new one, and then uninstalled) and fixed a bug in the main module's upgrade process.

FBSST's view argument doesn't validate the term if the correct vocabulary isn't selected (or none of them is). An easy fix is to unset all vocabularies (to search for the tag in all of them)...

It doesn't make sense to search for the tag in all vocabularies; we only want to look for the #hashtag in the #hashtag vocabulary, and if the #hashtag doesn't exist, we don't show anything (there would be nothing to show anyway). In other words, this is the expected behavior.

Having said that, I realize that the problem is that the vocabulary isn't automatically selected in many cases. I just committed a fix for that, although it might be better to make that fix in hook_views_default_views_alter() because right now it is very easy to forgetfully overwrite if I just copy-pasted a Views export over the current version.

Also, the $view->tag in that view needs to be changed to 'Facebook-style Statuses' to have the comments show up.

Committed a fix.

Just realized that tokens need to be updated in Pathauto as well (at least). So I added that to the update function in facebook_status_update_6300() (it's the best place to do it, right?).

I suppose so. I mean, those variables would be sitting around anyway even if fbss_pathauto was never installed.

------------

We're basically ready for beta now. I need to write the release notes (which include upgrade instructions, differences from the 2.x branch, etc.) and tag the release and that's pretty much it. So, everyone, please test now!

The to-do list for RC in #160 is still correct, although I should probably add that documentation needs to be written. Per #178 I will close this issue and put the to-do list on the project page once a beta is released.

Two remaining thoughts: I haven't actually reviewed the default views provided by the facebook_status_tags module. It's a huge pain to upgrade between versions of default Views if you've customized them, so I'd like to get it right the first time. Also, I'm thinking that during the upgrade process it might be a good idea to automatically install any of the submodules whose integration we can detect was in use. For example we can check to see if Pathauto has values stored for FBSS, and if it does, enable fbss_pathauto.

bensnyder’s picture

ice, would it be hard/complicated to have the status views PREPEND the latest updates, instead of refreshing the whole thing?

IceCreamYou’s picture

ice, would it be hard/complicated to have the status views PREPEND the latest updates, instead of refreshing the whole thing?

Both nearly impossible and not generic.

bensnyder’s picture

shoutbox seems to manage to do it... (...probably uses a different mechanism).

I'd have to disagree with about "not generic". I think more people would want it to prepend, rather than refresh the whole view IMHO.

IceCreamYou’s picture

shoutbox seems to manage to do it... (...probably uses a different mechanism).

No, it doesn't. It refreshes the entire view, which it loads from a callback. This means Shoutbox can only update Views when a shout is submitted. In contrast, FBSS can refresh any part of the current page, so it needs to silently reload the entire current page to get the relevant refreshed content.

I'd have to disagree with about "not generic". I think more people would want it to prepend, rather than refresh the whole view IMHO.

"Generic" is not a concept you can agree or disagree with. FBSS supports refreshing more than just Views; that is generic. Creating a system specifically for Views would not be generic. It has nothing to do with people's preferences.

In the next generation of FBSS, I'm planning to add a feature that allows developers to specify specific callbacks to refresh content (in other words, basically adding the option to do it the way Shoutbox does now, except generically). If no page regions have been specified for refreshing that don't have custom callbacks, then a full page refresh won't be necessary. The reason for doing this is to allow refreshing content that isn't immediately available on the page, such as QuickTabs that get loaded in via AJAX (see #847802: Content loaded on the page via AJAX can't be automatically refreshed when a status is submitted). However, used injudiciously, this could actually be slower than just a single full page load, since several callbacks could be specified instead of just one. (It might look faster though, since they would be executed one at a time.)

But anyway, that's a long way off. (Unless, of course, someone wants to write a patch for it sooner.)

IceCreamYou’s picture

By the way, before I wrote FBSS's refreshing mechanism, I talked to merlinofchaos (creator of Views) about how difficult it would be to prepend just the most recently added records to Views. He told me that he tried it once, spent a few days on it, and then gave up. At the time, I assumed that if merlinofchaos couldn't do it, then there was no way I would be able to. I'm not so sure any more that it's that hard, but I have plenty of more important things that I'm a lot more confident about.

bensnyder’s picture

No, it doesn't. It refreshes the entire view, which it loads from a callback. This means Shoutbox can only update Views when a shout is submitted.

....(/facepalm)

Thanks for the through answer as always.

torvall’s picture

And what about having an option to refresh the view every N seconds? There are similar feature requests posted to Views (#343542: Enable optional auto-refreshing views using AJAX) and Ajax Views Refresh (#541226: Automatic View Refresh Every X [timeframe]), but both of those issues are sort of abandoned.

Anyway, it might be a bit of an overkill to use an external module to do that (and that could cause conflicts with AJAX loaded views; have lots of unnecessary dependencies; etc.), if it was "easy" to do in FBSS.

Although it would be ideal to be able to select a time span per view display, it could be just a module's global option to start with and a boolean option attached to the views' display basic settings to enable (or something of the sort).

I really don't know if it's a simple matter or not, but I think that it would be a great feature to have.

pribeh’s picture

@torvall the best approach for automatically refreshing views is to do so only if new content is detected. This is how all the major social networks handle this and I think makes most sense.

http://drupal.org/project/views_hacks#views_autorefresh

... already gets us auto-refreshing of a view every number of seconds. I put out the request to the maintainer of views_hacks to do something like I described above: http://drupal.org/node/943174. Of course, I'm not positive about whether or not this will save resources.

Facebook and Twitter actually go one step further to simply display a number of new items ("x new tweets") at the top of their activity/status feeds which link to refreshing the display. The user then must determine whether or not to refresh the view (or "prepend" which I'm not sure either are doing). This is definitely the best approach if your user status generation is of a high volume.

I've become a bit obsessed with how important this simple number display is from a user experience perspective. I really think that one of the most crucial factors for any social networking site is the simple display (or notification) of however many new statuses/activity-items there are (at the top of the list, in the browser tab, in a notifications block, on the mobile app icon, etc.). Just my two-cents.

Michelle’s picture

I know I love it. I used to refresh Twitter over and over when I was in the middle of a conversation. Now I can just keep an eye on the tab for the number to pop up. Much more convenient. :)

Michelle

IceCreamYou’s picture

Views Hacks' Views Autorefresh is the best way to automatically refresh Views (linked by pribeh in #193). It doesn't make sense to re-implement the same functionality in FBSS in a less generic way.

I agree that it is very important to let users know when new content is available. That was the original intent of my Appbar module. However it requires a lot of resources to push content. You can read more about various techniques to do so at Wikipedia.

FBSS is not going to implement a feature like this in the forseeable future. I am focused on getting the 6.x-3.x branch out and stable and getting it ported to Drupal 7.

pribeh’s picture

FBSS is not going to implement a feature like this in the forseeable future. I am focused on getting the 6.x-3.x branch out and stable and getting it ported to Drupal 7.<\blockquote>
Agreed, for those interested let's focus on the generic approach in the views hack thread I linked to.

torvall’s picture

I didn't find the Views Hacks module when searching for a way to refresh views automatically; don't know how I missed it, it's exactly what I'm looking for. Thanks pribeh!

bensnyder’s picture

Either the "Like" flag in Views (after enabling it) is still broken or I'm a complete moron. I can't find it anywhere.. Anyone here having luck? If so, can you share?

pribeh’s picture

@bensnyder I'm pretty sure integration with Flag 2.x is broken so perhaps that's the culprit if you're using Flag 2.x. Also, in the view you might have to add a relationship to the flag for it to show up - I can't remember if this is the case for FBSS and I'm not in front of a testing terminal - so check that as well.

IceCreamYou’s picture

IceCreamYou’s picture

Status: Active » Fixed

Woohoo! A beta has been posted. Time to finally close this issue, 5 months after it was opened!

Thanks and congratulations to those who helped.

Any additional issues with the 3.x branch should now be posted in their own issues.

bensnyder’s picture

#200 HALLELUJAH!!!!

Thanks to all involved as well! Looking forward to the success of this mighty fine specimen!

Michelle’s picture

Thanks all. I look forward to trying it soon as I can. :)

Michelle

pribeh’s picture

Great work! Congrats!

Status: Fixed » Closed (fixed)

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

AntiNSA’s picture

Category: feature » support
Status: Closed (fixed) » Active

sorry for the stupid question, but after this epic thread can you tell me in two sentances what is abstraction of statuses from users and how it is different from the original module?

New in 3.x! - groups and other types of nodes (as well as potentially taxonomy terms, comments, etc.) can have their own status streams.

I mean,, I accomplished that with a simple argument to only display the status of members which belong to the group. How is what this long thread different than addign a simple arguement/filter?

IceCreamYou’s picture

Category: support » feature
Status: Active » Closed (fixed)

Please don't open closed issues (especially feature requests). Open a new one and link to the old one. This is especially true on a huge thread like this with lots of subscribers who get updated about every new post.

Anyway, in 2.x you can show a list of all the statuses of users in a group, but you can't have a list of messages to the group. In 3.x, groups (and other things) can have their own streams to which users can post statuses that only show up there.

AntiNSA’s picture

Sorryy I am afraid if I comment to a closed thread no one can read it.... I understand now ver 3 allows statuses to be posted TO existing OG groups which the member is a part of, similiar to selecting whch group to post content too? I will try this if there is a user interface which allows this in a simple manner..