Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
131 changes: 131 additions & 0 deletions admin/ticket/tickets.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,131 @@
<!DOCTYPE html>
<html lang="en">
<head>
<meta charset="UTF-8" />
<title>Admin | Support Tickets</title>

<!-- Role check -->
<script src="/style/js/roleCheck.js"></script>
<script>
checkRoleAccess(['superAdmin', 'maintenanceAdmin']);
</script>

<script src="https://ajax.googleapis.com/ajax/libs/jquery/1.11.2/jquery.min.js"></script>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

security-critical critical

The version of jQuery being used (1.11.2) is very old and has known security vulnerabilities, including Cross-Site Scripting (XSS). It is critical to update to a modern, secure version of jQuery (e.g., 3.x series) to protect the application and its users.

Suggested change
<script src="https://ajax.googleapis.com/ajax/libs/jquery/1.11.2/jquery.min.js"></script>
<script src="https://ajax.googleapis.com/ajax/libs/jquery/3.7.1/jquery.min.js"></script>

<script src="../../style/js/config.js"></script>
<link rel="stylesheet" href="../../style/css/styles.css" />

<style>
.drawer {
position: fixed;
top: 0;
right: -40%;
width: 40%;
height: 100%;
background: #fff;
box-shadow: -2px 0 8px rgba(0, 0, 0, 0.3);
transition: right 0.3s ease;
z-index: 999;
padding: 20px;
overflow-y: auto;
}

.drawer.open {
right: 0;
}

.messages {
border: 1px solid #ddd;
padding: 10px;
max-height: 300px;
overflow-y: auto;
margin-bottom: 10px;
}

.message {
margin-bottom: 8px;
}

.message.admin {
text-align: right;
color: #0066cc;
}

.message.user {
text-align: left;
color: #333;
}

.message span {
font-size: 11px;
color: #777;
}

textarea {
width: 100%;
height: 70px;
}
</style>
</head>

<body>
<h2>Support Tickets</h2>

<!-- Filters -->
<div style="margin-bottom: 10px">
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Using inline style attributes mixes presentation with structure, making the code harder to maintain and violating the principle of separation of concerns. It's better to use external stylesheets and classes for styling.

<select id="statusFilter">
<option value="">All Status</option>
<option value="open">Open</option>
<option value="in progress">In Progress</option>
<option value="resolved">Resolved</option>
<option value="closed">Closed</option>
</select>

<select id="serviceFilter">
<option value="">All Services</option>
<option value="wifi">WiFi</option>
<option value="room">Room</option>
<option value="maintenance">Maintenance</option>
</select>

<button onclick="fetchTickets()">Apply</button>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Using onclick attributes directly in HTML mixes markup with application logic, which can make the code harder to maintain and debug. It's a better practice to attach event listeners from your JavaScript file using addEventListener. This improves separation of concerns.

For example, you could give this button an ID and attach the listener in tickets.js:

HTML:

<button id="apply-filters-btn">Apply</button>

JavaScript (tickets.js):

document.getElementById('apply-filters-btn').addEventListener('click', fetchTickets);

This should be applied to all buttons with onclick handlers in this file.

</div>

<!-- Tickets Table -->
<table border="1" width="100%" id="ticketTable">
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The border and width attributes are presentational and are considered deprecated in modern HTML. All styling should be handled via CSS to ensure a clean separation of structure and presentation. While the PR title says to ignore CSS, using semantic HTML is important for maintainability.

Suggested change
<table border="1" width="100%" id="ticketTable">
<table width="100%" id="ticketTable">

<thead>
<tr>
<th>Ticket</th>
<th>Issued By</th>
<th>Service</th>
<th>Status</th>
<th>Created</th>
<th>Action</th>
</tr>
</thead>
<tbody></tbody>
</table>

<!-- Side Drawer -->
<div class="drawer" id="ticketDrawer">
<h3 id="ticketTitle"></h3>
<div id="ticketMeta"></div>

<hr />

<div class="messages" id="messageContainer"></div>

<textarea
id="adminMessage"
placeholder="Type your reply..."
></textarea>

<div style="margin-top: 10px; text-align: right">
<button onclick="sendReply()">Send</button>
<button onclick="closeTicket()">Close Ticket</button>
<button onclick="closeDrawer()">Cancel</button>
</div>
</div>

<script src="tickets.js"></script>
</body>
</html>
254 changes: 254 additions & 0 deletions admin/ticket/tickets.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,254 @@
let ticketListInterval = null;
let currentTicketId = null;
let refreshInterval = null;

document.addEventListener('DOMContentLoaded', () => {
fetchTickets();
startTicketListRefresh();
});

/* =====================================================
UNREAD TRACKING (LAST MESSAGE BASED)
===================================================== */

