Simplify php code

[sci2tech]

I know that this was already discussed but all code is still a mess. Checks are missing or are redundant. Same function in many file. Message passed with $_SESSION and header. Let`s take a case:
admin/delete_user.php:

Code:

require '../include/ispcp-lib.php';
check_login(__FILE__);
/* do we have a proper delete_id ? */
if (!isset($_GET['delete_id']) or !is_numeric($_GET['delete_id'])) {
  header( "Location: manage_users.php" );
  die();
}

So if for some reason (bad template as example) we will never be able to delete a user, allways will end at manage_users.php without known why

Code:

$delete_id = $_GET['delete_id'];

$query = <<<SQL_QUERY
    select
        admin_type
    from
        admin
    where
        admin_id=?
SQL_QUERY;
$rs = exec_query($sql, $query, array($delete_id));

$local_admin_type = $rs->fields['admin_type'];

What if GET variable is not numeric? or id does not exist ( no harm at all but some query are made without result and a messege that a user was removed

Code:

if ($local_admin_type == 'admin') {
  $query = <<<SQL_QUERY
        select
            count(admin_id) as children
        from
            admin
        where
            created_by = ?
SQL_QUERY;

} else if ($local_admin_type == 'reseller') {
  $query = <<<SQL_QUERY
        select
            count(admin_id) as children from admin
        where
            created_by = ?
SQL_QUERY;

} else if ($local_admin_type == 'user') {
  $query = <<<SQL_QUERY
        select
            count(domain_id) as children from domain
        where
            domain_admin_id = ?
SQL_QUERY;

}
$rs = exec_query($sql, $query, array($delete_id));

if ($rs->fields['children'] > 0 && $local_admin_type !== 'user') {
  /* this user have domain ! */
  $hdomain = 1;
  $_SESSION['hdomain'] = 1;
  header("Location: manage_users.php");
  die();
} else {

All this code to check if user is a reseller or admin and have depending users respectively resellers/users

Code:

if ($local_admin_type == 'admin' || $local_admin_type == 'reseller') {
    $query = <<<SQL_QUERY
        SELECT
            count(admin_id) as children
        FROM
            `admin`
        WHERE
            `created_by` = ?
SQL_QUERY;
    if ($rs->fields['children'] > 0) {
        /* this user have domain ! */
        $hdomain = 1;
        $_SESSION['hdomain'] = 1;//that hurts
        header("Location: manage_users.php");//this too
        die();
}

is enough.

Code:

if ($local_admin_type == 'admin') {
    $query = <<<SQL_QUERY
            delete
                from email_tpls
            where
                owner_id = ? and
                name = 'add-user-auto-msg'
SQL_QUERY;
    $rs = exec_query($sql, $query, array($delete_id));

What is this admin has hosting plans? (btw if admin has hosting plans there is no way to add new orders)

Code:

} else if ($local_admin_type == 'reseller') {
    $query = <<<SQL_QUERY
            delete
                from email_tpls
            where
                owner_id = ? and
                name = 'add-user-auto-msg'
SQL_QUERY;
    $rs = exec_query($sql, $query, array($delete_id));

    $query = <<<SQL_QUERY
            delete
                from reseller_props
            where
                reseller_id = ?
SQL_QUERY;
    $rs = exec_query($sql, $query, array($delete_id));

    // delete orders
     $query = <<<SQL_QUERY
            delete from
                orders
            where
                user_id  = ?

SQL_QUERY;
    $rs = exec_query($sql, $query, array($delete_id));

    // delete orders settings
     $query = <<<SQL_QUERY
            delete from
                orders_settings
            where
                user_id  = ?

SQL_QUERY;
    $rs = exec_query($sql, $query, array($delete_id));


    $query = <<<SQL_QUERY
            delete
                from hosting_plans
            where
                reseller_id = ?
SQL_QUERY;
    $rs = exec_query($sql, $query, array($delete_id));

  } else if ($local_admin_type == 'user') {
    rm_rf_user_account($delete_id);

Also this function don`t clear quotalimits, quotatallies and domain_traffic(!!! this is a ticket if i remember) tables

Code:

check_for_lock_file();
    send_request();
  }

  $query = <<<SQL_QUERY
        delete
            from admin
        where
            admin_id = ?
SQL_QUERY;
  $rs = exec_query($sql, $query, array($delete_id));

  $query = <<<SQL_QUERY
            delete
                from user_gui_props
            where
                user_id = ?
SQL_QUERY;
  $rs = exec_query($sql, $query, array($delete_id));
  $user_logged= $_SESSION['user_logged'];
  $local_admin_name = $_GET['delete_username'];
  write_log("$user_logged: deletes user $local_admin_name, $local_admin_type, $delete_id!");
  $_SESSION['user_deleted'] = 1;
  header("Location: manage_users.php");//ouch
  die();
}

So before going to implement any new feature I think this mess should be cleaned. And to clean this mess i think that must remove php4 support (end of life) and go oop (like vacancy started). As far as I see there are some users here that can help with this (ispcomm come in my mind). If my time will permit I`ll take every file to analyze and try to clean but this is a Sisif work. Is better to start plan a oop structure than must mantains compatibility with current engine. And this must happend (in my opinion) before release a stable version.
I hope not to annoy anybody by reopening this problem again.

[sci2tech]

Include/login.php

Code:

function unset_user_login_data ($ignorePreserve = false) {
    $sql = Database::getInstance();

    if (isset($_SESSION['user_logged'])) {

        $sess_id = session_id();

        $admin_name = $_SESSION['user_logged'];

        $query = <<<SQL_QUERY
            delete from
              login
          where
              session_id = ?
          and
              user_name = ?
SQL_QUERY;

        $rs = exec_query($sql, $query, array($sess_id, $admin_name));

    }

    $preserve_list = array('user_def_lang', 'user_theme');
    $preserve_vals = array();

    if (!$ignorePreserve) {
        foreach ($preserve_list as $p) {
            if (isset($_SESSION[$p])) {
                $preserve_vals[$p] = $_SESSION[$p];
            }
        }
    }

    $_SESSION = array();

    foreach ($preserve_list as $p) {
        if (isset($preserve_vals[$p])) {
            $_SESSION[$p] = $preserve_vals[$p];
        }
    }

}

can become

Code:

function unset_user_login_data ($ignorePreserve = false) {
    $sql = Database::getInstance();

    if (isset($_SESSION['user_logged'])) {
        $sess_id = session_id();
        $admin_name = $_SESSION['user_logged'];
        $query = "DELETE FROM `login` WHERE `session_id` = ? AND `user_name` = ?";
        $rs = exec_query($sql, $query, array($sess_id, $admin_name));
    }
    $preserve_list = array('user_def_lang', 'user_theme');//may a Config::get here?
    foreach ($_SESSION as $key=>$value)
        if(!in_array($key,$preserve_list)||$ignorePreserve)
            unset($_SESSION[$key])
}

[Zothos]

verify, test twice, and make a patch Ill commit it then ^^ Would love to see some simplifications patches KISS is the way

[sci2tech]

Ok I`ll start now and every day I hope to update a file .

[xister]

great !

[marryroy01]

I am marry and I am a php programming trainee. Awesome PHP coding by you. I tested and It is working perfectly. If you are having more coding then please reply it so wecan learn new things. I am also sharing something which I know it is new for the others. This is really great forum.

[joximu]

This thread is very old - closing