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

Issue 305: Add action link to edit Stream Settings #317

Merged
merged 21 commits into from
Mar 11, 2014
Merged

Conversation

powelski
Copy link

@powelski powelski commented Mar 9, 2014

Fix #305

@powelski
Copy link
Author

powelski commented Mar 9, 2014

That one was tricky!

@@ -217,14 +218,20 @@ public static function action_links( $links, $record ) {
array( 2 => $submenu_slug )
);

if ( ! empty( $found_submenus ) ) {
if ( ! empty( $found_submenus ) || $record->context === 'wp_stream' ) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

Done in 45fad36

@frankiejarrett
Copy link
Contributor

@powelski Sorry, this doesn't appear to be working for me. There is no action link displayed.

screen shot 2014-03-09 at 7 52 41 pm

@powelski
Copy link
Author

@fjarrett Please make sure once again - I checked it again now and it works fine for me.

What needs to be noticed, settings of Stream will be highlighted (after clicking action link) since this update. Previously, we saved only option name in meta, which is wp_stream. I made it save both - option name in option meta (I didn't change that) plus option_key meta when option is serialized.

@powelski
Copy link
Author

Voilà:

action link

@frankiejarrett
Copy link
Contributor

@powelski I just still can't see the action link appearing at all here.

@shadyvb
Copy link
Contributor

shadyvb commented Mar 10, 2014

@fjarrett I think @powelski is saying this should only appear in new records,

I made it save both - option name in option meta (I didn't change that) plus option_key meta when option is serialized.

are you trying newly created records ?

@powelski
Copy link
Author

@fjarrett @shadyvb This is true, but looking at Frankie's screenshot I assumed he tested on newly created entry. @shadyvb could you confirm if it works for you?

@frankiejarrett
Copy link
Contributor

@shadyvb @powelski Yes, I am trying both newly-created and existing records and cannot see the action link in either.

Conflicts:
	connectors/settings.php
$field_name = get_stream_meta( $record->ID, 'option', true );
$text = sprintf( __( 'Edit %s Settings', 'stream' ), $context_labels[$record->context] );

$url = apply_filters( 'wp_stream_action_link_url', admin_url( $submenu_slug ), $record );
Copy link
Contributor

Choose a reason for hiding this comment

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

@powelski Please add a docblock for this new filter. See #127

@powelski
Copy link
Author

@fjarrett I checked on my updated environment and it didn't work for me to. I applied one small fix - now it should work for you.

@powelski
Copy link
Author

I have improved logics for action links generator in #309, so this solution will be much more elegant once #319 is merged.

@powelski
Copy link
Author

In addition, capabilities required to view particular settings page will automatically be checked before displaying links - this will occur for wp_stream capability as well and any other capability assigned to any menu page in future.

@frankiejarrett
Copy link
Contributor

@powelski Great! This is working nicely for me.

Now we have to ensure backwards compatibility with past Stream Settings records, can this be achieved with an upgrade routine?

@frankiejarrett frankiejarrett merged commit ff88ed8 into develop Mar 11, 2014
@frankiejarrett
Copy link
Contributor

@powelski This is going out as-is, but we need to do the upgrade routine for the next release. See #328

@frankiejarrett frankiejarrett deleted the issue-305 branch March 11, 2014 18:41
@frankiejarrett
Copy link
Contributor

@powelski I have this in the master branch and now the feature is not working. The action link will not display.

@powelski
Copy link
Author

@fjarrett Are you sure you have proper capabilities? Make sure you can access Stream Settings page in standard way.

@frankiejarrett
Copy link
Contributor

@powelski Yes, I'm sure of it. That's how I'm creating new records is by accessing the Stream > Settings page and making changes.

@frankiejarrett
Copy link
Contributor

@powelski OK I figured it out. It seems that dd8df71 and ff88ed8 were both overwritten by a1bc6aa in issue-309.

Not sure what to do here myself, I think you will need to look into it and make the correct changes. Please do so directly in the master branch as a hotfix. I will hold off on the 1.3.0 release until this fix is applied.

I think this is the code that is needed:

if ( ! empty( $found_submenus ) || 'wp_stream' === $record->context ) {
    $target_submenu = array_pop( $found_submenus );

    if ( ! is_array( $target_submenu ) || current_user_can( $target_submenu[1] ) ) {
        $text = sprintf( __( 'Edit %s Settings', 'stream' ), $context_labels[ $record->context ] );

        $url = apply_filters( 'wp_stream_action_link_url', admin_url( $submenu_slug ), $record );

        $field_name = get_stream_meta( $record->ID, 'option_key', true );

        if ( '' === $field_name ) {
            $field_name = get_stream_meta( $record->ID, 'option', true );
        }

        if ( '' !== $field_name ) {
            $url = sprintf( '%s#%s%s', rtrim( preg_replace( '/#.*/', '', $url ), '/' ), self::HIGHLIGHT_FIELD_URL_HASH_PREFIX, $field_name );
        }

        $links[ $text ] = $url;
    }
}

@powelski
Copy link
Author

@fjarrett I'm taking care of it now. As I mentioned before, we will use nicer code now for Stream action link - this $rules array from #309 makes it much easier to maintain.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add action link to edit Stream Settings
3 participants