Patch: Some improvements

Hello,

Thank you for your contribution!
I will do my best to merge it to our code shortly

However short question:
The redirect to login page works well with the shortcode?

As we used to have redirect to login page but then removed it as redirect didn’t work with shortcode.

Regards,
Marco

Hello,

That sounds good!

I’ve documented the most important code, which explains the part you mention about redirection. You’re right that it normally won’t work because headers will already have been sent; however, you can hook into actions that happen before rendering begins, then look for the shortcode within the post content.

The negative side is that the (extra) lookup doesn’t only happen just when the shortcode is used.

I’m not quite sure what will happen if the shortcode is used outside of an actual post/page, or used in other unusual situations, so there may be room for more testing.

Ah, i see.
I didn’t know that part.

We still getting the hang of WordPress plugin development and limitations, before we worked with drupal which is a bit more easy from a development perspective haha ??

The testing part: I assume for the most part it will be alright, don’t think many people will put tickets listing in odd places.

No worries. I’m pretty sure I have less experience with WordPress than you have, and that’s one of the reasons why I’m careful to suggest that my code is good. ??

I saw another idea to workaround the mentioned problem, and that was to use hooks to enable output buffering right before rendering, then disable it after rendering.

The method I decided to use – while it’s unlikely to change (any time soon) – will always depend on the current syntax for shortcodes.

Because I don’t like the idea of a widget, shortcode, or any other small part make a big (intrusive) decision like enabling buffering for the whole page on each request, I thought the string-search was better.

I’m not sure how I missed it but there’s a problem with the code I posted earlier (broken HTML).

I would like to mention that although I used wp_enqueue_*() where I could, it works exactly the same way as before. It would be more proper to hook the actions wp_enqueue_scripts and admin_enqueue_scripts and register scripts/styles within the callback function.

Also, since I think you’ve mentioned that you’re cleaning up code, I think that plugins_url('...', __FILE__) looks nicer than plugin_dir_url(__FILE__).'...'.

New patch:

Index: admin/ost-config.php
===================================================================
--- admin/ost-config.php	(revision 968450)
+++ admin/ost-config.php	(working copy)
@@ -121,4 +121,4 @@
 </div>
 </form>
 </div><!--End of wrap-->
-<script language="javascript" src="<?php echo plugin_dir_url(__FILE__).'../js/fade.js';?>"></script>
\ No newline at end of file
+<?php wp_enqueue_script('ost-bridge-fade', plugin_dir_url(__FILE__).'../js/fade.js');?>
\ No newline at end of file
Index: admin/ost-ticketview.php
===================================================================
--- admin/ost-ticketview.php	(revision 968450)
+++ admin/ost-ticketview.php	(working copy)
@@ -141,4 +141,4 @@
 </tr>
 </table>
 </div><!--End wrap-->
-<script language="javascript" src="<?php echo plugin_dir_url(__FILE__).'../js/fade.js';?>"></script>
\ No newline at end of file
+<?php wp_enqueue_script('ost-bridge-fade', plugin_dir_url(__FILE__).'../js/fade.js');?>
\ No newline at end of file
Index: ost-bridge.php
===================================================================
--- ost-bridge.php	(revision 968450)
+++ ost-bridge.php	(working copy)
@@ -41,7 +41,8 @@

 	require_once( WP_PLUGIN_DIR . '/key4ce-osticket-bridge/osticket-wp.php');
 }
-add_shortcode('addosticket', 'addtemplate');
+define('OST_SHORTCODE_ADDOSTICKET', 'addosticket');
+add_shortcode(OST_SHORTCODE_ADDOSTICKET, 'addtemplate');
 function custom_toolbar_openticket() {
 	global $wp_admin_bar;
 	$wp_admin_bar->add_menu(array(
@@ -127,7 +128,7 @@
 } }

 function mb_admin_css() {
-echo '<link rel="stylesheet" type="text/css" media="all" href="'.plugin_dir_url(__FILE__).'css/admin-style.css">';
+wp_enqueue_style('ost-bridge-admin', plugin_dir_url(__FILE__).'css/admin-style.css');
 }

 function mb_install()
@@ -285,4 +286,34 @@
     }
   }
 }
+
+// Begin: Workaround when redirecting from shortcodes.
+// Because we potentially need to redirect within a shortcode, we must make sure that headers are not sent beforehand.
+// Ref.: https://stackoverflow.com/questions/17120634/how-to-require-a-login-on-a-wordpress-page-with-a-shortcode/17124281#17124281
+
+// Looks for a shortcode within the current post's content.
+// Optimized for shortcodes that don't have parameters.
+function ost_has_shortcode_without_params($shortcode = '') {
+  global $post;
+
+  if (!$shortcode || $post == null) {
+    return false;
+  }
+
+  if (stripos($post->post_content, '[' . $shortcode . ']') === false) {
+    return false;
+  }
+
+  return true;
+}
+
+// User must be logged in to view pages that use the shortcode
+function ost_enforce_login_action() {
+  if(ost_has_shortcode_without_params(OST_SHORTCODE_ADDOSTICKET) && !is_user_logged_in()) {
+    auth_redirect();
+  }
+}
+
+add_action('wp', 'ost_enforce_login_action');
+// End: Workaround when redirecting from shortcodes.
 ?>
