1
\$\begingroup\$

I was given an opportunity to join a WordPress contractor/freelancer website and was asked to build a plugin that would list the wordpress users in the database, allow you to filter them based on their role, allow you sort them based on the display_name and the username, and have pagination.

But the response I got from them was quite blunt, with no pointers in what would have made the code better etc. I get that they have to have the best of the best, but I have been building websites for 15 years and programming for many more.

I don't comment my code. Any comments would go in git comments, methods and variables would be named in a way that it is obvious what is going on e.g. getUsers, renderScripts.

Anyway, they knocked me back and I want to be proactive by learning from this, hopefully making me a better developer in the future.

So as this is a code review site... could someone help point out why I would get a response as harsh as "pretty bad overall"? It could be I transferred my .net programming style into PHP which isn't the done thing?

//codeitwise.php

<?php
   /*
   Plugin Name: Code It Wise Test
   Plugin URI: 
   description: This is my test plugin for a certain website.
   Version: 0.1
   Author: Me
   Author URI:
   License: GPL2
   */

require(__DIR__ . '\codeitwise-admin-page.php');
class CodeItWise {

    function __construct() {        
        add_action( 'admin_menu', array( 'CodeItWise', 'codeitwise__adminMenuActions' ) );
    }

    public static function codeitwise__adminMenuActions() {
        add_menu_page( 'Code It Wise - Codeable Test', 'Code It Wise', 'manage_options', 'ciw-admin-page.php', 'CodeItWise__Admin::renderAdminPage', 'dashicons-admin-generic', 6  );       
    }
}

$ciw = new CodeItWise();

?>

//codeitwise-admin-page.php

<?php
    class CodeItWise__Admin {

        function __construct() {        
            add_action( 'admin_enqueue_scripts', 'CodeItWise__Admin::renderScripts' );
            add_action( 'wp_ajax_get_users', 'CodeItWise__Admin::getUsers' );
        }

        public static function renderAdminPage ()
        {?>
            <div class="wrap">
                <h2>Code It Wise - Codeable Test</h2>
            </div>
            <?php 
                global $wp_roles;
                $all_roles = $wp_roles->get_names();
            ?>
            <div class="tablenav top">
                <div class="alignleft">
                    <label class="screen-reader-text">Filter role</label>
                    <select name="role" js-filter-role>
                        <option value="">Roles</option>
                        <?php                       
                            foreach($all_roles as $role) 
                            {
                                echo '<option value="' . $role. '">' . $role . '</option>';
                            }
                        ?>                                              
                    </select>
                </div>
            </div>          
            <div class="wrap" js-users-container>
                <!-- the table will be loaded in here -->
            </div>
            <div class="wrap" js-pagination-container>

            </div>
            <?php
            include(__DIR__ . "\handlebar-templates.html");
        }

        public static function renderScripts() {
            wp_enqueue_script('codeitwise__handlebars', plugin_dir_url(__FILE__) . '/scripts/handlebars-v4.0.10.min.js');
            wp_enqueue_script('codeitwise__core', plugin_dir_url(__FILE__) . '/scripts/codeitwise.js');
        }       

        public static function getUsers() {

            $role = $_POST['role'];
            $page = $_POST['page'];
            $orderby = $_POST['orderby'];
            $order = $_POST['order'];

            if(!isset($page)) {
                $page = 1;
            }

            $page_size = 10;
            $columns = array(
                'display_name',
                'user_login'                
            );
            $meta_key = array(

            );

            $args = array(
                'role'          => $role,
                'offset'        => (($page - 1) * $page_size),
                'number'        => $page_size,              
                'fields'        => $columns,
                'orderby'       => $orderby,
                'order'         => $order
             ); 

            $counted = count_users();           
            $count = ($role === "" ? $counted["total_users"] : $counted["avail_roles"][strtolower($role)]);
            if($count === null) $count = 0;
            $response= array(
                'users'             => get_users($args),
                'total_count'       => $count,
                'role'              => $role, 
                'page'              => $page,
                'page_size'         => $page_size,
                'orderby'           => $orderby,
                'order'             => $order
            );
            wp_send_json_success( $response );

        }       
    }

    $ciw_admin = new CodeItWise__Admin();
?>

// scripts.js

