From ed2a73ff2c5a7b8d09026a2bd1ffcaea31e24e42 Mon Sep 17 00:00:00 2001
From: Jared Hancock <jared@osticket.com>
Date: Fri, 13 Feb 2015 14:21:47 -0600
Subject: [PATCH] Add several edits after initial proposal

---
 include/class.attachment.php      |  6 +++---
 include/class.client.php          |  7 +++----
 include/class.collaborator.php    | 14 +++++++++-----
 include/class.cron.php            |  4 ++++
 include/class.lock.php            |  3 ---
 include/class.orm.php             | 14 +++++++++-----
 include/class.ticket.php          | 15 +++++++++------
 include/staff/ticket-view.inc.php |  5 ++---
 js/redactor-osticket.js           |  2 +-
 scp/tickets.php                   |  4 ++--
 10 files changed, 42 insertions(+), 32 deletions(-)

diff --git a/include/class.attachment.php b/include/class.attachment.php
index 2a2fa7dc5..748266003 100644
--- a/include/class.attachment.php
+++ b/include/class.attachment.php
@@ -78,9 +78,9 @@ class Attachment extends VerySimpleModel {
     }
 
     static function lookup($var, $objectId=0) {
-        return is_numeric($var)
-            ? parent::lookup($var)
-            : static::lookupByFileHash($var, $objectId);
+        return (is_string($var))
+            ? static::lookupByFileHash($var, $objectId)
+            : parent::lookup($var);
     }
 }
 
