I wanted a way to grab the Flag data about a node when I loaded the node. Four lines did it in the nodeapi hook. Patch is included as well.

case 'view':
case 'load':
	$node->flags = flag_get_counts("node", $node->nid);
	break;
CommentFileSizeAuthor
flag.module.diff375 bytespianomansam

Comments

quicksketch’s picture

quicksketch
he/him
commented

Yes this is possible, but probably unnecessary. Why should we unconditionally cause extra data to be loaded into every node when it only takes a single function call to get that data?

mooffie’s picture

mooffie commented

(This is a duplicate of #302062: Expose user-defined flags to modules/themes via $node)

quicksketch’s picture

quicksketch
he/him
commented

It's very similar, but it's not quite a duplicate since #302062 just puts Flag objects into the $node object. This issue should probably be setting $node->flag_counts and not $node->flags, since it's not really the applicable flags that are being added to the $node. This patch also has a greater performance hit than the other issue, since the flag counts are NOT always loaded.

pianomansam’s picture

pianomansam commented

Good points. For me, it made sense to put the flags inside the node object to keep the logic (controller) separate from the display (view). Do what you will with it, but I do think there should be some easy way to grab the flags for a node without having to use PHP.

mooffie’s picture

mooffie commented

For me, it made sense to put the flags inside the node object to keep the logic (controller) separate from the display (view).
[...] without having to use PHP.

You have some misunderstanding of the MVC paradigm:

  1. You're confusing between "logic" and "buisness logic". The MVC paradigm nowhere says "don't put logic in your view". You can do virtually nothing, in your views, without logic.
  2. You're in fact breaking the MVC paradigm because you put views stuff (loading flagging statistics) in the controller (nodeapi).

===

What you're looking for is to make your views cleaner. That's a very good thing. So you can move your code to a node template preprocess function. This way it won't affect performance. (Another possibility: suggest us some useful query method(s) to add to the Flag handler class.)

Do what you will with it, but I do think there should [...]

When you load things unconditionally onto $node that's the opposite of lazy programming and that's something we should avoid. If we weren't hindered by the PHP language (or by current Drupal programming conventions) we could add a lazy method/property to $node, but we can't. So what we provide is the $flag handler. Perhaps, as Drpual evolves, things would change.

mooffie’s picture

mooffie commented

What you're looking for is to make your views cleaner. That's a very good thing. So you can move your code to a node template preprocess function.

Examples for this technique you'll find in the handbook. For example, in the "How to add CSS classes to nodes, based on flags set" page.

mitchell’s picture

mitchell commented
Version: 5.x-1.1 » 6.x-1.1
Component: Code » Documentation
Category: feature » support
Status: Needs review » Fixed

I personally do stuff like this using with views block. See the documentation page on How to setup a "Who's bookmarked this" tab. It's uses the same principle of setting the default argument based on the page you're looking at.

quicksketch’s picture

quicksketch
he/him
commented
Component: Documentation » Flag core
Category: support » feature
Status: Fixed » Active

I'd like to keep this issue open as something to consider. When answering #474410: Display image on node teaser/body if certain flag is checked, I thought it would be very helpful for *global* flags to be available when doing theming.

quicksketch’s picture

quicksketch
he/him
commented
Status: Active » Closed (won't fix)

Well after thinking about it further, I think our current approach is just fine. Loading things into $node is convenient but not particularly necessary. Let's stick with being efficient and not add this option.