This issue has some vague resemblance to #285237: Ability to disallow a flag/unflag operation. If anyone has some feedback about whether this is a worthy method or on the contrary a very bad idea, I am happy to be educated.

I'm trying to use flags to create a more versatile blogs setup. Users can flag their own content into their blog. I don't want 10 users to add the same 2 nodes to their collections of blog posts, so I the flag is global. The flag settings determine which types of content can be in users' blogs, so I don't have to revise my views whenever I need to allow new types of content to be added to user blogs. I can restrict the use of the flag to particular roles (though it would be better if I could use specific permissions).

In this design, the flag is a property of the node. Of all the nodes that might be created by a user, some 'type x' nodes are in that user's blog and others are not. So I wanted to use node_access to restrict who could see and use the flag, because it's not just about the permission to use the flag, it's also about the user's permissions vis-a-vis the node.

The default node_access is "view." If the user can't even see the node, is there a conceivable reason for the user to flag it? For my purposes, I want to restrict the flag to users who can update the node.

In coming up with this, I thought that it would be nice if I could define multiple flag handlers for the same flag content type.

I also accidentally brutally crushed the "applies_to_content_object" method. I brought the user in there, and it's always going to be the logged in user.

use at your own risk... it's late!

CommentFileSizeAuthor
#3 flag-node-access.patch1.68 KBAnonymous (not verified)
flag-node-access.patch3.01 KBAnonymous (not verified)

Comments

mooffie’s picture

mooffie commented

This issue has some vague resemblance to #285237: Ability to disallow a flag/unflag operation
[...]
because it's not just about the permission to use the flag, it's also about the user's permissions vis-a-vis the node.

I don't see why this has only a "vague resemblance" to #285237. That issue deals exactly with your concerns. How is it different?

I've just opened a new issue, #322034: Have a $flag->access() method, that introduce one building block of #285237.

Anonymous’s picture

Anonymous (not verified) commented

It has a vague resemblance because I stumbled on the issue after I had already written the code, and I wrote the code to implement a narrower bunch of features than #285237: Ability to disallow a flag/unflag operation considers. For instance, my patch isn't about slicing access permissions between flag and unflag, and I really want to rely on node_access rather than side-stepping it by making assumptions about what the node's author can do to the node.

But the new task is good. I'll refactor. Thanks!

Anonymous’s picture

Anonymous (not verified) commented
StatusFileSize
new flag-node-access.patch1.68 KB

With the patch I submitted at #322034: Have a $flag->access() method, this feature can now be implemented with a much smaller patch.

This doesn't slice permissions between flag and unflag.

If a node flag's use_node_access property is "view," the flag behaves like flags presently do. They appear on any node the user can see.

For "update," the flag does what I need it to do...

I can't identify a use case for the "create" permission. This node access permission has nothing to do with the particular node, so the flags become a weird and wrong version of the default.

For "delete" it could be a way to develop a kind of trash can limbo for nodes. It doesn't remove the nodes from the site, but it does mark them for deletion.

mooffie’s picture

mooffie commented

Benjamin, thanks for contributing.

Please see my reply at #322034: Have a $flag->access() method (I didn't notice your new comment here before I replied there.)

BTW, I posted a re-roll of the patch there, and note that in my pseudo code I give modules a chance to veto the action. So your feature could also be implemented by implementing a hook.

The access checks described at #285237: Ability to disallow a flag/unflag operation seem to be more useful, so, I think, we should begin with them.

mooffie’s picture

mooffie commented

$node = $this->_load_content($content_id);

1. There's $this->fetch_content().
2. We want to defer fetching the node (an action which may trigger DB action) till we have to. So I think it's better to call parent::access() first and continue with node_access() only if the parent returned TRUE.

mooffie’s picture

mooffie commented

See the new comment I added to #285237. "All access checks need to be duplicated in applies_to_content_id_array()". We don't want to load nodes in that method, so there you can't call node_access().

mitchell’s picture

mitchell commented
Component: Code » Flag core
Status: Needs review » Closed (duplicate)

Marking as a duplicate of #322034: Have a $flag->access() method.