Jump to content

How can I optimize my PHP code for better performance


djohnstone

Recommended Posts

I am very new to PHP and have tried various techniques but I am getting a 500 error when clicking on the export button to download a csv report. I'm not sure why the previous developer did it this way.

Is there a better why in PHP to make this code better? Willing to understand and learn from an PHP expert.

The database is MYSQL.

            $coursefilterid = $_GET['course'];

            $conn = new mysqli($host, $username, $password, $database);
            if ($conn->connect_error) {
                die("Connection failed: " . $conn->connect_error);
            }

            $sqluserenrolled = "select mdl_user.username, mdl_user_enrolments.userid as enrolleduserid, mdl_enrol.courseid from mdl_user_enrolments
    Inner Join
    mdl_enrol
    on 
    mdl_enrol.id = mdl_user_enrolments.enrolid
    Inner Join
    mdl_user
    on
    mdl_user.id = mdl_user_enrolments.userid
    where  
    mdl_enrol.courseid = '" . $coursefilterid . "'
    order by mdl_user.username
    
    ";

            $queryenrolleduser = mysqli_query($conn, $sqluserenrolled);

            ?>

<html>
<head>
</head>
<body>

    <form method="post"
        action="<?php echo "userlistssiexport.php?id=$coursefilterid"?>">
        <input type="hidden" name="exportcourseid"
            value="<?php echo $coursefilterid;?>"> <input type="hidden"
            name="sessid" value="<?php echo $USER->sesskey;?>"> <input
            class="btn btn-primary" type="submit" name="submit"
            value="<?php echo "Export";?>">
    </form>


