SkyVerge First Interview Code Review

skyverge-interview-code-review-1.php

<?php
/**
 * This code retrieves course data from an external API and displays it in the user's
 * My Account area. A merchant has noticed that there's a delay when loading the page.
 * 
 * 1) What changes would you suggest to reduce or remove that delay?
 * 2) Is there any other code changes that you would make?
 */
public function add_my_courses_section() {
	// MR - Todos 
	// 1. Re-indentation of the code > [done]
	// 2. Code should not have inline styles instead CSS classes and IDs should be added > [done]
	// 3. To remove delay - Synchronous API calls should not be used rather a loader is displayed and When page is loaded, initiate an asynchronous request using Javascript to retrieve and display courses. 
	// 4. GET variable must never be used directly. WordPress offer wrapper function get_query_var() which should be used instead > [done]
	// 5. [Concern] Having a $active_course as CSS class of a button does not make sense to me.
	// 6. [Suggestion] HTML part should be moved to a partial view (if possible) 
	// 7. Dynamic data can not be internationalized. So, there is no need to wrap them using __() > [done]
	// 8. Dynamic data should be wrapped with esc_html() if/when it should not have HTML code > [done]
	// 9. Generic slug for Text domain 'text-domain' should not be used. It should be based off of plugin or theme name
	// 10. [Suggestion] Whole block should be wrapped with a div element (if its already not) having an id > [done] 
	// 11. Dynamic data should be wrapped with esc_url() if it should be a URL > [done]
	// 12. Dynamic data should be wrapped with esc_attr() if it is used as an html attribute > [done]

	$active_course = get_query_var('active_course');

	$api_user_id = get_user_meta( get_current_user_id(), '_external_api_user_id', true );
	if ( ! $api_user_id ) {
		return;
	}
	$courses  = $this->get_api()->get_courses_assigned_to_user( $api_user_id );
	$sso_link = $this->get_api()->get_sso_link( $api_user_id );
	?>
	<div id="my-courses">
		<!-- MR - Inline styles should not be used -->
		<h2 class="my-courses-heading"><?php echo __( 'My Courses', 'text-domain' ); ?></h2>
		<table class="my-courses-table">
			<thead>
				<tr>
					<th><?php echo __( 'Course Code', 'text-domain' ); ?></th>
					<th><?php echo __( 'Course Title', 'text-domain' ); ?></th>
					<th><?php echo __( 'Completion', 'text-domain' ); ?></th>
					<th><?php echo __( 'Date Completed', 'text-domain' ); ?></th>
				</tr>
			</thead>
			<tbody>
			<?php foreach( $courses as $course ) : ?>
				<tr>
					<td><?php echo esc_html( $course['Code'] ); ?></td>
					<td><?php echo esc_html( $course['Name'] ); ?></td>
					<td><?php echo esc_html( $course['PercentageComplete'] ); ?> &#37;</td>
					<td><?php echo esc_html( $course['DateCompleted'] ); ?></td>
				</tr>
			<?php endforeach;	?>
			</tbody>
		</table>
		<p>
			<?php 
				echo sprintf( '<a href="%s" target="_blank" class="button %s">%s</a>', 
								esc_url($sso_link), 
								esc_attr($active_course), 
								__( 'Course Login', 'text-domain' ) 
							);
			?>
		</p>
	</div>

	<?php
}