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>