Problem/Motivation

The procedural function flag() in flag.module is basically just a wrapper for flag_flag::flag() in flag.inc (implemented via $flag->flag()). However, the latter OO function has $skip_permission_check as a parameter whilst the former lacks this. For the sake of API consistency, the $skip_permission_check should be added to flag().

Proposed resolution

Provide $skip_permission_check as an optional boolean argument for flag(). Alternatively, deprecate the flag() function in favor of $flag->flag().

Remaining tasks

Patch rolling shortly, reviews needed. Also, arguments whether to deprecate flag()?

User interface changes

None

API changes

The procedural function flag() now accepts $skip_permission_check as an optional argument, defaults to FALSE.

Comments

joachim’s picture

joachim commented

> Alternatively, deprecate the flag() function in favor of $flag->flag().

I'm tempted.

Or, keep the procedural flag() but don't make it as fully-featured as the method.

#871064: Making flaggings fieldable adds yet another parameter to the method :/

socketwench’s picture

socketwench commented

Or, keep the procedural flag() but don't make it as fully-featured as the method.

I'm much more in favor of object APIs myself. The downside is that this is already an established API call. If we're going to break something, let's break it once to allow for a better API.

joachim’s picture

joachim commented
Version: 7.x-2.x-dev » 7.x-3.x-dev

Hmmm this one fell off my radar.

If we're going to break APIs we need to decide on something soon, before 3.x reaches beta release.

socketwench’s picture

socketwench commented

Let's not change everything at once. We're already moving everything to entities, and that's a big enough change already. Unless we want more headaches, let's let that simmer and focus on an object oriented API in 4.x?

ivnish’s picture

ivnish
he/him
commented
Issue summary: View changes
Status: Active » Closed (outdated)

Drupal 7 is EOL. No more new features will be added.