<?php
            $noteid = "";
            $cmId = "";
            ?>
          
    <table class="data-table">
        <caption class="title">User info</caption>
        <thead>
            <tr>
                <th>Username</th>
                <th>Firstname</th>
                <th>Lastname</th>
                <th>Email</th>
                <th>Last login</th>
                <th>Createddate</th>
                <th>Position</th>
                <th>Organization</th>
                <th>Certificate Request Date</th>
                <th>Role1</th>
                <th>Role2</th>
                <th>Role3</th>
            </tr>
        </thead>
        <tbody>

    <?php

            while ($row = mysqli_fetch_array($queryenrolleduser)) {

                $enrolleduserid = $row['enrolleduserid'];

                $sql = "select mdl_user.username as username, 
        mdl_user.firstname as firstname,
        mdl_user.lastname as lastname,
        mdl_user.email as email,
       mdl_user.lastlogin as lastaccess,
        mdl_user.timecreated as createddate,
        mdl_user_info_data.data as position 
        
        
        from mdl_user
        Inner Join 
        mdl_user_info_data
        on
        mdl_user_info_data.userid = mdl_user.id
        Inner Join
        mdl_user_info_field
        on 
        mdl_user_info_field.id = mdl_user_info_data.fieldid
        Inner Join
        mdl_user_lastaccess
        on
        mdl_user_lastaccess.userid = mdl_user.id
        
        
        where  mdl_user_info_field.id = 1
        and
        mdl_user.deleted = 0
        and
        mdl_user.id = '" . $enrolleduserid . "'
        group by mdl_user.username
        order by mdl_user.username
    
        ";
                $query = mysqli_query($conn, $sql);

                if (! $query) {
                    die('SQL Error: ' . mysqli_error($conn));
                } else {}

                ?>

        <?php
                $no = 1;
                $total = 0;
                $username = '';
                $coursename = '';
                $content = '';
                $modulename = '';
                $organization = '';
                $userid = '';
                $certificatedate = '';
                $userrole = '';
                $enrolleduserid = '';

                while ($row = mysqli_fetch_array($query)) {
                    // Do something here
                    $username = $row['username'];
                    $coursename = $row['coursename'];
                    $content = $row['content'];
                    $noteid = $row['noteid'];
                    // $notedatetime = date("d/m/y g:i (A)", $row['notedate']);
                    $notedatetime = date("D M j Y G:i A", $row['notedate']);
                    $lastaccess = date("D M j Y G:i A", $row['lastaccess']);
                    $createddate = date("D M j Y G:i A", $row['createddate']);
                    $datafile = $username . $coursename . $content;

                    echo '<tr>      
                    <td>' . $row['username'] . '</td>
                    <td>' . $row['firstname'] . '</td>
                    <td>' . $row['lastname'] . '</td>
                    <td>' . $row['email'] . '</td>                          
                    <td>' . $lastaccess . '</td>                                    
                    <td>' . $createddate . '</td>   
                    <td>' . $row['position'] . '</td>    
                                                    
                ';
                    $modid = $row['contextid'];
                    // Get module name

                    $sqlmodule = "select mdl_user.username as username, 
        mdl_user.firstname as firstname,
        mdl_user.lastname as lastname,
        mdl_user.email as email,
        FROM_UNIXTIME(mdl_user_lastaccess.timeaccess) as lastaccess,
        FROM_UNIXTIME(mdl_user.timecreated) as createddate,
        mdl_user_info_data.data as organization 
        
        
        from mdl_user
        Inner Join 
        mdl_user_info_data
        on
        mdl_user_info_data.userid = mdl_user.id
        Inner Join
        mdl_user_info_field
        on 
        mdl_user_info_field.id = mdl_user_info_data.fieldid
        Inner Join
        mdl_user_lastaccess
        on
        mdl_user_lastaccess.userid = mdl_user.id
        
        
        where  mdl_user_info_field.id = 3
        and
        mdl_user.deleted = 0
        and
        mdl_user.username ='" . $username . "'";

                    $querymodule = mysqli_query($conn, $sqlmodule);
                    ?>              
            <?php
                    $modulenamelink = "";
                    while ($row = mysqli_fetch_array($querymodule)) {

                        $organization = $row['organization'];
                    }

                    echo '<td>' . $organization . '</td>';

                    $sqlCertificateDateuid = "select id from mdl_user where username = '" . $username . "'";

                    $queryCertificateDateuid = mysqli_query($conn, $sqlCertificateDateuid);
                    while ($row = mysqli_fetch_array($queryCertificateDateuid)) {

                        $userid = $row['id'];
                    }

                    $sqlcertificatedate = "select * from mdl_certificateemail where userid = '" . $userid . "'
        and courseid = '" . $coursefilterid . "'";

                    $querycertificaterequestdate = mysqli_query($conn, $sqlcertificatedate);
                    while ($row = mysqli_fetch_array($querycertificaterequestdate)) {

                         $certificatedate = date("D M j Y g:i:s A", $row['unixdatetimecertificate']);
                    }
                    echo '<td>' . $certificatedate . '</td>';

                    $sqluserrole = "select mdl_role_assignments.userid, mdl_role_assignments.roleid,mdl_course_modules.course, mdl_role.shortname as rolename,FROM_UNIXTIME(mdl_role_assignments.timemodified)  from mdl_role_assignments 
        Inner Join
        mdl_context
        on
        mdl_context.id = mdl_role_assignments.contextid
        Inner Join
        mdl_course_modules
        on 
        mdl_course_modules.instance = mdl_context.instanceid
        Inner Join
        mdl_role
        on 
        mdl_role.id = mdl_role_assignments.roleid
        
        where mdl_course_modules.course = '" . $coursefilterid . "'
        and
        mdl_role_assignments.userid = '" . $userid . "'
        group by
        mdl_role_assignments.userid,
        mdl_role_assignments.roleid,
        mdl_course_modules.course,
        mdl_role.shortname,
        mdl_role_assignments.timemodified
        
        order by mdl_role_assignments.timemodified
        ";

                    $userlistrole = '';
                    $queryuserrole = mysqli_query($conn, $sqluserrole);
                    while ($row = mysqli_fetch_array($queryuserrole)) {

                        $userrole = $row['rolename'];
                        $userlistrole = array(
                            array(
                                $userrole
                            )
                        );
                        // echo '<td>'.$userrole.'</td>';
                    }

                    foreach ($userlistrole as $listrole) {

                        // echo $listrole;
                    }

                    $teacherrole = array(
                        'student'
                    );
                    foreach ($teacherrole as $rolename) {
                        $role = $DB->get_record('role', array(
                            'shortname' => $rolename
                        ));
                        $context = get_context_instance(CONTEXT_COURSE, $coursefilterid);
                        // $context = context_course::instance($cid1);
                        $teachers = get_role_users($role->id, $context);

                        foreach ($teachers as $teacher) {

                            $teacherid = $teacher->id;
                            if ($teacherid == $userid) {
                                echo '<td>student</td>';
                            }
                        }
                    }

                    $teacherrole = array(
                        'editingteacher'
                    );
                    foreach ($teacherrole as $rolename) {
                        $role = $DB->get_record('role', array(
                            'shortname' => $rolename
                        ));
                        $context = get_context_instance(CONTEXT_COURSE, $coursefilterid);
                        // $context = context_course::instance($cid1);
                        $teachers = get_role_users($role->id, $context);

                        foreach ($teachers as $teacher) {

                            $teacherid = $teacher->id;
                            if ($teacherid == $userid) {
                                echo '<td></td>';
                                echo '<td>editingteacher</td>';
                            }
                        }
                    }

                    $teacherrole = array(
                        'manager'
                    );
                    foreach ($teacherrole as $rolename) {
                        $role = $DB->get_record('role', array(
                            'shortname' => $rolename
                        ));
                        $context = get_context_instance(CONTEXT_COURSE, $coursefilterid);
                        // $context = context_course::instance($cid1);
                        $teachers = get_role_users($role->id, $context);

                        foreach ($teachers as $teacher) {

                            $teacherid = $teacher->id;
                            if ($teacherid == $userid) {
                                echo '<td>manager</td>';
                            }
                        }
                    }
                    echo '</tr>';
                }
            }

            ?>

    
        </tbody>
        <tfoot>

        </tfoot>
    </table>
