Skip to content

Commit 4ac0ed9

Browse files
committed
fix: resolve code review issues in phonebook module
Fix critical bug where $result.getMessages() was called on boolean instead of record object. Add null check before accessing record properties in save action. Optimize mass delete to use direct SQL instead of iterating over records. Add type casting and empty row handling for Excel import. Remove duplicate search handler in datatable JS. Update Settings.php license header to GPL-3.0.
1 parent 83f3589 commit 4ac0ed9

File tree

6 files changed

+118
-88
lines changed

6 files changed

+118
-88
lines changed

App/Controllers/ModulePhoneBookController.php

Lines changed: 12 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -168,7 +168,7 @@ public function saveAction(): void
168168
$record = null;
169169
if (stripos($dataId, 'new') === false) {
170170
$record = PhoneBook::findFirstById($dataId);
171-
if ($record->number !== $number) {
171+
if ($record !== null && $record->number !== $number) {
172172
$record->delete();
173173
$record = null;
174174
}
@@ -213,16 +213,18 @@ public function deleteAction(?string $id = null): void
213213
*/
214214
public function deleteAllRecordsAction(): void
215215
{
216-
$records = PhoneBook::find();
217-
foreach ($records as $record) {
218-
if (!$record->delete()) {
219-
$this->flash->error(implode('<br>', $record->getMessages()));
220-
$this->view->result = false;
221-
return;
222-
}
216+
$phoneBook = new PhoneBook();
217+
$connection = $phoneBook->getWriteConnection();
218+
$tableName = $phoneBook->getSource();
219+
220+
try {
221+
$connection->execute("DELETE FROM {$tableName}");
222+
$this->view->result = true;
223+
$this->view->reload = 'module-phone-book/module-phone-book/index';
224+
} catch (\Throwable $e) {
225+
$this->flash->error($e->getMessage());
226+
$this->view->result = false;
223227
}
224-
$this->view->result = true;
225-
$this->view->reload = 'module-phone-book/module-phone-book/index';
226228
}
227229

228230
/**

Lib/PhoneBookImport.php

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -65,8 +65,13 @@ public function run(string $uploadedFilePath): PBXApiResult
6565

6666
// Iterate over rows and process each record
6767
for ($row = 2; $row <= $highestRow; ++$row) {
68-
$callId = $sheet->getCell([1, $row])->getValue();
69-
$numberRep = $sheet->getCell([2, $row])->getValue();
68+
$callId = (string)($sheet->getCell([1, $row])->getValue() ?? '');
69+
$numberRep = (string)($sheet->getCell([2, $row])->getValue() ?? '');
70+
71+
// Skip empty rows
72+
if (empty($callId) && empty($numberRep)) {
73+
continue;
74+
}
7075

7176
$res = $this->savePhonebookRecord($callId, $numberRep);
7277
if (!$res->success) {

Models/Settings.php

Lines changed: 14 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,21 @@
11
<?php
22

3-
/**
4-
* Copyright © MIKO LLC - All Rights Reserved
5-
* Unauthorized copying of this file, via any medium is strictly prohibited
6-
* Proprietary and confidential
7-
* Written by Alexey Portnov, 2 2019
8-
*/
9-
103
/*
11-
* https://docs.phalconphp.com/3.4/ru-ru/db-models-metadata
4+
* MikoPBX - free phone system for small business
5+
* Copyright © 2017-2024 Alexey Portnov and Nikolay Beketov
6+
*
7+
* This program is free software: you can redistribute it and/or modify
8+
* it under the terms of the GNU General Public License as published by
9+
* the Free Software Foundation; either version 3 of the License, or
10+
* (at your option) any later version.
11+
*
12+
* This program is distributed in the hope that it will be useful,
13+
* but WITHOUT ANY WARRANTY; without even the implied warranty of
14+
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
15+
* GNU General Public License for more details.
1216
*
17+
* You should have received a copy of the GNU General Public License along with this program.
18+
* If not, see <https://www.gnu.org/licenses/>.
1319
*/
1420

1521
namespace Modules\ModulePhoneBook\Models;

Setup/PbxExtensionSetup.php

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -46,9 +46,8 @@ public function installDB(): bool
4646
$username = mb_strtolower($record->call_id);
4747
// Combine all fields into a single string
4848
$record->search_index = $username . $record->number . $record->number_rep;
49-
$result = $record->save();
50-
if (!$result) {
51-
SystemMessages::sysLogMsg(__METHOD__, implode(' ', $result->getMessages()));
49+
if (!$record->save()) {
50+
SystemMessages::sysLogMsg(__METHOD__, implode(' ', $record->getMessages()));
5251
return false;
5352
}
5453
}

0 commit comments

Comments
 (0)