(function () {
    const captains = console;
    const $ = jQuery;
    const defaultOrder = "display_name";

    let role = "";    
    let column = defaultOrder;
    let direction = "DESC";

    Handlebars.registerHelper('if_even', function (index) {
        if ((index % 2) == 0) {
            return "";
        } else {
            return new Handlebars.SafeString("class='alternate'");
        }
    });
    Handlebars.registerHelper("inc", function (value, by, options) {
        return parseInt(value) + by;
    });
    Handlebars.registerHelper('ifCond', function (v1, operator, v2, options) {

        switch (operator) {
            case '==':
                return (v1 == v2) ? options.fn(this) : options.inverse(this);
            case '===':                
                return (v1 === v2) ? options.fn(this) : options.inverse(this);
            case '!=':
                return (v1 != v2) ? options.fn(this) : options.inverse(this);
            case '!==':
                return (v1 !== v2) ? options.fn(this) : options.inverse(this);
            case '<':
                return (v1 < v2) ? options.fn(this) : options.inverse(this);
            case '<=':
                return (v1 <= v2) ? options.fn(this) : options.inverse(this);
            case '>':
                return (v1 > v2) ? options.fn(this) : options.inverse(this);
            case '>=':
                return (v1 >= v2) ? options.fn(this) : options.inverse(this);
            case '&&':
                return (v1 && v2) ? options.fn(this) : options.inverse(this);
            case '||':
                return (v1 || v2) ? options.fn(this) : options.inverse(this);
            default:
                return options.inverse(this);
        }
    });
    Handlebars.registerHelper('colOrder', function (orderby, order, column, options) {
        if(orderby === column) {
            if(order == "desc")
            {
                return "asc";
            }
        }
        return "desc";
    });

    $(function () {

        BuildTable(role, 1, defaultOrder, "ASC");
        $("[js-filter-role]").on("change", function () {
            role = $(this).val();
            BuildTable(role, 1, defaultOrder, "ASC");
        });

        $('body').on('click', '[data-trigger-page]', function (e) {
            e.preventDefault();
            BuildTable(role, $(this).data("trigger-page"), column, direction);
        });

        $('body').on('click', '[js-sortable]', function(e) {
            e.preventDefault();

            column = $(this).data("column");
            direction = $(this).data("direction");            
            BuildTable(role, 1, column, direction);

        });

    });

    function BuildTable(role, page, orderby, order) {
        let data = { action: 'get_users', page, role, orderby, order };
        let url = '/wp-admin/admin-ajax.php';

        $.post(url, data, 'json')
            .done(function (response) {
                let source = $("[js-users]").html();
                let template = Handlebars.compile(source);
                let html = template({ users: response.data.users, order: response.data.order, orderby: response.data.orderby });
                let container = $("[js-users-container]");

                container.empty();
                container.append(html);
                if(response.data.total_count > 0)
                {
                    source = $("[js-pagination]").html();
                    template = Handlebars.compile(source);
                    html = template({ total_count: response.data.total_count, current_page: page, page_count: Math.ceil(response.data.total_count / response.data.page_size) });
                    container = $("[js-pagination-container]");
                    container.empty();
                    container.append(html);                                        
                }
                else {
                    $("[js-pagination-container]").empty();
                }

            })
            .fail(function (xhr, status, error) {
                // error handling
                alert(error);
            });
    }


})();

// handlebar-templates.html

<script type="text/x-handlebars-template" js-users>

    <table class="wp-list-table widefat fixed" cellspacing="0">
        <thead>
            <tr>                
                <th class="manage-column sortable {{colOrder orderby order 'display_name'}}" scope="col" js-sortable data-column="display_name" data-direction="{{colOrder orderby order 'display_name'}}">
                    <a href="#">
                        <span>Name</span>
                        <span class="sorting-indicator"></span>
                    </a>
                </th>
                <th class="manage-column sortable {{colOrder orderby order 'username'}}" scope="col" js-sortable data-column="username" data-direction="{{colOrder orderby order 'username'}}">
                    <a href="#">
                        <span>Username</span>
                        <span class="sorting-indicator"></span> 
                    </a>
                </th>
            </tr>
        </thead>
        {{#each users }}
            <tr {{if_even @index}}>
                <td>                    
                    {{display_name}}
                </td>
                <td>
                    {{user_login}}
                </td>
            </tr>
        {{else}}
            <tr>
                <td colspan="2">                    
                    No users to display.
                </td>
            </tr>
        {{/each}}
    </table>
</script>

<script type="text/x-handlebars-template" js-pagination>
    <div class="tablenav top">
        <div class="tablenav-pages">
            <span class="displaying-num">{{total_count}} items</span>
            <span class="pagination-links">
            {{#ifCond current_page '===' 1}}                
                <span class="tablenav-pages-navspan" aria-hidden="true">«</span>
                <span class="tablenav-pages-navspan" aria-hidden="true">‹</span>
            {{/ifCond}}
            {{#ifCond current_page '>' 1}}
                <a class="prev-page" data-trigger-page="1" href="#"><span class="screen-reader-text">Next page</span><span aria-hidden="true">«</span></a>
                <a class="first-page" data-trigger-page="{{inc current_page -1}}" href="#"><span class="screen-reader-text">Last page</span><span aria-hidden="true">‹</span></a>
            {{/ifCond}}

                <span class="current-page">{{current_page}}</span> of <span class="page-count">{{page_count}}</span>

            {{#ifCond current_page '==' page_count}}
                <span class="tablenav-pages-navspan" aria-hidden="true">›</span>
                <span class="tablenav-pages-navspan" aria-hidden="true">»</span>
            {{/ifCond}}
            {{#ifCond current_page '<' page_count}}
                <a class="next-page" data-trigger-page="{{inc current_page 1}}" href="#"><span class="screen-reader-text">Next page</span><span aria-hidden="true">›</span></a>
                <a class="last-page" data-trigger-page="{{page_count}}" href="#"><span class="screen-reader-text">Last page</span><span aria-hidden="true">»</span></a>
            {{/ifCond}}
            </span>
        </div>
    </div>
</script>
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Browse other questions tagged or ask your own question.