Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve flexibility of context actions #374

Merged
merged 11 commits into from Jan 4, 2016
39 changes: 25 additions & 14 deletions fieldmanager.php
Expand Up @@ -243,12 +243,15 @@ function _fieldmanager_registry( $var, $val = null ) {
*
* @see fm_calculate_context() for detail about the returned array values.
*
* @param bool $force Optional. If true, FM will recalculate the current context.
* This is necessary for testing and perhaps other
* programmatic purposes.
* @return array Contextual information for the current request.
*/
function fm_get_context() {
function fm_get_context( $force = false ) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To me, $force reads as if you're forcing FM to return something when it otherwise wouldn't, which isn't quite what's happening, if I'm following correctly. Maybe FM could do with a dedicated clear or recalculate workflow, or a way to get a new calculated context independently of the static one? (I realize this comment is coming a little late in the 1.0 cycle, though.)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good feedback. I think in the short term, I'll just rename this to $recalculate.

static $calculated_context;

if ( $calculated_context ) {
if ( ! $force && $calculated_context ) {
return $calculated_context;
} else {
$calculated_context = fm_calculate_context();
Expand Down Expand Up @@ -408,30 +411,38 @@ function fm_match_context( $context, $type = null ) {
*/
function fm_trigger_context_action() {
$calculated_context = fm_get_context();
if ( empty( $calculated_context ) ) {
if ( empty( $calculated_context[0] ) ) {
return;
}

$context = $calculated_context[0];
if ( $type = $calculated_context[1] ) {
list( $context, $type ) = $calculated_context;

if ( $type ) {
/**
* Fires when a specific Fieldmanager context and type load.
*
* The dynamic portions of the hook name, $context and $type, refer to
* the values returned by fm_calculate_context(). For example, the Edit
* screen for the Page post type would fire "fm_post_page".
*/
do_action( "fm_{$context}_{$type}" );
} else {
/**
* Fires when a specific Fieldmanager context, but not type, loads.
*
* The dynamic portion of the hook name, $context, refers to the first
* value returned by fm_calculate_context(). For example, the Edit User
* screen would fire "fm_user".
* @param string $type The context subtype, e.g. the post type, taxonomy
* name, submenu option name.
*/
do_action( "fm_{$context}" );
do_action( "fm_{$context}_{$type}", $type );
}

/**
* Fires when any Fieldmanager context loads.
*
* The dynamic portion of the hook name, $context, refers to the first
* value returned by fm_calculate_context(). For example, the Edit User
* screen would fire "fm_user".
*
* @param string|null $type The context subtype, e.g. the post type,
* taxonomy name, submenu option name. null if this
* context does not have a subtype.
*/
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would note that the new @params are missing docs.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point, thanks!

do_action( "fm_{$context}", $type );
}
add_action( 'init', 'fm_trigger_context_action', 99 );

Expand Down
96 changes: 96 additions & 0 deletions tests/php/test-fieldmanager-calculate-context.php
Expand Up @@ -26,6 +26,45 @@ public function tearDown( $value = null ) {
_fieldmanager_registry( 'submenus', $this->submenus );
}

/**
* Test arbitrary context actions given a context and type.
*
* @param string $context The context being tested.
* @param string $type The subcontext being tested.
*/
protected function _context_action_assertions( $context, $type ) {
$a = new MockAction();

if ( $context ) {
add_action( "fm_{$context}", array( &$a, 'action' ) );
}
if ( $type ) {
add_action( "fm_{$context}_{$type}", array( &$a, 'action' ) );
}

fm_get_context( true );
fm_trigger_context_action();

if ( $type ) {
// only two events occurred for the hook
$this->assertEquals( 2, $a->get_call_count() );
// only our hooks were called
$this->assertEquals( array( "fm_{$context}_{$type}", "fm_{$context}" ), $a->get_tags() );
// The $type should have been passed as args
$this->assertEquals( array( array( $type ), array( $type ) ), $a->get_args() );
} elseif ( $context ) {
// only one event occurred for the hook
$this->assertEquals( 1, $a->get_call_count() );
// only our hook was called
$this->assertEquals( array( "fm_{$context}" ), $a->get_tags() );
// null should have been passed as an arg
$this->assertEquals( array( array( null ) ), $a->get_args() );
} else {
// No event should have fired
$this->assertEquals( 0, $a->get_call_count() );
}
}

/**
* Provide data for test_submenu_contexts.
*
Expand Down Expand Up @@ -69,6 +108,7 @@ public function test_submenu_contexts( $parent, $php_self = null ) {
}
$_GET['page'] = $submenu;
$this->assertEquals( array( 'submenu', $submenu ), fm_calculate_context() );
$this->_context_action_assertions( 'submenu', $submenu );
}

/**
Expand All @@ -78,5 +118,61 @@ public function test_non_fm_submenu() {
$_SERVER['PHP_SELF'] = '/themes.php';
$_GET['page'] = rand_str();
$this->assertEquals( array( null, null ), fm_calculate_context() );
$this->_context_action_assertions( null, null );
}

/**
* Provide data for test_basic_contexts.
*
* @see https://phpunit.de/manual/4.7/en/writing-tests-for-phpunit.html#writing-tests-for-phpunit.data-providers
*
* @return array
*/
public function basic_contexts_args() {
return array(
array( '/post-new.php', array( 'post_type' => 'page' ), 'post', 'page' ),
array( '/profile.php', null, 'user', null ),
array( '/user-edit.php', null, 'user', null ),
array( '/edit.php', array( 'post_type' => 'page' ), 'quickedit', 'page' ),
array( '/edit-tags.php', array( 'taxonomy' => 'category' ), 'term', 'category' ),
);
}

/**
* Test context calculations for assorted contexts.
*
* @dataProvider basic_contexts_args
*/
public function test_basic_contexts( $self_override, $get_override, $context, $type ) {
$_SERVER['PHP_SELF'] = $self_override;
$_GET = $get_override;
$this->assertEquals( array( $context, $type ), fm_calculate_context() );
$this->_context_action_assertions( $context, $type );
}

public function test_post_screen_context() {
$_SERVER['PHP_SELF'] = '/post.php';
$post_id = $this->factory->post->create( array( 'post_title' => rand_str(), 'post_date' => '2015-07-01 00:00:00', 'post_type' => 'page' ) );
$_GET = array( 'post' => strval( $post_id ) );
$this->assertEquals( array( 'post', 'page' ), fm_calculate_context() );
$this->_context_action_assertions( 'post', 'page' );
}

public function test_post_save_screen_context() {
$_SERVER['PHP_SELF'] = '/post.php';
$_POST = array( 'action' => 'editpost', 'post_type' => 'page' );
$this->assertEquals( array( 'post', 'page' ), fm_calculate_context() );
$this->_context_action_assertions( 'post', 'page' );
}

public function test_ajax_direct_context() {
$_SERVER['PHP_SELF'] = '/admin-ajax.php';
$_POST = array( 'fm_context' => 'foo' );
$this->assertEquals( array( 'foo', null ), fm_calculate_context() );
$this->_context_action_assertions( 'foo', null );

$_POST = array( 'fm_context' => 'foo', 'fm_subcontext' => 'bar' );
$this->assertEquals( array( 'foo', 'bar' ), fm_calculate_context() );
$this->_context_action_assertions( 'foo', 'bar' );
}
}