no $db->exec in controller. all related db operations moved into Ticket model. requireLogin is centralised to BaseController

README.md

@@ -1,3 +1,16 @@
+# Coding Approach
+- Classes
+  - Class names should be in `CapitalCamelCase`
+  - Class functions will be in `camelCase`
+- Functions
+  - Function variables will be in `snake_case`
+- Arrays
+  - Array keys will be in `snake_case`
+- SQL
+  - table names, and columns names will be in `snake_case`
+- Don't repeat yourself (DRY)
+- Each fucntion should have a single purpose
+
 # tp_servicedesk
 
 A { service desk, ticket, knowledge base } web application written in PHP using fat free framework.

app/controllers/BaseController.php

@@ -0,0 +1,58 @@
+<?php
+
+abstract class BaseController
+{
+
+    use RequiresAuth;
+    
+    protected $f3;
+
+    public function __construct()
+    {
+        $this->f3 = \Base::instance();
+    }
+
+
+    // helper function 
+
+    protected function getDB()
+    {
+        return $this->f3->get('DB');
+    }
+
+    /**
+     * Enforce that the user is logged in before proceeding.
+     */
+    protected function requireLogin()
+    {
+        // using trait
+        $this->check_access($this->f3);
+        return;
+
+        // abstract
+        if(!$this->f3->exists('SESSION.user')){
+            $this->f3->set('SESSION.redirect', $this->f3->get('PATH'));
+            $this->f3->reroute('/login');
+        }
+    }
+
+    /**
+     * Set up a main layout template and inject the specified view path
+     * optional $data to pass variables down to template
+     */
+    protected function renderView(string $viewPath, array $data = []):void
+    {
+        foreach($data as $key => $value){
+            $this->f3->set($key, $value);
+        }
+
+        // set {{content}}
+        $this->f3->set('content', $viewPath);
+        
+        // render tempalte
+        echo \Template::instance()->render('../ui/templates/layout.html');
+        
+        // clear SESSION.error
+        $this->f3->clear('SESSION.error');
+    }
+}

app/controllers/TicketController.php

@@ -1,24 +1,22 @@
 <?php
 
