Issue - postcalendar_userapi_pcQueryEvents

cfapress wrote on Friday, February 08, 2008:

Oh boy… I’ve gotten pretty deep now.

I’ve found that regardless of which providers are selected from the UI, all provider’s events are loaded from the database when querying. So, when your database grows large and/or you have many providers then the query will take longer to process and also consume more memory.

I’m digging through all this and have been adding little bits to the pnuserapi.php file. I’ll be proposing my changes once I’ve sort this out.

But here’s the quandry I’m working through. I see that somebody added this code:

$pc_username = $_SESSION[‘pc_username’]; // from Michael Brinson 2006-09-19
if (empty($pc_username) || is_array($pc_username)) {
  $pc_username = “__PC_ALL__”;
}

if(!empty($pc_username) && (strtolower($pc_username) != ‘anonymous’)) {
  if($pc_username==’__PC_ALL__’ || $pc_username == -1) {
    $ruserid = -1;
  } else {
    $ruserid = getIDfromUser($pc_username);
  }
}

Then later we check ruserid with this:
if(isset($ruserid)) { …

Well, because of the prior code checks for pc_username the value for ruserid will *always* be set to something. So we never reach these lines of code which impact the SQL query:

  } elseif(!pnUserLoggedIn()) {
    // get all events for anonymous users
    $sql .= “AND a.pc_sharing = '” . SHARING_GLOBAL . "’ ";
  } else {
    // get all events for logged in user plus global events
    $sql .= “AND (a.pc_aid = " . $_SESSION[‘authUserID’] . " OR a.pc_sharing = '” . SHARING_GLOBAL . "’) ";
  }

Is this getting ugly yet? I’m doing a stream of thought here, so please bear with me.

I’d like to change things around here because nowhere in the SQL query do we check for the chosen Provider IDs. I’ve got it nearly right and will have it working for my situation here. But I needed somebody else to chime in about why we have this $ruserid value clashing horribly with the remainder of the code.

It seems that when the $ruserid idea was added to the code we were limited to queries of the effect:
- get ALL events
- get events matching the logged in user only

Well that’s pretty broad and narrow at the same time.

I propose that the $_SESSION[‘pc_username]’ be taken out of this function. Then it could be passed into the function as a value just like everything else. This way we’re not overriding the function arguments with the overbearing SESSION value.

Sorry if this is pretty thick. As I said, I’m in quite deep right now. This is not a simple question to answer.

Jason

sunsetsystems wrote on Friday, February 08, 2008:

Yeah that code gives me a headache too.  It was before my time, but I guess what happened is that PostCalendar code was borrowed and adapted in a not-very-elegant way to work as the OpenEMR calendar.  Would be nice to see it re-thought and redesigned.

Rod
www.sunsetsystems.com

cfapress wrote on Friday, February 08, 2008:

Well… I’ve done a little tweaking and not much cleaning. I’d like to say my solution works well and makes the system faster but that remains to be seen. I sort of short-circuited part of the code by allowing the ‘provider_id’ argument precedence over the ‘pc_username’ argument.

Seems to work just fine but I’d suggest more testing. So far it has reduced the query result set from 35 rows to zero when I’m asking for events for providers who don’t have any. And I’m working with a *very* small group of events for testing. I can only imagine that it would make the system much faster if you’re dealing with 10+ providers and each with a full booking and you’re looking at a week-view or month-view.

This came up today because I’m adding a new calendar UI with some AJAX stuff. I’m keeping to the coding style used with the ‘Fancy UI’ so this calendar style can be per-user. I’m modeling it on the Google Calendar look-and-feel. But not quite as AJAX rich and slick.

jason

bradymiller wrote on Saturday, February 09, 2008:

hey,
I like it when people talk about AJAX. Just so you don’t need to re-invent things, we’ve added JQuery for javascript and AJAX. Check out this forum link:
http://sourceforge.net/forum/forum.php?thread_id=1887583&forum_id=202506

-Brady

cfapress wrote on Monday, February 11, 2008:

Hey Brady,

Indeed, AJAX is the best way to go. OpenEMR already requires a Javascript compatible browser so including jQuery should be just part of the globals.php. But I expect that will come later on.

In the meantime I’ve been building the Ajax stuff around the jQuery I found in the CVS repository. I have hopes that my changes to the calendar will be worth including in future versions.

Jason