function getLastSeen(ticketId) {
return sessionStorage.getItem(`ticket_last_msg_seen_${ticketId}`);
}

function setLastSeen(ticketId, time) {
sessionStorage.setItem(`ticket_last_msg_seen_${ticketId}`, time);
}

/* =====================================================
FETCH TICKETS (LIST VIEW)
===================================================== */

async function fetchTickets() {
const status = document.getElementById('statusFilter').value;
const service = document.getElementById('serviceFilter').value;

const params = new URLSearchParams();
if (status) params.append('status', status);
if (service) params.append('service', service);

const res = await fetch(
`${CONFIG.basePath}/tickets?${params.toString()}`,
{
headers: {
Authorization: `Bearer ${sessionStorage.getItem('token')}`
}
}
);

const result = await res.json();
renderTicketTable(result.data || []);
Comment on lines +34 to +44
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

The fetch call lacks error handling. If the network request fails or the server returns an error status (e.g., 500), the promise will reject and cause an unhandled exception, which could crash the application. It's important to wrap asynchronous operations in try...catch blocks and check the response status with res.ok.

This should be applied to all fetch calls in this file (loadTicketDetails, sendReply, closeTicket).

  try {
    const res = await fetch(
      `${CONFIG.basePath}/tickets?${params.toString()}`,
      {
        headers: {
          Authorization: `Bearer ${sessionStorage.getItem('token')}`
        }
      }
    );

    if (!res.ok) {
      console.error('Failed to fetch tickets:', res.status, res.statusText);
      // Consider showing an error message to the user
      return;
    }

    const result = await res.json();
    renderTicketTable(result.data || []);
  } catch (error) {
    console.error('Error fetching tickets:', error);
    // Consider showing an error message to the user
  }

}

/* =====================================================
RENDER TICKET TABLE (UNREAD INDICATOR)
===================================================== */

function renderTicketTable(tickets) {
const tbody = document.querySelector('#ticketTable tbody');
tbody.innerHTML = '';

tickets.forEach(t => {
const lastSeen = getLastSeen(t.id);
const lastMsg = t.last_message_at;

const unread =
lastMsg && (!lastSeen || new Date(lastMsg) > new Date(lastSeen));

const dot = unread
? '<span style="color:red;font-weight:bold;">●</span> '
: '';

const tr = document.createElement('tr');
tr.innerHTML = `
<td>${dot}${t.id}</td>
<td>${t.issued_by}</td>
<td>${t.service}</td>
<td>${t.status}</td>
<td>${new Date(t.createdAt).toLocaleString()}</td>
<td>
<button onclick="openTicket('${t.id}')">View</button>
</td>
`;
Comment on lines +67 to +76
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

security-high high

Directly setting innerHTML with data received from an API (t.id, t.issued_by, etc.) creates a Cross-Site Scripting (XSS) vulnerability. If any of these fields contain malicious user-provided data, it could execute scripts in the browser. It is much safer to create DOM elements programmatically and set their textContent property.

Example of a safer approach:

const tr = document.createElement('tr');

const createCell = (text) => {
    const td = document.createElement('td');
    td.textContent = text;
    return td;
};

const idCell = document.createElement('td');
idCell.innerHTML = dot; // 'dot' is safe as it's generated in the code
idCell.appendChild(document.createTextNode(t.id));

const actionCell = document.createElement('td');
const viewButton = document.createElement('button');
viewButton.textContent = 'View';
viewButton.addEventListener('click', () => openTicket(t.id));
actionCell.appendChild(viewButton);

tr.append(
    idCell,
    createCell(t.issued_by),
    createCell(t.service),
    createCell(t.status),
    createCell(new Date(t.createdAt).toLocaleString()),
    actionCell
);
tbody.appendChild(tr);

tbody.appendChild(tr);
});
}

/* =====================================================
OPEN TICKET (DRAWER)
===================================================== */

async function openTicket(ticketId) {
currentTicketId = ticketId;
stopTicketListRefresh(); // 👈 pause list polling
await loadTicketDetails();
openDrawer();
startAutoRefresh();
}

/* =====================================================
LOAD TICKET DETAILS (MESSAGES)
===================================================== */

async function loadTicketDetails() {
if (!currentTicketId) return;

const res = await fetch(
`${CONFIG.basePath}/tickets/${currentTicketId}`,
{
headers: {
Authorization: `Bearer ${sessionStorage.getItem('token')}`
}
}
);

const result = await res.json();
const ticket = result.data;

populateDrawer(ticket);

/*
🔑 CRITICAL FIX:
Mark ticket as read using LAST MESSAGE TIME,
NOT current time
*/
if (ticket.messages && ticket.messages.length > 0) {
const lastMsg =
ticket.messages[ticket.messages.length - 1];
setLastSeen(currentTicketId, lastMsg.createdAt);
}
}

/* =====================================================
POPULATE DRAWER UI
===================================================== */

function populateDrawer(ticket) {
document.getElementById(
'ticketTitle'
).innerText = `Ticket #${ticket.id} (${ticket.status})`;

document.getElementById('ticketMeta').innerHTML = `
<p><b>Service:</b> ${ticket.service}</p>
<p><b>Issued By:</b> ${ticket.issued_by}</p>
<p><b>Description:</b> ${ticket.description}</p>
`;
Comment on lines +135 to +139
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

security-high high

Using innerHTML to set content from API data (ticket.service, ticket.issued_by, ticket.description) is unsafe and can lead to Cross-Site Scripting (XSS) vulnerabilities. Malicious scripts in these fields would be executed. Use textContent for any data that should be treated as plain text.

  const meta = document.getElementById('ticketMeta');
  meta.innerHTML = ''; // Clear previous content

  const createMetaLine = (label, value) => {
    const p = document.createElement('p');
    const b = document.createElement('b');
    b.textContent = `${label}: `;
    p.appendChild(b);
    p.appendChild(document.createTextNode(value));
    return p;
  };

  meta.appendChild(createMetaLine('Service', ticket.service));
  meta.appendChild(createMetaLine('Issued By', ticket.issued_by));
  meta.appendChild(createMetaLine('Description', ticket.description));


const container = document.getElementById('messageContainer');

const wasAtBottom =
container.scrollTop + container.clientHeight >=
container.scrollHeight - 10;

container.innerHTML = '';

ticket.messages.forEach(msg => {
const div = document.createElement('div');
div.className = `message ${msg.sender_type}`;
div.innerHTML = `
<p>${msg.message}</p>
<span>${msg.sender_type} • ${new Date(
msg.createdAt
).toLocaleString()}</span>
`;
Comment on lines +152 to +157
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

security-critical critical

Setting innerHTML with user-generated message content (msg.message) is a critical security vulnerability. This allows for Cross-Site Scripting (XSS) attacks, where a malicious user can inject scripts that will be executed in the admin's browser. Always use textContent to render user-provided content as plain text.

      <p></p>
      <span>${msg.sender_type}  ${new Date(
      msg.createdAt
    ).toLocaleString()}</span>
    `;
    div.querySelector('p').textContent = msg.message;

container.appendChild(div);
});

if (wasAtBottom) {
container.scrollTop = container.scrollHeight;
}
}

/* =====================================================
SEND ADMIN REPLY
===================================================== */

async function sendReply() {
const message = document.getElementById('adminMessage').value.trim();
if (!message || !currentTicketId) return;

await fetch(
`${CONFIG.basePath}/tickets/${currentTicketId}/messages`,
{
method: 'POST',
headers: {
'Content-Type': 'application/json',
Authorization: `Bearer ${sessionStorage.getItem('token')}`
},
body: JSON.stringify({ message })
}
);

document.getElementById('adminMessage').value = '';
await loadTicketDetails();
}

/* =====================================================
CLOSE TICKET
===================================================== */

async function closeTicket() {
if (!currentTicketId) return;

await fetch(
`${CONFIG.basePath}/tickets/${currentTicketId}/status`,
{
method: 'PATCH',
headers: {
'Content-Type': 'application/json',
Authorization: `Bearer ${sessionStorage.getItem('token')}`
},
body: JSON.stringify({ status: 'closed' })
}
);

closeDrawer();
fetchTickets();
}

/* =====================================================
DRAWER CONTROL
===================================================== */

function openDrawer() {
document.getElementById('ticketDrawer').classList.add('open');
}

function closeDrawer() {
document.getElementById('ticketDrawer').classList.remove('open');
stopAutoRefresh();
startTicketListRefresh(); // 👈 resume list polling
currentTicketId = null;
}

/* =====================================================
AUTO REFRESH (POLLING)
===================================================== */

function startAutoRefresh() {
stopAutoRefresh();
refreshInterval = setInterval(loadTicketDetails, 60000);
}

function stopAutoRefresh() {
if (refreshInterval) {
clearInterval(refreshInterval);
refreshInterval = null;
}
}

function startTicketListRefresh() {
stopTicketListRefresh();
ticketListInterval = setInterval(fetchTickets, 5000);
}

function stopTicketListRefresh() {
if (ticketListInterval) {
clearInterval(ticketListInterval);
ticketListInterval = null;
}
}
Comment on lines +232 to +254
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

The application uses polling via setInterval to check for updates. This is inefficient, causing unnecessary network traffic and server load, and introduces delays in UI updates. Since socket.io dependencies have been added to package.json, it is strongly recommended to refactor this implementation to use WebSockets for real-time, event-driven updates. This will be more performant and provide a better user experience.

Loading