-class TicketController implements CRUD {
+class TicketController extends BaseController implements CRUD {
 
     use RequiresAuth;
 
     // list all tickts
     public function index($f3){
-        $this->check_access($f3);
 
-        $db = $f3->get('DB');
+        $this->requireLogin();
 
         // retrieve tickets
-        $tickets = $db->exec('SELECT * FROM tickets ORDER BY created_at DESC');
+        $ticket_mapper = new Ticket($this->getDB());
-
+        $tickets = $ticket_mapper->findAll();
-        // pass data to template
-        $f3->set('tickets', $tickets);
 
         // render
-        $f3->set('content', '../ui/views/ticket/index.html');
+        $this->renderView('../ui/views/ticket/index.html', 
-        echo \Template::instance()->render('../ui/templates/layout.html');
+            ['tickets' => $tickets]
+        );
 
         $f3->clear('SESSION.error');
     }
@@ -26,296 +24,129 @@ class TicketController implements CRUD {
     // view a single ticket
     // TODO_PROJECTS: show a link back to the related project
     public function view($f3){
-        $this->check_access($f3);
+        $this->requireLogin();
 
         $ticket_id = $f3->get('PARAMS.id');
-        $db = $f3->get('DB');
+        $ticket_mapper = new Ticket($this->getDB());
-
+        $ticket = $ticket_mapper->findById($ticket_id);
-        $this->get_ticket($f3, $db, $ticket_id);
-        $this->get_child_tickets($f3, $db, $ticket_id);
-        $this->get_parent_tickets($f3, $db, $ticket_id);
-        $this->get_custom_fields($f3, $db, $ticket_id);
 
         // render
-        // $f3->set('js', 'ticket_view.js');
+        $this->renderView('../ui/views/ticket/view.html', [
-        $f3->set('content', '../ui/views/ticket/view.html');
+            'ticket' => $ticket,
-        echo \Template::instance()->render('../ui/templates/layout.html');
+            'attachments' => $ticket->attachments(),
+            'comments' => $ticket->comments(),
+            'parent_tickets' => $ticket->getParentTickets(),
+            'child_tickets' => $ticket->getChildTickets(),
+            'ticket_meta' => $ticket->getMetaAssoc()
+        ]);
 
     }
 
     // show create form
     // TODO_PROJECTS: dropdown to associate ticket with project
     public function createForm($f3){
-        $this->check_access($f3);
+
-        $f3->set('content', '../ui/views/ticket/create.html');
+        $this->requireLogin();
-        echo \Template::instance()->render('../ui/templates/layout.html');
+        $this->renderView('../ui/views/ticket/create.html');
     }
 
     // handle POST
     // including custom forms
     public function create($f3){
-        $this->check_access($f3);
+        
+        $this->requireLogin();
 
-        $title = $f3->get('POST.title');
+        $data = [
-        $description = $f3->get('POST.description');
+            'title' => $this->f3->get('POST.title'),
-        $priority = $f3->get('POST.priority');   // eg - low, medium, high
+            'description' => $this->f3->get('POST.description'),
-        $status = $f3->get('POST.status');   // eg - new, in_progress
+            'priority' => $this->f3->get('POST.priority'),
-        $created_by = $f3->get('SESSION.user.id'); // current logged in user
+            'status' => $this->f3->get('POST.status'),
+            'created_by' => $this->f3->get('SESSION.user.id')
+        ];
 
-        $db = $f3->get('DB');
+        $ticket_mapper = new Ticket($this->getDB());
+        $new_ticket_id = $ticket_mapper->createTicket($data);
 
-        $db->exec(
+        // custom field
-            'INSERT 
+        $meta_keys = $this->f3->get('POST.meta_key');
-                INTO tickets (title, description, priority, status, created_by, created_at, updated_at)
+        $meta_values = $this->f3->get('POST.meta_value');
-                VALUES (?,?,?,?,?,NOW(), NOW())',
+        $meta_assoc = $ticket_mapper->assocMetaFromKeyValue($meta_keys, $meta_values);
-                [$title, $description, $priority, $status, $created_by]
+        $ticket_mapper->setCustomFields($meta_assoc);
-        );
 
-        $ticket_id = $db->lastInsertId();
+        $this->f3->reroute('/ticket/' . $new_ticket_id);
-
-        // custom fields
-        $meta_keys = $f3->get('POST.meta_key'); // eg ['department', 'category']
-        $meta_values = $f3->get('POST.meta_value');
-
-        if(is_array($meta_keys) && is_array($meta_values)){
-            foreach($meta_keys as $index => $key){
-                $val = $meta_values[$index] ?? '';
-                if(!empty($key) && $val !== ''){
-                    $db->exec(
-                        'INSERT INTO ticket_meta (ticket_id, meta_key, meta_value)
-                        VALUES (?,?,?)',
-                        [$ticket_id, $key, $val]
-                    );
-                }
-            }
-        }
-
-        // reroute to ticket
-        $f3->reroute('/ticket/'.$ticket_id);
     }
 
     // show edit form
     // including custom forms
     // TODO_PROJECTS: allow reasssigning or removing a project association
     public function editForm($f3){
-        $this->check_access($f3);
+        $this->requireLogin();
 
         $ticket_id = $f3->get('PARAMS.id');
-        $db = $f3->get('DB');
+        $ticket_mapper = new Ticket($this->getDB());
-        
+        $ticket = $ticket_mapper->findById($ticket_id);
 
-        $ticket = $this->get_ticket_check_edit_permission($f3);
+        if(!$ticket){
-        $f3->set('ticket', $ticket);
+            $this->f3->set('SESSION.error', 'Ticket not found.');
+            $this->f3->reroute('/tickets');
+        }
 
-        // fetch custom fields
+        $this->renderView('../ui/views/ticket/edit.html',[
-        $meta = $db->exec(
+            'ticket' => $ticket,
-            'SELECT id, meta_key, meta_value
+            'ticket_meta' => $ticket->getMeta()
-            FROM ticket_meta
+            ]
-            WHERE ticket_id = ?',
-            [$ticket_id]
         );
-        $f3->set('ticket_meta', $meta);
+        return;
-
-        $f3->set('ticket', $ticket);
-        $f3->set('content', '../ui/views/ticket/edit.html');
-        echo \Template::instance()->render('../ui/templates/layout.html');
     }
 
     // process edit POST TODO: if assigned or admin
     public function update($f3){
-        $this->check_access($f3);
-        $ticket = $this->get_ticket_check_edit_permission($f3);
-        $ticket_id = $ticket['id'];
-        $db = $f3->get('DB');
 
-        // get updated fields from post
+        $this->requireLogin();
-        $title = $f3->get('POST.title');
+
-        $description = $f3->get('POST.description');
+        $ticket_id = $this->f3->get('PARAMS.id');
-        $priority = $f3->get('POST.priority');   // eg - low, medium, high
+        $ticket_mapper = new Ticket($this->getDB());
-        $status = $f3->get('POST.status');   // eg - new, in_progress
+        $ticket = $ticket_mapper->findById($ticket_id);
-        $updated_by = $f3->get('SESSION.user.id'); // current logged in user
+
+        if(!$ticket){
+            $this->f3->set('SESSION.error', 'Ticket not found.');
+            $this->f3->reroute('/tickets');
+        }
+
+        $data = [
+            'title' => $this->f3->get('POST.title'),
+            'description' => $this->f3->get('POST.description'),
+            'priority' => $this->f3->get('POST.priority'),
+            'status' => $this->f3->get('POST.status'),
+            'updated_by' => $this->f3->get('SESSION.user.id') 
+        ];
+        $ticket->updateTicket($data);
         
-        // TODO: if you want to update assignment, should be added here.
+        // deal with meta data / custom fields
-
+        $meta_keys = $this->f3->get('POST.meta_key');
-        $db->exec(
+        $meta_values = $this->f3->get('POST.meta_value');
-            'UPDATE tickets
+        $meta_assoc = $ticket->assocMetaFromKeyValue($meta_keys, $meta_values);
-            SET title=?, description=?, priority=?, status=?, updated_by=?, updated_at=?
+        $ticket->setCustomFields($meta_assoc);
-            WHERE id=?', 
-            [$title, $description, $priority, $status, $updated_by, 'NOW()', $ticket_id]
-        );
-
-        // handle custom fields
-        // 1. update existing custom fields
-        $meta_ids = $f3->get('POST.meta_id');
-        $meta_keys = $f3->get('POST.meta_key');
-        $meta_values = $f3->get('POST.meta_value');
-
-        if(is_array($meta_ids) && is_array($meta_keys) && is_array($meta_values)){
-            foreach($meta_ids as $idx => $m_id){
-                $m_key = $meta_keys[$idx] ?? '';
-                $m_val = $meta_values[$idx] ?? '';
-
-                if(!empty($m_key) && $m_val !== ''){
-                    $db->exec(
-                        'UPDATE ticket_meta
-                        SET meta_key = ?, meta_value=?
-                        WHERE id = ? AND ticket_id = ?',
-                        [$m_key, $m_val, $m_id, $ticket_id]
-                    );
-                } else {
-                    // delete if the user cleared it
-                    $db->exec(
-                        'DELETE FROM ticket_meta
-                        WHERE id =? AND ticket_id = ?',
-                        [$m_id, $ticket_id]
-                    );
-                }
-            }
-        }
-
-        // 2. insert new fields
-        $new_keys = $f3->get('POST.new_meta_key');
-        $new_values = $f3->get('POST.new_meta_value');
-        if(is_array($new_keys) && is_array($new_values)){
-            foreach($new_keys as $idx => $n_key){
-                $n_val = $new_values[$idx] ?? '';
-                if(!empty($n_key) && $n_val !== ''){
-                    $db->exec(
-                        'INSERT INTO ticket_meta (ticket_id, meta_key, meta_value)
-                        VALUES (?,?,?)',
-                        [$ticket_id, $n_key, $n_val]
-                    );
-                }
-            }
-        }
 
         $f3->reroute('/ticket/' . $ticket_id);
     }
 
     // subtask
     public function addSubtask($f3){
-        $this->check_access($f3);
+        $this->requireLogin();
 
         $parent_id = (int) $f3->get('PARAMS.id');
         $child_id = (int) $f3->get('POST.child_ticket_id');
-        $db = $f3->get('DB');
 
-        // TODO check that both parent and child tickets exist
+        $ticket_mapper = new Ticket($this->getDB());
-        // ensure you don't link a ticket to itself, etc.
+        $ticket = $ticket_mapper->findById($parent_id);
 
-        // insert or ignore
+        if(!$ticket){
-        $db->exec(
+            $this->f3->set('SESSION.error', 'Parent Ticket not found');
-            'INSERT IGNORE INTO ticket_relations (parent_ticket_id, child_ticket_id)
+            $this->f3->reroute('/tickets');
-            VALUES (?, ?)',
-            [$parent_id, $child_id]
-        );
-        $f3->reroute('/ticket/' . $parent_id);
-    }
-
-    
-
-    protected function get_ticket_check_edit_permission($f3){
-
-
-        $db = $f3->get('DB');
-        
-        $ticket_id = $f3->get('PARAMS.id');
-        $result = $db->exec('SELECT * FROM tickets WHERE id = ? LIMIT 1', [$ticket_id]);
-
-        if(!$result){
-            $f3->set('SESSION.error', 'Ticket not found.');
-            $f3->reroute('/tickets');
         }
-
+        $ticket->addChildTicket($child_id);
-        $ticket = $result[0];
+        $this->f3->reroute('/ticket/' . $parent_id);
-
-        // TODO: refine
-        $current_user = $f3->get('SESSION.user');
-        $is_admin = (isset($current_user['role_name']) && $current_user['role_name'] == 'admin');
-        $is_assigned = ($ticket['assigned_to'] == $current_user['id']);
-
-        if(!$is_admin && !$is_assigned){ // should this be ||
-            // if not assigned and not admin, disallow edit
-            $f3->set('SESSION.error', 'You do not have permission to edit this ticket.');
-            $f3->reroute('/tickets');
-        }
-
-        return $ticket;
-
-    }
-
-    protected function get_ticket($f3, $db, $ticket_id){
-
-        // new
-        $db = $f3->get('DB');
-        $ticket_id = $f3->get('PARAMS.id');
-
-        $ticketModel = new Ticket($db);
-        $ticket = $ticketModel->findById($ticket_id);
-
-        if(!$ticket->dry()){
-            $f3->set('ticket', $ticket);
-            $f3->set('attachments', $ticket->attachments());
-            $f3->set('comments', $ticket->comments());
-        } else {
-            $f3->error(404, "Ticket not found!");
-        }
-        return;
-
-        // old:
-
-        $result = $db->exec(
-            'SELECT t.*, u.username as created_by_name
-            FROM tickets t
-            LEFT JOIN users u ON t.created_by = u.id
-            WHERE t.id =? LIMIT 1',
-            [$ticket_id]
-        );
-
-        if(!$result){
-            // no record
-            $f3->set('SESSION.error', 'Ticket not found.');
-            $f3->reroute('/tickets');
-        }
-
-        $ticket = $result[0];
-        $f3->set('ticket', $ticket);
-    }
-
-    protected function get_child_tickets($f3, $db, $ticket_id){
-            $child_tickets = $db->exec(
-                'SELECT c.* 
-                FROM ticket_relations r
-                INNER JOIN tickets c ON r.child_ticket_id = c.id
-                WHERE r.parent_ticket_id = ?',
-                [$ticket_id]);
-            $f3->set('child_tickets', $child_tickets);
-    }
-
-    protected function get_parent_tickets($f3, $db, $ticket_id){
-        $parent_tickets = $db->exec(
-            'SELECT p.*
-            FROM ticket_relations r
-            INNER JOIN tickets p ON r.parent_ticket_id = p.id
-            WHERE r.child_ticket_id = ?',
-            [$ticket_id]
-        );
-        $f3->set('parent_tickets', $parent_tickets);
-    }
-
-    protected function get_custom_fields($f3, $db, $ticket_id){
-        $meta = $db->exec(
-            'SELECT meta_key, meta_value
-            FROM ticket_meta
-            WHERE ticket_id = ?',
-            [$ticket_id]
-        );
-        // convert meta rows into assoc array
-        $meta_array = [];
-        foreach($meta as $m){
-            $meta_array[$m['meta_key']] = $m['meta_value'];
-        }
-        $f3->set('ticket_meta', $meta_array);
     }
 
 }

app/models/Ticket.php

@@ -5,9 +5,47 @@ class Ticket extends \DB\SQL\Mapper {
         parent::__construct($db, 'tickets');
     }
 
-    public function findById($id){
+    /**
+     * Return all tickets in descending order of creation
+     */
+    public function findAll(): array
+    {
+        return $this->db->exec(
+            'SELECT * FROM tickets ORDER BY created_at DESC'
+        );
+    }
+
+    public function findById($id): ?Ticket
+    {
         $this->load(['id = ?', $id]);
-        return $this;
+        return $this->dry() ? null : $this;
+    }
+
+    public function createTicket(array $data): int
+    {
+        $this->reset();
+
+        $this->title = $data['title'] ?? '';
+        $this->description = $data['description'] ?? '';
+        $this->priority = $data['priority'] ?? 'Low';
+        $this->status = $data['status'] ?? 'New';
+        $this->created_by = $data['created_by'] ?? null;
+        $this->created_at = date('Y-m-d H:i:s');
+        $this->updated_at = date('Y-m-d H:i:s');
+
+        $this->save();
+        return (int)$this->id;
+    }
+
+    public function updateTicket(array $data): void
+    {
+        if(isset($data['title'])){ $this->title = $data['title']; }
+        if(isset($data['description'])) { $this->descrtipion = $data['description']; }
+        if(isset($data['priority'])) { $this->priority = $data['priority']; }
+        if(isset($data['status'])) { $this->status = $data['status']; } 
+        if(isset($data['updated_by'])) { $this->status = $data['updated_by']; }
+        $this->updated_at = date('Y-m-d H:i:s');
+        $this->save();
     }
 
     public function attachments(){
@@ -19,4 +57,102 @@ class Ticket extends \DB\SQL\Mapper {
         $comment = new Comment($this->db);
         return $comment->findWithUserByTicketId($this->id);
     }
+
+    public function getParentTickets()
+    {
+        return $this->db->exec(
+            'SELECT p.*
+            FROM ticket_relations r
+            INNER JOIN tickets p ON r.parent_ticket_id = p.id
+            WHERE r.child_ticket_id = ?',
+            [$this->id]
+        );
+    }
+
+    public function getChildTickets()
+    {
+        return $this->db->exec(
+            'SELECT c.*
+            FROM ticket_relations r
+            INNER JOIN tickets c ON r.child_ticket_id = c.id
+            WHERE r.parent_ticket_id = ?',
+            [$this->id]
+        );
+    }
+
+    public function addChildTicket(int $childId)
+    {
+        $this->db->exec(
+            'INSERT IGNORE INTO ticket_relations (parent_ticket_id, child_ticket_id)
+            VALUES (?, ?)',
+            [$this->id, $childId]
+        );
+    }
+
+    // meta data
+    public function getMeta()
+    {
+        return $this->db->exec(
+            'SELECT id, meta_key, meta_value
+            FROM ticket_meta
+            WHERE ticket_id = ?',
+            [$this->id]
+        );
+    }
+
+    public function getMetaAssoc()
+    {
+        $rows = $this->getMeta();
+        $assoc = [];
+        foreach($rows as $row){
+            $assoc[$row['meta_key']] = $row['meta_value'];
+        }
+        return $assoc;
+    }
+
+    public function assocExistingMeta($meta_ids, $meta_keys, $meta_values){
+        if(is_array($meta_ids) && is_array($meta_keys) && is_array($meta_values)){
+            $field_assoc = [];
+            foreach($meta_ids as $i => $m_id){
+                $key = $meta_keys[$i] ?? '';
+                $value = $meta_values[$i] ?? '';
+                if(!empty($key) && $value !== ''){
+                    $field_assoc[$key] = $value;
+                }
+            }
+            return $field_assoc;
+        }
+        return [];
+    }
+
+    public function assocMetaFromKeyValue($meta_keys, $meta_values)
+    {
+        if(is_array($meta_keys) && is_array($meta_values)){
+            $field_assoc = [];
+            foreach($meta_keys as $i => $key){
+                $val = $meta_values[$i] ?? '';
+                if(!empty($key) && $val != ''){
+                    $field_assoc[$key] = $val;
+                }
+            }
+            return $field_assoc;
+        }
+        return [];
+    }
+
+    public function setCustomFields(array $fields)
+    {
+        $this->db->exec(
+            'DELETE FROM ticket_meta WHERE ticket_id = ?', [$this->id]
+        );
+
+        foreach($fields as $key => $value){
+            $this->db->exec(
+                'INSERT INTO ticket_meta (ticket_id, meta_key, meta_value)
+                VALUES (?, ?, ?)',
+                [$this->id, $key, $value]
+            );
+        }
+    }
+    
 }

ui/session/error.html

@@ -0,0 +1,5 @@
+<check if="{{isset(@SESSION.error)}}">
+    <div class="notification is-warning">
+        {{ @SESSION.error }}
+    </div>
+</check>

ui/views/kb/index.html

@@ -1,12 +1,6 @@
 <h1 class="title">Knowledge Base</h1>
-
+<include href="/ui/session/error.html"></include>
-<check if="{{isset(@SESSION.error)}}">
+<p><a class="button" href="/kb/create">create kb article</a></p>
-    <div class="notification is-warning">
-        {{ @SESSION.error }}
-    </div>
-</check>
-
-<p><a href="/kb/create">create new kb article</a></p>
 <hr>
 
 <div class="block">

ui/views/project/index.html

@@ -1,6 +1,8 @@
-<h3 class="title">Projects</h3>
+<h1 class="title">Projects</h1>
-<a href="/project/create">create new project</a>
+<include href="/ui/session/error.html"></include>
+<p><a class="button" href="/project/create">create project</a></p>
 <hr>
+
 <table class="table is-fullwidth is-hoverable is-bordered">
     <thead>
         <tr class="has-background-primary">

ui/views/tag/index.html

@@ -1,12 +1,6 @@
 <h1 class="title">Tags</h1>
-
+<include href="/ui/session/error.html"></include>
-<check if="{{isset(@SESSION.error)}}">
+<p><a class="button" href="/tag/create">create tag</a></p>
-    <div class="notification is-warning">
-        {{ @SESSION.error }}
-    </div>
-</check>
-
-<p><a href="/tag/create">create new tag</a></p>
 <hr>
 
 <check if="{{ count(@tags) > 1 }}">

ui/views/ticket/edit.html

@@ -35,12 +35,12 @@
             <div class="field is-grouped is-grouped-right">
                 <div class="control">
                     <label class="label">Key:</label>
-                    <input class="input" type="text" name="new_meta_key[]"
+                    <input class="input" type="text" name="meta_key[]"
                         placeholder="eg. Department">
                 </div>
                 <div class="control">
                     <label class="label">Value:</label>
-                    <input class="input" type="text" name="new_meta_value[]"
+                    <input class="input" type="text" name="meta_value[]"
                         placeholder="eg. Finance">
                 </div>
             </div>

ui/views/ticket/index.html

@@ -1,12 +1,6 @@
-<h1 class="title">View Tickets</h1>
+<h1 class="title">Tickets</h1>
-
+<include href="/ui/session/error.html">
-<check if="{{isset(@SESSION.error)}}">
+<p><a class="button" href="/ticket/create">create ticket</a></p>
-    <div class="notification is-warning">
-        {{ @SESSION.error }}
-    </div>
-</check>
-
-<p><a href="/ticket/create">create ticket</a></p>
 <hr>
 
 <table class="table is-fullwidth is-bordered">