-
Notifications
You must be signed in to change notification settings - Fork 472
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/6923 - Include Plugin URI in the System Info report #8775
issue/6923 - Include Plugin URI in the System Info report #8775
Conversation
Include the Plugin URI or Author URI in the plugins list
Include plugin URI in System Info
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks good, @mihaijoldis, thanks! Just made a suggestion (x3) which might make the logic a little bit safer, maybe.
includes/admin/tools.php
Outdated
@@ -1377,7 +1377,8 @@ function edd_tools_sysinfo_get() { | |||
continue; | |||
|
|||
$update = ( array_key_exists( $plugin_path, $updates ) ) ? ' (needs update - ' . $updates[$plugin_path]->update->new_version . ')' : ''; | |||
$return .= $plugin['Name'] . ': ' . $plugin['Version'] . $update . "\n"; | |||
$plugin_url = isset( $plugin['PluginURI'] ) && ! empty( $plugin['PluginURI'] ) ? $plugin['PluginURI'] : $plugin['AuthorURI']; | |||
$return .= $plugin['Name'] . ': ' . $plugin['Version'] . $update . ' - ' . $plugin_url . "\n"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just a question for you--as you look at the list, if a plugin needs an update, is it more helpful to have them in the order you do already, or would you want the $update
bit at the end? I'm seeing this:
Advanced Custom Fields PRO: 5.9.5 (needs update - 5.9.9) - https://www.advancedcustomfields.com
AffiliateWP: 2.7-beta1 (needs update - 2.8-alpha2) - https://affiliatewp.com
(I am just asking--either way is fine with me.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think its easier and more "in front" of you to see the Needs update message right at the start than having to perhaps scroll to the right if its a long URI
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@mihaijoldis this is looking great, but I discovered a plugin in my site which meets none of these parameters, so the output is:
Plugin Name: 1.5 -
What would you think of updating the logic a little to drop that if the $plugin_url
can't be retrieved? Something like:
if ( $plugin_url ) {
$plugin_url = ' — ' . $plugin_url;
}
$return .= $plugin['Name'] . ': ' . $plugin['Version'] . $update . $plugin_url . "\n";
(I threw in the —
as our extension names have a hyphen in them already and thought that might help differentiate a bit.)
@robincornett Do you think its too much to also add a final else check and put the Author in there just in case the URI and Author URI are not set ?
I noticed on my local site that I have one of our snippets from hte library that has neither of the PluginURI/AuthorURI set but it does the Author so mybe even if it just shows this Its still helpful? |
Sure, I would say just make it a final elseif rather than an else, so you're never trying to use something that doesn't exist. |
@robincornett PR updated with latest changes. |
Fixes #6923
Proposed Changes: