Skip to content

Issue #247530 Fix : Resolved the issue while you try to edit notification it throws save error #176

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 3 commits into
base: release-3.1.0
Choose a base branch
from
Open

Issue #247530 Fix : Resolved the issue while you try to edit notification it throws save error #176

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
5fbc89f
Issue #247530 Fix : Resolved the issue of notification save
imrohitkodam Sep 25, 2025
47a58e1
Issue #247530 Fix : Resovled PR Comments
imrohitkodam Sep 25, 2025
7158eac
Issue #247530 Fix : Resovled PR Comments
imrohitkodam Sep 25, 2025
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
2 changes: 2 additions & 0 deletions src/com_tjnotifications/admin/language/en-GB.com_tjnotifications.ini
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,8 @@ COM_TJNOTIFICATIONS_FIELD_WEB_STATUS_LABEL="Web Status"
COM_TJNOTIFICATIONS_FIELD_WEB_SUBJECT_LABEL="Subject"
COM_TJNOTIFICATIONS_FIELD_WEB_BODY_LABEL="Web Body"
COM_TJNOTIFICATIONS_FIELD_CREATED_SUCCESSFULLY="Notification successfully saved."
COM_TJNOTIFICATIONS_ERROR_TEMPLATE_ID_MISSING="Template ID is missing. Cannot save notification."
COM_TJNOTIFICATIONS_ERROR_SAVE_BACKEND_CONFIG="Failed to save %s backend configuration: %s"
; Status description
COM_TJNOTIFICATIONS_FIELD_EMAIL_STATUS_DESC="Turn on this after filling the body to send an Email notification"
COM_TJNOTIFICATIONS_FIELD_SMS_STATUS_DESC="Turn on this after filling the body to send an SMS notification"
Expand Down
80 changes: 67 additions & 13 deletions src/com_tjnotifications/admin/models/notification.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -351,14 +351,24 @@ public function save($data)
{
$db = Factory::getDbo();
// IMPORTANT to set new id in state, it is fetched in controller later
// Get current Template id
$templateId = $db->insertid();
// Get current Template id - Fix for existing records
if (!empty($data['id']))
{
$templateId = $data['id'];
$isNew = false;
}
else
{
$templateId = $db->insertid();
}

$this->setState('com_tjnotifications.edit.notification.id', $templateId);
$this->setState('com_tjnotifications.edit.notification.new', $isNew);
}

if (empty($templateId))
{
$this->setError(Text::_('COM_TJNOTIFICATIONS_ERROR_TEMPLATE_ID_MISSING'));
return false;
}