diff --git a/include/class.client.php b/include/class.client.php
index a8b9da332..4932f4c40 100644
--- a/include/class.client.php
+++ b/include/class.client.php
@@ -81,10 +81,9 @@ implements EmailContact, ITicketUser {
                     $user = null;
                 break;
             case 'o': //Ticket owner
-                if (($ticket = Ticket::lookup($matches['tid']))) {
-                    if (($user = $ticket->getOwner())
-                            && $user->getId() != $matches['uid'])
-                        $user = null;
+                if (($user = $ticket->getOwner())
+                        && $user->getId() != $matches['uid']) {
+                    $user = null;
                 }
                 break;
         }
diff --git a/include/class.collaborator.php b/include/class.collaborator.php
index e7c65cec6..35e5ea028 100644
--- a/include/class.collaborator.php
+++ b/include/class.collaborator.php
@@ -23,6 +23,7 @@ implements EmailContact, ITicketUser {
     static $meta = array(
         'table' => THREAD_COLLABORATOR_TABLE,
         'pk' => array('id'),
+        'select_related' => array('user'),
         'joins' => array(
             'thread' => array(
                 'constraint' => array('thread_id' => 'Thread.id'),
@@ -51,14 +52,14 @@ implements EmailContact, ITicketUser {
     }
 
     function getTicketId() {
-        if ($this->thread->ticket)
+        if ($this->thread->object_type == ObjectModel::OBJECT_TYPE_TICKET)
             return $this->thread->object_id;
     }
 
     function getTicket() {
         // TODO: Change to $this->thread->ticket when Ticket goes to ORM
-        $ticket = Ticket::lookup($this->getTicketId());
-        return $ticket;
+        if ($id = $this->getTicketId())
+            return Ticket::lookup($id);
     }
 
     function getUser() {
@@ -72,6 +73,9 @@ implements EmailContact, ITicketUser {
     function getName() {
         return $this->user->getName();
     }
+    function sendAccessLink($ticket) {
+        return $this->user->sendAccessLink($ticket);
+    }
 
     // VariableReplacer interface
     function getVar($what) {
@@ -139,10 +143,10 @@ implements EmailContact, ITicketUser {
         return false;
     }
 
-    static function forTicket($tid, $criteria=array()) {
+    static function forThread($tid, $criteria=array()) {
 
         $collaborators = static::objects()
-            ->filter(array('thread__ticket__ticket_id' => $tid));
+            ->filter(array('thread_id' => $tid));
 
         if (isset($criteria['isactive']))
             $collaborators->filter(array('isactive' => $criteria['isactive']));
diff --git a/include/class.cron.php b/include/class.cron.php
index 7598d7ffe..94e1af314 100644
--- a/include/class.cron.php
+++ b/include/class.cron.php
@@ -28,6 +28,10 @@ class Cron {
     function TicketMonitor() {
         require_once(INCLUDE_DIR.'class.ticket.php');
         Ticket::checkOverdue(); //Make stale tickets overdue
+        // Cleanup any expired locks
+        require_once(INCLUDE_DIR.'class.lock.php');
+        Lock::cleanup();
+
     }
 
     function PurgeLogs() {
diff --git a/include/class.lock.php b/include/class.lock.php
index be408cca3..e4870d465 100644
--- a/include/class.lock.php
+++ b/include/class.lock.php
@@ -113,9 +113,6 @@ class Lock extends VerySimpleModel {
         if (!$staffId or !$lockTime)
             return null;
 
-        // Cleanup any expired locks
-        self::cleanup();
-
         // Create the new lock.
         $lock = parent::create(array(
             'created' => SqlFunction::NOW(),
diff --git a/include/class.orm.php b/include/class.orm.php
index 92e28df8e..0e0568e44 100644
--- a/include/class.orm.php
+++ b/include/class.orm.php
@@ -92,10 +92,13 @@ class ModelMeta implements ArrayAccess {
                 $j['null'] = true;
         }
         // XXX: Make this better (ie. composite keys)
-        $keys = array_keys($j['constraint']);
-        $foreign = $j['constraint'][$keys[0]];
-        $j['fkey'] = explode('.', $foreign);
-        $j['local'] = $keys[0];
+        foreach ($j['constraint'] as $local => $foreign) {
+            list($class, $field) = explode('.', $foreign);
+            if ($local[0] == "'" || $field[0] == "'" || !class_exists($class))
+                continue;
+            $j['fkey'] = array($class, $field);
+            $j['local'] = $local;
+        }
     }
 
     function offsetGet($field) {
@@ -1971,7 +1974,8 @@ class MySqlCompiler extends SqlCompiler {
                     $fields[] = $T;
                 }
             }
-            if (!$queryset->values) {
+            // If no group by has been set yet, use the root model pk
+            if (!$group_by) {
                 foreach ($model::$meta['pk'] as $pk)
                     $group_by[] = $rootAlias .'.'. $pk;
             }
diff --git a/include/class.ticket.php b/include/class.ticket.php
index 8e96ca3eb..b7560b8a1 100644
--- a/include/class.ticket.php
+++ b/include/class.ticket.php
@@ -292,7 +292,7 @@ implements RestrictedAccess, Threadable {
 
     function loadDynamicData() {
         if (!$this->_answers) {
-            foreach (DynamicFormEntry::forTicket($this->getId(), true) as $form) {
+            foreach (DynamicFormEntry::forTicket($this->getThreadId(), true) as $form) {
                 foreach ($form->getAnswers() as $answer) {
                     $tag = mb_strtolower($answer->getField()->get('name'))
                         ?: 'field.' . $answer->getField()->get('id');
@@ -650,12 +650,15 @@ implements RestrictedAccess, Threadable {
         return $this->tlock;
     }
 
-    function releaseLock() {
+    function releaseLock($staffId=false) {
         if (!($lock = $this->getLock()))
-            return;
+            return false;
+
+        if ($staffId && $lock->staff_id != $staffId)
+            return false;
 
         if (!$lock->delete())
-            return;
+            return false;
 
         $sql = 'UPDATE '.TICKET_TABLE.' SET `lock_id` = 0 WHERE `ticket_id` = '
             . db_input($this->getId());
@@ -900,10 +903,10 @@ implements RestrictedAccess, Threadable {
     function getCollaborators($criteria=array()) {
 
         if ($criteria)
-            return Collaborator::forTicket($this->getId(), $criteria);
+            return Collaborator::forThread($this->getThreadId(), $criteria);
 
         if (!isset($this->collaborators))
-            $this->collaborators = Collaborator::forTicket($this->getId());
+            $this->collaborators = Collaborator::forThread($this->getThreadId());
 
         return $this->collaborators;
     }
diff --git a/include/staff/ticket-view.inc.php b/include/staff/ticket-view.inc.php
index 50301d498..0560eaf00 100644
--- a/include/staff/ticket-view.inc.php
+++ b/include/staff/ticket-view.inc.php
@@ -514,7 +514,6 @@ $tcount = $ticket->getThreadEntries($types)->count();
         <input type="hidden" name="id" value="<?php echo $ticket->getId(); ?>">
         <input type="hidden" name="msgId" value="<?php echo $msgId; ?>">
         <input type="hidden" name="a" value="reply">
-        <input type="hidden" name="lockId" value="<?php echo $ticket->getLock()->getId(); ?>">
         <input type="hidden" name="lockCode" value="<?php echo $ticket->getLock()->getCode(); ?>">
         <span class="error"></span>
         <table style="width:100%" border="0" cellspacing="0" cellpadding="3">
@@ -704,7 +703,6 @@ $tcount = $ticket->getThreadEntries($types)->count();
         <input type="hidden" name="id" value="<?php echo $ticket->getId(); ?>">
         <input type="hidden" name="locktime" value="<?php echo $cfg->getLockTime(); ?>">
         <input type="hidden" name="a" value="postnote">
-        <input type="hidden" name="lockId" value="<?php echo $ticket->getLock()->getId(); ?>">
         <input type="hidden" name="lockCode" value="<?php echo $ticket->getLock()->getCode(); ?>">
         <table width="100%" border="0" cellspacing="0" cellpadding="3">
             <?php
@@ -1077,7 +1075,8 @@ $(function() {
 // Hover support for all inline images
 $urls = array();
 foreach (AttachmentFile::objects()->filter(array(
-    'attachments__thread_entry__thread__id' => $ticket->getThreadId()
+    'attachments__thread_entry__thread__id' => $ticket->getThreadId(),
+    'attachments__inline' => true,
 )) as $file) {
     $urls[strtolower($file->getKey())] = array(
         'download_url' => $file->getDownloadUrl(),
diff --git a/js/redactor-osticket.js b/js/redactor-osticket.js
index 559b9a476..39ffaad30 100644
--- a/js/redactor-osticket.js
+++ b/js/redactor-osticket.js
@@ -67,7 +67,7 @@ RedactorPlugins.draft = function() {
         // Slight workaround. Signal the 'keyup' event normally signaled
         // from typing in the <textarea>
         if ($.autoLock
-            && this.$box.closest('form').find('input[name=lockId]').val()
+            && this.$box.closest('form').find('input[name=lockCode]').val()
             && this.code.get()
         ) {
             $.autoLock.handleEvent();
diff --git a/scp/tickets.php b/scp/tickets.php
index 96b4ee51b..0a57e94c5 100644
--- a/scp/tickets.php
+++ b/scp/tickets.php
@@ -101,7 +101,7 @@ if($_POST && !$errors):
                 $response_form->getField('attachments')->reset();
 
                 // Remove staff's locks
-                $ticket->releaseLock();
+                $ticket->releaseLock($thisstaff->getId());
 
                 // Cleanup response draft for this user
                 Draft::deleteForNamespace(
@@ -182,7 +182,7 @@ if($_POST && !$errors):
                          $msg = __('Ticket is NOW assigned to you!');
                      } else {
                          $msg=sprintf(__('Ticket assigned successfully to %s'), $ticket->getAssigned());
-                         $ticket->releaseLock();
+                         $ticket->releaseLock($thisstaff->getId());
                          $ticket=null;
                      }
                  } elseif(!$errors['assign']) {
-- 
GitLab