There's no reason for _flag_entity_enabled() to be a helper function.
- it's pseudoprivate already
- the only place it's called is flag_get_flags(), which has access to $flag objects (and indeed, the next test calls $flag->user_access())
The only question is what to call its replacement, as we already have a slew of methods called foo_access() and access_foo().
As part of the rewrite, breaking up the complex expression would be good too -- see coding standards item called 'Control structure conditions should also NOT attempt to win the Most Compact Condition In Least Lines Of Code Award™' ;)
| Comment | File | Size | Author |
|---|---|---|---|
| #1 | flag-access_entity_enabled-1784112.patch | 2.29 KB | laughnan |
Comments
Comment #1
laughnanI am fairly new a creating git patches, but here is novice attempt.
- removed _flag_entity_enabled() function in the flag.module file
- created access_entity_enabled() method in the flag base class (flag_flag.inc)
If I didn't approach this correctly, please let me know. I'm just starting out and want to be more involved.
::Alex
Comment #2
joachim commentedPatch looks pretty good.
Remember to set the issue to 'needs review' so other people and the testbot notice it. Let's see what the testbot says now...
Comment #3
laughnanThank you @joachim!
Comment #4
joachim commentedTestbot likes it too!
Committed. Thanks.
Issue #1784112 by laughnan: Moved _flag_entity_enabled() to a method of flag base class."
I'll file a follow-up for refactoring the expression -- makes more sense to do it this way so the code move can be seen as just a code move.