Index: osticket-wp.php
===================================================================
--- osticket-wp.php	(revision 968450)
+++ osticket-wp.php	(working copy)
@@ -5,8 +5,8 @@
 if(is_user_logged_in()) {
 $config = get_option('os_ticket_config');
 extract($config);
-?>
-<link rel="stylesheet" type="text/css" media="all" href="<?php echo plugin_dir_url(__FILE__).'css/style.css'; ?>" />
+
+wp_enqueue_style('ost-bridge', plugin_dir_url(__FILE__).'css/style.css'); ?>
 <div id="ost_container"><!--ost_container Start-->
 <?php require_once( WP_PLUGIN_DIR . '/key4ce-osticket-bridge/includes/functions.php' ); ?>
 <?php
@@ -108,23 +108,5 @@
 </div><!--ost_container End-->
 <?php
 } else {
-	$Login_args = array(
-        'echo'           => true,
-        'redirect'       => site_url( $_SERVER['REQUEST_URI'] ),
-        'form_id'        => 'loginform',
-        'label_username' => __( 'Username' ),
-        'label_password' => __( 'Password' ),
-        'label_remember' => __( 'Remember Me' ),
-        'label_log_in'   => __( 'Log In' ),
-        'id_username'    => 'user_login',
-        'id_password'    => 'user_pass',
-        'id_remember'    => 'rememberme',
-        'id_submit'      => 'wp-submit',
-        'remember'       => true,
-        'value_username' => NULL,
-        'value_remember' => false
-);
-?><div><br /><br /> <h3>Sorry, you must first log in to view your tickets. If you do not have a account yet you can <a style="color: #2991D6;" href="<?php echo wp_registration_url(); ?>">register here</a>. </h3>
-<br /><br />
-<?php wp_login_form(@$login_args); ?>
-</div><?php } ?>
\ No newline at end of file
+throw new Exception('Should not happen: User is not logged in');
+} ?>
Index: templates/nav_bar.php
===================================================================
--- templates/nav_bar.php	(revision 968450)
+++ templates/nav_bar.php	(working copy)
@@ -52,5 +52,5 @@
 	echo '<div style="clear: both"></div>';
 	echo '</div>';
     echo '</div><hr style="border-color:#D5E5EE; border-width: 3px; height: 3px;">';
-?>
-<script type="text/javascript" src="<?php echo plugin_dir_url(__FILE__).'../js/validate.js';?>"></script>
\ No newline at end of file
+
+wp_enqueue_script('ost-bridge-validate', plugin_dir_url(__FILE__).'../js/validate.js');
\ No newline at end of file
Index: templates/new_ticket.php
===================================================================
--- templates/new_ticket.php	(revision 968450)
+++ templates/new_ticket.php	(working copy)
@@ -47,7 +47,7 @@
 <div id="new_ticket_subject">Subject:</div>
 <div id="new_ticket_subject_input"><input class="ost" id="subject" type="text" name="subject" size="35"/><font class="error">?*</font></div>
 <div style="clear: both"></div>
-<div id="new_ticket_catagory">Catagories:</div>
+<div id="new_ticket_catagory">Categories:</div>
 <div id="new_ticket_catagory_input">
 <select id="deptId" name="deptId">
 <option value="" selected="selected"> Select a Category </option>

Thanks,

I have updated the plugin with your latest patch.
I also added to the release notes you contributed them (using your username).

Can you verify it works OK with 1.1.5?

Thank you! Yes, it seemed to work alright in v1.1.5. I found other problems that I started fixing, but I can now see that v1.1.5 has disappeared from this site, so I’ll wait with posting the patch.

yes! and sorry
will re-publish tomorrow.
found out shortly after punlish on my own live sites that the contact form short code failed horridly.

so took it back offline.
I assume those issues will be sorted by tomorrow.
If you like i can provide you with current code of 1.1.5 but the contact page shortcode isn’t working proper there. (you can check for other issues and/or help fix contact page tho :))

Currently our dev is sleeping so it will be fixed tomorrow when his back ??

No problem!

I believe I fixed only some small issues on the contact page, probably not the things that failed horridly in your case. I noticed somewhat serious issues on that page though, so if I wait, I may not need to comment on those. I’m however still interested in keeping my patch in sync with your development branch, even if only temporary.

I’ve decided to post one last patch because I’ve realized that fixing up the old code will be quite a bit of work, and it will be pretty messy and error-prone. I would actually recommend rewriting the whole thing from scratch.

Ah ??
well, if you got any fixes so far always happy to hear them ??
( i do aim to give credit to every fix you make though wp makes it a bit difficult).

altho new contact shortcode can be ignored as it pretty much fails at basics (not showing up for not-logged in people).

your previous patches should have been applied tho. (i did add your name in 1.1.5 readme to. if you prefer a different name then your username please let me know and i will update the readme with it)

Just to clarify: I meant rewriting the whole plugin, not (only) the contact page. I understand your work is based on the work of someone else, so what I wanted to recommend was to get rid of the old code completely rather than fixing it. ??

By the way, my use of wp_enqueue_*() was not really a fix, but makes the code a little cleaner; it could however be improved by using those with proper hooks like mentioned earlier.

If you would like to give me credit, then I appreciate it! You can use my real name as seen on my website:
https://www.steffenl.com/

haha well getting rid of old code completely takes time.
Always alot of things to do, change, fix, add yet so little time to do it.

i have added you in the changelog with your real (full) name

Hello I’d like to be able to use the following deregister code in my custom functions.php – since it looks like you are now using enqueue in the plugin can you confirm wether this will work as I’ve not had much success so far?

I wish to deregister the ost-bridge-admin style for a particular page in my Dashboard. The code function call is:

wp_deregister_style( 'ost-bridge-admin' );

e.g.:

add_action( 'admin_print_styles ', 'my_deregister_css', 100 );

function my_deregister_css() {
   if ( is_page('wysija_campaigns') ) {
	wp_deregister_style( 'ost-bridge-admin' );
     }
}

Thank you