Expand All @@ -377,7 +387,13 @@ public function save($data)
{
// 2.1 Check if current backend exists in posted data
// If not $data['email'] or $data['sms']
if (empty($data[$backend]))
if (empty($data[$backend]) || !isset($data[$backend][$backend . 'fields']))
{
continue;
}

// Validate backend data structure
if (!is_array($data[$backend][$backend . 'fields']))
{
continue;
}
Expand Down Expand Up @@ -454,22 +470,44 @@ public function save($data)
$this->deleteBackendConfigs($backendConfigIdsToBeDeleted);

// 2.3 Common data for saving
$createdOn = !empty($data['created_on']) ? $data['created_on'] : '';
$updatedOn = !empty($data['updated_on']) ? $data['updated_on'] : '';
$date = Factory::getDate();
$currentDateTime = $date->toSql(true);

// Always use current datetime for backend configs to avoid empty string issues
$createdOn = $currentDateTime;
$updatedOn = $currentDateTime;

// 2.4 try saving all backend specific configs
// This has repeatable data eg: $data['email']['emailfields'] or $data['sms']['smsfields']
foreach ($data[$backend][$backend . 'fields'] as $backendName => $backendFieldValues)
{
// Validate backend field values
if (!is_array($backendFieldValues) || empty($backendFieldValues))
{
continue;
}

$templateConfigTable = Table::getInstance('Template', 'TjnotificationTable', array('dbo', $db));
$templateConfigTable->load(array('template_id' => $templateId, 'backend' => $backendName));
$isExistingRecord = $templateConfigTable->load(array('template_id' => $templateId, 'backend' => $backend));

// Non-repeat data
$templateConfigTable->template_id = $templateId;
$templateConfigTable->backend = $backend;
$templateConfigTable->state = $data[$backend]['state'];
$templateConfigTable->created_on = $createdOn;
$templateConfigTable->updated_on = $updatedOn;

// Set datetime fields with proper values
// For existing records, only update the updated_on field
if ($isExistingRecord && !empty($templateConfigTable->created_on))
{
$templateConfigTable->updated_on = $updatedOn;
// Keep existing created_on value
}
else
{
// For new records, set both created_on and updated_on
$templateConfigTable->created_on = $createdOn;
$templateConfigTable->updated_on = $updatedOn;
}
Comment on lines 490 to +510
Copy link

@coderabbitai coderabbitai bot Sep 25, 2025

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

Do not preload the backend config with a generic template_id/backend lookup

Because $templateConfigTable->load() runs on every iteration with only template_id/backend, the table object keeps the first matching row’s primary key. When you later save a repeatable item whose id is empty, that stale primary key forces save() to overwrite an existing row instead of inserting a new one. Even when an id is present, the preserved created_on value now comes from whichever row happened to load first, so timestamps drift between language records. Please load by the actual repeatable row id (only when it’s provided) and keep the table reset for new rows so inserts stay inserts.

-               $templateConfigTable = Table::getInstance('Template', 'TjnotificationTable', array('dbo', $db));
-               $isExistingRecord = $templateConfigTable->load(array('template_id' => $templateId, 'backend' => $backend));
+               $templateConfigTable = Table::getInstance('Template', 'TjnotificationTable', array('dbo', $db));
+               $templateConfigTable->reset();
+
+               $isExistingRecord = false;
+
+               if (!empty($backendFieldValues['id']))
+               {
+                   $isExistingRecord = $templateConfigTable->load((int) $backendFieldValues['id']);
+               }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
$templateConfigTable = Table::getInstance('Template', 'TjnotificationTable', array('dbo', $db));
$templateConfigTable->load(array('template_id' => $templateId, 'backend' => $backendName));
$isExistingRecord = $templateConfigTable->load(array('template_id' => $templateId, 'backend' => $backend));
// Non-repeat data
$templateConfigTable->template_id = $templateId;
$templateConfigTable->backend = $backend;
$templateConfigTable->state = $data[$backend]['state'];
$templateConfigTable->created_on = $createdOn;
$templateConfigTable->updated_on = $updatedOn;
// Set datetime fields with proper values
// For existing records, only update the updated_on field
if ($isExistingRecord && !empty($templateConfigTable->created_on))
{
$templateConfigTable->updated_on = $updatedOn;
// Keep existing created_on value
}
else
{
// For new records, set both created_on and updated_on
$templateConfigTable->created_on = $createdOn;
$templateConfigTable->updated_on = $updatedOn;
}
$templateConfigTable = Table::getInstance('Template', 'TjnotificationTable', array('dbo', $db));
$templateConfigTable->reset();
$isExistingRecord = false;
if (!empty($backendFieldValues['id']))
{
$isExistingRecord = $templateConfigTable->load((int) $backendFieldValues['id']);
}
// Non-repeat data
$templateConfigTable->template_id = $templateId;
$templateConfigTable->backend = $backend;
$templateConfigTable->state = $data[$backend]['state'];
// Set datetime fields with proper values
// For existing records, only update the updated_on field
if ($isExistingRecord && !empty($templateConfigTable->created_on))
{
$templateConfigTable->updated_on = $updatedOn;
// Keep existing created_on value
}
else
{
// For new records, set both created_on and updated_on
$templateConfigTable->created_on = $createdOn;
$templateConfigTable->updated_on = $updatedOn;
}
🤖 Prompt for AI Agents 
In src/com_tjnotifications/admin/models/notification.php around lines 490-510,
avoid preloading the backend config using only template_id/backend because that
preserves the first row’s primary key and created_on across repeatable
iterations; instead, only call ->load() with the specific repeatable row id when
that id is present, and for new repeatable rows ensure the Table object is reset
(clear id/primary key and unset created_on) so save() performs an INSERT; when
an existing id is loaded, update updated_on only and leave created_on intact.


// Get params data
// State, emailfields / smsfields
Expand Down Expand Up @@ -505,14 +543,30 @@ public function save($data)
}

// Save backend in config table
if (empty($backendFieldValues['id']))
try
{
$templateConfigTable->save($templateConfigTable);
if (empty($backendFieldValues['id']))
{
if (!$templateConfigTable->save($templateConfigTable))
{
$this->setError(Text::sprintf('COM_TJNOTIFICATIONS_ERROR_SAVE_BACKEND_CONFIG', $backend, $templateConfigTable->getError()));
return false;
}
}
else
{
$templateConfigTable->id = $backendFieldValues['id'];
if (!$templateConfigTable->save($templateConfigTable))
{
$this->setError(Text::sprintf('COM_TJNOTIFICATIONS_ERROR_SAVE_BACKEND_CONFIG', $backend, $templateConfigTable->getError()));
return false;
}
}
}
else
catch (Exception $e)
{
$templateConfigTable->id = $backendFieldValues['id'];
$templateConfigTable->save($templateConfigTable);
$this->setError(Text::sprintf('COM_TJNOTIFICATIONS_ERROR_SAVE_BACKEND_CONFIG', $backend, $e->getMessage()));
return false;
}
}
}
Expand Down