It's time to reap the benefits of our OOP, and to squash some bugs along the way.
The purpose of this patch is to clean up flag_link(), flag(), create_flag_link().
The patch simplifies these functions by introducting a handful of methods on the flag object:
$flag->user_access($account)
(Can the user acces this flag?)
$flag->applies_to_content($object)
(Does the flag apply to a certain piece of content?)
$flag->uses_hook_link($teaser)
(Is the flag configured to show links in hook_link?)
$flag->is_flagged($content_id, $uid = NULL)
(Is a piece of content flagged?)
(And the are three new private methods to handle flagging CRUD: $flag->_flag(), $flag->_unflag(), $flag->_update_count(). The purpose here is to insulate the main code from the details.)
Surpringly, using only these methods the code in 'flag.module' is greatly simplified.
For example, after applying this patch flag_link will look like:
function flag_link($type, $object = NULL, $teaser = FALSE) {
if (!isset($object) || !flag_fetch_definition($type)) {
return;
}
global $user;
// Anonymous users can't create flags with this system.
if (!$user->uid) {
return;
}
// Get all possible flags for this content-type.
$flags = flag_get_flags($type);
foreach ($flags as $flag) {
if (!$flag->user_access($user)) {
// User has no permission to use this flag.
continue;
}
if (!$flag->uses_hook_link($teaser)) {
// Flag is not configured to show its link here.
continue;
}
if (!$flag->applies_to_content_object($object)) {
// Flag does not apply to this content.
continue;
}
$content_id = $flag->get_content_id($object);
// Token replacements.
$flag = flag_process_labels($flag, $type, $content_id, array('flag_short', 'flag_long', 'flag_message'), array('teaser' => $teaser));
// The flag links are actually fully rendered theme functions.
// The HTML attribute is set to TRUE to allow whatever the themer desires.
$links['flag-'. $flag->name] = array(
'title' => $flag->theme($flag->is_flagged($content_id) ? 'unflag' : 'flag', $content_id),
'html' => TRUE,
);
}
if (isset($links)) {
return $links;
}
}
(In this example the code hasn't been shortened much, but it's more straightforward, and putting all the access control around one place makes other features easier to implement.)
Some of the benefits:
* The "does this flag applies to X?" bugs are now gone. Both flag() and create_flag_link() suffered from this bug: either completely or partially, since they partially duplicated the needed code (which is now tucked in $flag->applies_to_content_object()).
* Other similar bugs are less likely to happen in the main module: since the flow is now quite clear by just reading the code, there's not much opportunity to err.
* Support for flagging users will soon be "a piece of cake" to implement: notice that words such as "node" and "comment" do not appear in the code above.
| Comment | File | Size | Author |
|---|---|---|---|
| oopcleanup.diff | 15.73 KB | mooffie |
Comments
Comment #1
mooffie commentedFixed.
http://drupal.org/cvs?commit=133480
http://drupal.org/cvs?commit=133517
Comment #2
quicksketchGreat work mooffie!
Comment #3
Anonymous (not verified) commentedAutomatically closed -- issue fixed for two weeks with no activity.