From 7961e2909a8c8c456c1b0d6bee2944cabde592c0 Mon Sep 17 00:00:00 2001 From: Nik Okuntseff Date: Thu, 22 Nov 2018 19:33:31 +0000 Subject: [PATCH] Implemented delete group feature in group editor. --- WEB-INF/lib/ttAdmin.class.php | 2 +- WEB-INF/lib/ttGroupHelper.class.php | 105 ++++++++++++++++++++++++++++ WEB-INF/lib/ttUser.class.php | 12 ++-- WEB-INF/templates/footer.tpl | 2 +- group_delete.php | 46 +++++++----- 5 files changed, 142 insertions(+), 25 deletions(-) diff --git a/WEB-INF/lib/ttAdmin.class.php b/WEB-INF/lib/ttAdmin.class.php index ef55ae50..a2388edf 100644 --- a/WEB-INF/lib/ttAdmin.class.php +++ b/WEB-INF/lib/ttAdmin.class.php @@ -115,7 +115,7 @@ class ttAdmin { return true; } - // markGroupDeleted marks the group and everything in it as deleted. + // markGroupDeleted marks a group and everything in it as deleted. function markGroupDeleted($group_id) { // Keep the logic simple by returning false on first error. diff --git a/WEB-INF/lib/ttGroupHelper.class.php b/WEB-INF/lib/ttGroupHelper.class.php index 41c03114..a6d5fea7 100644 --- a/WEB-INF/lib/ttGroupHelper.class.php +++ b/WEB-INF/lib/ttGroupHelper.class.php @@ -100,4 +100,109 @@ class ttGroupHelper { return (!is_a($affected, 'PEAR_Error')); // TODO: design subgroup roles carefully. Perhaps roles are not to be touched in subgroups at all. } + + // markGroupDeleted marks a group and everything in it as deleted. + // This function is called in context of a logged on user (global $user object). + // It uses current user attributes for access checks and in sql queries. + // Compare this with admin: + // admin can delete any group. + // user can delete only relevant groups and only if allowed. + static function markGroupDeleted($group_id) { + global $user; + + $mdb2 = getConnection(); + $org_id = $user->org_id; + + // Security check. + if (!$user->isGroupValid($group_id)) + return false; + + // Keep the logic simple by returning false on first error. + + // Obtain subgroups and call self recursively on them. + $subgroups = $user->getSubgroups($group_id); + foreach($subgroups as $subgroup) { + if (!$this->markGroupDeleted($subgroup['id'])) + return false; + } + + // Now do actual work with all entities. + + // Some things cannot be marked deleted as we don't have the status field for them. + // Just delete such things (until we have a better way to deal with them). + $tables_to_delete_from = array( + 'tt_config', + 'tt_predefined_expenses', + 'tt_client_project_binds', + 'tt_project_task_binds' + ); + foreach($tables_to_delete_from as $table) { + if (!ttGroupHelper::deleteGroupEntriesFromTable($group_id, $table)) + return false; + } + + // Now mark status deleted where we can. + // Note: we don't mark tt_log, tt_custom_field_lod, or tt_expense_items deleted here. + // Reasoning is: + // + // 1) Users may mark some of them deleted during their work. + // If we mark all of them deleted here, we can't recover nicely + // as we'll lose track of what was accidentally deleted by user. + // + // 2) DB maintenance script (Clean up DB from inactive groups) should + // get rid of these items permanently eventually. + $tables_to_mark_deleted_in = array( + 'tt_cron', + 'tt_fav_reports', + // 'tt_expense_items', + // 'tt_custom_field_log', + 'tt_custom_field_options', + 'tt_custom_fields', + // 'tt_log', + 'tt_invoices', + 'tt_user_project_binds', + 'tt_users', + 'tt_clients', + 'tt_projects', + 'tt_tasks', + 'tt_roles' + ); + foreach($tables_to_mark_deleted_in as $table) { + if (!ttGroupHelper::markGroupDeletedInTable($group_id, $table)) + return false; + } + + // Mark group deleted. + $sql = "update tt_groups set status = null where id = $group_id and org_id = $org_id"; + $affected = $mdb2->exec($sql); + if (is_a($affected, 'PEAR_Error')) return false; + + return true; + } + + // markGroupDeletedInTable is a generic helper function for markGroupDeleted. + // It updates ONE table by setting status to NULL for all records belonging to a group. + static function markGroupDeletedInTable($group_id, $table_name) { + global $user; + $mdb2 = getConnection(); + + // TODO: add modified info to sql for some tables, depending on table name. + + $org_id = $user->org_id; // The only security measure we use here for match. + $sql = "update $table_name set status = null where group_id = $group_id and org_id = $org_id"; + $affected = $mdb2->exec($sql); + return (!is_a($affected, 'PEAR_Error')); + } + + // deleteGroupEntriesFromTable is a generic helper function for markGroupDeleted. + // It deletes entries in ONE table belonging to a given group. + static function deleteGroupEntriesFromTable($group_id, $table_name) { + global $user; + $mdb2 = getConnection(); + + $org_id = $user->org_id; // The only security measure we use here for match. + $sql = "delete from $table_name where group_id = $group_id and org_id = $org_id"; + $affected = $mdb2->exec($sql); + return (!is_a($affected, 'PEAR_Error')); + } } diff --git a/WEB-INF/lib/ttUser.class.php b/WEB-INF/lib/ttUser.class.php index 16b536c2..ad5c0410 100644 --- a/WEB-INF/lib/ttUser.class.php +++ b/WEB-INF/lib/ttUser.class.php @@ -380,14 +380,14 @@ class ttUser { if ($selected_group_id != $this->group_id) { // We are in one of subgroups, and a parent exists. // Get parent group info. - $sql = "select parent_id from tt_groups where org_id = $this->org_id and id = $selected_group_id"; + $sql = "select parent_id from tt_groups where org_id = $this->org_id and id = $selected_group_id and status = 1"; $res = $mdb2->query($sql); if (!is_a($res, 'PEAR_Error')) { $val = $res->fetchRow(); $parent_id = $val['parent_id']; if ($parent_id) { // Get parent group name. - $sql = "select name from tt_groups where org_id = $this->org_id and id = $parent_id"; + $sql = "select name from tt_groups where org_id = $this->org_id and id = $parent_id and status = 1"; $res = $mdb2->query($sql); if (!is_a($res, 'PEAR_Error')) { $val = $res->fetchRow(); @@ -401,11 +401,12 @@ class ttUser { $groups[] = array('id'=>$selected_group_id,'name'=>$selected_group_name); // Add subgroups. - $sql = "select id, name from tt_groups where org_id = $this->org_id and parent_id = $selected_group_id"; + $sql = "select id, name from tt_groups where org_id = $this->org_id and parent_id = $selected_group_id and status = 1"; + //die($sql); $res = $mdb2->query($sql); if (!is_a($res, 'PEAR_Error')) { while ($val = $res->fetchRow()) { - $groups[] = array('id'=>$val['id'],'name'=>$val['name']); + $groups[] = $val; } } return $groups; @@ -417,7 +418,8 @@ class ttUser { if (!$group_id) $group_id = $this->getActiveGroup(); - $sql = "select id, name, description from tt_groups where org_id = $this->org_id and parent_id = $group_id"; + $sql = "select id, name, description from tt_groups where org_id = $this->org_id". + " and parent_id = $group_id and status is not null"; $res = $mdb2->query($sql); if (!is_a($res, 'PEAR_Error')) { while ($val = $res->fetchRow()) { diff --git a/WEB-INF/templates/footer.tpl b/WEB-INF/templates/footer.tpl index 0419e9e9..50749d42 100644 --- a/WEB-INF/templates/footer.tpl +++ b/WEB-INF/templates/footer.tpl @@ -12,7 +12,7 @@
-
 Anuko Time Tracker 1.18.27.4494 | Copyright © Anuko | +  Anuko Time Tracker 1.18.27.4495 | Copyright © Anuko | {$i18n.footer.credits} | {$i18n.footer.license} | {$i18n.footer.improve} diff --git a/group_delete.php b/group_delete.php index b2977ee4..db24a448 100644 --- a/group_delete.php +++ b/group_delete.php @@ -28,26 +28,21 @@ require_once('initialize.php'); import('form.Form'); -import('ttAdmin'); +import('ttGroupHelper'); // Access checks. if (!ttAccessAllowed('delete_group')) { - header('Location: access_denied.php'); + header('Location: access_denied.php'); // No rights. exit(); } -$group_id = (int)$request->getParameter('id'); -if ($user->group_id != $group_id) { - header('Location: access_denied.php'); +if (!$user->isGroupValid($request->getParameter('id'))) { + header('Location: access_denied.php'); // Wrong group id. exit(); } // End of access checks. -// Note: reuse ttAdmin class here because deleting a group is a complicated task. -// This creates an issue of using the class for not intended purpose. -// However, otherwise we have to duplicate code, so reuse it is, for now. -$admin = new ttAdmin(); -$group_details = $admin->getGroupDetails($group_id); -$group_name = $group_details['group_name']; +$group_id = (int)$request->getParameter('id'); +$group_name = ttGroupHelper::getGroupName($group_id); $form = new Form('groupForm'); $form->addInput(array('type'=>'hidden','name'=>'id','value'=>$group_id)); @@ -56,18 +51,33 @@ $form->addInput(array('type'=>'submit','name'=>'btn_cancel','value'=>$i18n->get( if ($request->isPost()) { if ($request->getParameter('btn_delete')) { - if ($admin->markGroupDeleted($group_id)) { - $auth->doLogout(); - session_unset(); - header('Location: login.php'); - exit(); + $markedDeleted = ttGroupHelper::markGroupDeleted($group_id); + if ($markedDeleted) { + // TODO: conditional redirects don't look nice. Any better ideas? + if ($group_id == $user->group_id) { + // We marked deleted our own group. Logout and redirect to login page. + $auth->doLogout(); + session_unset(); + header('Location: login.php'); + exit(); + } else { + // We marked deleted a subgroup. Redirect to groups.pgp. + header('Location: groups.php'); + exit(); + } } else $err->add($i18n->get('error.db')); } if ($request->getParameter('btn_cancel')) { - header('Location: group_edit.php'); - exit(); + // TODO: conditional redirects don't look nice. Any better ideas? + if ($group_id == $user->group_id) { + header('Location: group_edit.php'); + exit(); + } else { + header('Location: groups.php'); + exit(); + } } } // isPost -- 2.20.1