</body>
</html>

<?php
    }
}

else {
    header("Location:/index.php");
    // echo "something";
    die();
}
}

else {

header("Location:/index.php");
die();

}

?>

 

Link to comment
Share on other sites

Without looking too closely I'm pretty sure there's significant room for improvement.

But don't do that yet. If you get a 500 error then that means the script is crashing. You need to fix that.
Check your server and/or PHP error logs for information about what went wrong.

Link to comment
Share on other sites

1 hour ago, djohnstone said:

export button

the code handling that from submission isn't even part of the posted code, though it should be - the form should submit to the same page it is on, so the most immediate problem is in some different code.

1 hour ago, djohnstone said:

The PHP error logs are clean, which is strange. I receive in the browser "The service is temporarily unavailable. Please try again later."  which has a status code of 500 in Chrome.

you need to find the php.ini that php is using and set error_reporting to E_ALL and set display_errors to ON, so that php will report and display all the errors it detects. stop and start your web server to insure that any changes made to the php.ini will take effect and then use a phpinfo(); statement in a .php script file to check that those settings actually got set to those values.

Link to comment
Share on other sites

Hi Mac, I have tried that and it's still not displaying any errors. The phpinfo(); statement worked. 

Really appreciate everyone's help. 

I even added this code to the top of my PHP file as suggested by other online articles and restarted the apache2 server using sudo service apache2 restart

ini_set('display_errors', '1');
ini_set('display_startup_errors', '1');
error_reporting(E_ALL);

I starting to think it might be timing out due to overload on the DB? Is that possible? It's using an jQuery 3.2 AJAX request which calls the above backend code.

PHP is not my strong point, so would optimizing the code and the way it calls the SQL selects help?

Link to comment
Share on other sites

7 hours ago, djohnstone said:

would optimizing the code and the way it calls the SQL selects help?

you have to find what's causing a problem in order to fix it, otherwise you are just putting Band-Aids on top of symptoms, and the real problem remains. there are significant problems in the posted code - no comments to let anyone know what the intent of any section is, variables created for nothing, inconstant/no error handling, putting external data directly into sql query statements, running queries inside of loops...

you will only see a http 500 error page in a browser due to a 'primary' request to a page, not an ajax request. if you are seeing a http 500 error page, it's for the action of the form - userlistssiexport.php?id=$coursefilterid. what is the code for userlistssiexport.php? and it's not any of the posted code because there is no $_GET['id'] or $_POST variables, the data that the form will submit, anywhere in the code you have posted.

 

Link to comment
Share on other sites

AFAIR, if you were timing out you'd be seeing a 504 Bad Gateway error, so it's a problem with the form handling code. If you've turned on error reporting and verified it through phpinfo(), you should be seeing the problem in the browser. Make sure you've enabled 'display_startup_errors' in the same php.ini - your script may be crashing before it even gets to your code.

Link to comment
Share on other sites

This thread is more than a year old. Please don't revive it unless you have something important to add.

Join the conversation

You can post now and register later. If you have an account, sign in now to post with your account.

Guest
Reply to this topic...

×   Pasted as rich text.   Restore formatting

  Only 75 emoji are allowed.

×   Your link has been automatically embedded.   Display as a link instead

×   Your previous content has been restored.   Clear editor

×   You cannot paste images directly. Upload or insert images from URL.

×
×
  • Create New...

Important Information

We have placed cookies on your device to help make this website better. You can adjust your cookie settings, otherwise we'll assume you're okay to continue.