Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix/phpcs #286

Merged
merged 53 commits into from
Oct 4, 2022
Merged
Show file tree
Hide file tree
Changes from 38 commits
Commits
Show all changes
53 commits
Select commit Hold shift + click to select a range
b184483
Initial PHPCS checks
oscarssanchez Aug 1, 2022
6692501
PHPCBF fixes
oscarssanchez Aug 1, 2022
65171b2
brightcove-video-connect.php phpcs fixes
oscarssanchez Aug 4, 2022
015d546
cli/class-brightcove-cli.php phpcs fixes
oscarssanchez Aug 4, 2022
26a646f
class-bc-admin-media-api.php phpcs fixes
oscarssanchez Aug 4, 2022
3fea880
class-bc-admin-labels-page.php phpcs fixes
oscarssanchez Aug 4, 2022
cf0b1c3
class-bc-admin-menu.php phpcs fixes
oscarssanchez Aug 4, 2022
d7a48b4
class-bc-admin-playlists-page.php phpcs fixes
oscarssanchez Aug 4, 2022
3e70d61
class-bc-admin-settings-page.php
oscarssanchez Aug 4, 2022
3c7f09c
class-bc-admin-sources.php phpcs fixes
oscarssanchez Aug 4, 2022
53ca539
class-bc-admin-user-profile.php phpcs fixes
oscarssanchez Aug 4, 2022
5943efe
class-bc-admin-videos-page.php phpcs fixes
oscarssanchez Aug 4, 2022
d29cde5
class-bc-templates.php phpcs fixes
oscarssanchez Aug 4, 2022
feae4b8
class-bc-api.php phpcs fixes
oscarssanchez Aug 4, 2022
051658b
class-bc-cms-api.php phpcs fixes
oscarssanchez Aug 4, 2022
594c612
class-bc-experiences-api.php phpcs fixes
oscarssanchez Aug 4, 2022
31e1a98
includes/api/class-bc-oauth.php phpcs fixes
oscarssanchez Aug 4, 2022
b2976a1
includes/api/class-bc-player-management-api.php phpcs fixes
oscarssanchez Aug 4, 2022
898975c
includes/api/class-bc-player-management-api2.php phpcs fixes
oscarssanchez Aug 4, 2022
e022ca6
includes/api/class-bc-text-track.php phpcs fixes
oscarssanchez Aug 4, 2022
dad8c04
includes/class-bc-accounts.php phpcs fixes
oscarssanchez Aug 4, 2022
67da919
includes/class-bc-errors.php phpcs fixes
oscarssanchez Aug 4, 2022
df0fd2b
includes/class-bc-experiences-shortcode.php phpcs fixes
oscarssanchez Aug 4, 2022
9808c30
includes/class-bc-in-page-experience-shortcode.php phpcs fixes
oscarssanchez Aug 4, 2022
75d141b
includes/class-bc-labels.php phpcs fixes
oscarssanchez Aug 4, 2022
b196c2e
includes/class-bc-logging.php phpcs fixes
oscarssanchez Aug 4, 2022
e662a3f
includes/class-bc-notification-api.php phpcs fixes
oscarssanchez Aug 4, 2022
0004fcd
includes/class-bc-permissions.php phpcs fixes
oscarssanchez Aug 4, 2022
3888ac5
includes/class-bc-playlist-shortcode.php phpcs fixes
oscarssanchez Aug 4, 2022
e7bf58f
includes/class-bc-setup.php phpcs fixes
oscarssanchez Aug 4, 2022
ef7f50c
includes/class-bc-tags.php phpcs fixes
oscarssanchez Aug 4, 2022
40352ad
includes/class-bc-utility.php phpcs fixes
oscarssanchez Aug 4, 2022
39dc9d9
includes/class-bc-video-shortcode.php phpcs fixes
oscarssanchez Aug 4, 2022
f6ca93b
includes/class-bc-video-upload.php phpcs fixes
oscarssanchez Aug 4, 2022
1728cf2
includes/sync/class-bc-playlists.php phpcs fixes
oscarssanchez Aug 4, 2022
295fc2c
includes/sync/class-bc-videos.php phpcs fixes
oscarssanchez Aug 4, 2022
d01be57
uninstall.php phpcs fixes
oscarssanchez Aug 4, 2022
456df46
Merge branch 'develop' into fix/phpcs
oscarssanchez Aug 4, 2022
82b69f1
Fixes activation error
oscarssanchez Sep 5, 2022
552364b
Fixes bug with old folder ID update
oscarssanchez Sep 5, 2022
db2d1d9
Bump php lint version used
oscarssanchez Sep 21, 2022
db94cfc
Remove unused code
oscarssanchez Sep 21, 2022
1069e77
Align descriptions
oscarssanchez Sep 21, 2022
9158805
Align descriptions
oscarssanchez Sep 21, 2022
ff0df76
Deprecate actions
oscarssanchez Sep 22, 2022
5234eb4
Align docs
oscarssanchez Sep 22, 2022
ae933bc
Specify phpcs rule
oscarssanchez Sep 22, 2022
ab24ae0
Align docs
oscarssanchez Sep 23, 2022
af798f7
Add comma
oscarssanchez Sep 23, 2022
8b6cce4
Remove unused code
oscarssanchez Sep 23, 2022
b261726
Add phpcs rule
oscarssanchez Sep 23, 2022
31b09b8
Deprecate method
oscarssanchez Oct 3, 2022
454cb98
PHPCS fixes
oscarssanchez Oct 3, 2022
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 21 additions & 1 deletion .github/workflows/lint.yml
Original file line number Diff line number Diff line change
Expand Up @@ -22,4 +22,24 @@ jobs:
- name: npm install
run: npm install
- name: eslint
run: npm run lint-js
run: npm run lint-js

phpcs:
name: PHP Lint
runs-on: ubuntu-latest

steps:
- name: Checkout
uses: actions/checkout@v3

- name: PHP version
uses: shivammathur/setup-php@v2
with:
php-version: '7.2'
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@oscarssanchez I imagine you are not using the default PHP 8.1 because it is still incompatible with the codebase. Can we bump this to PHP 7.4 at least?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bumped here: db2d1d9

coverage: none

- name: composer install
run: composer install

- name: PHPCS check
run: composer run lint
6 changes: 3 additions & 3 deletions brightcove-video-connect.php
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@
* License: GPLv2+
* Text Domain: brightcove
* Domain Path: /languages
*
* @package Brightcove_Video_Connect
*/

/**
Expand Down Expand Up @@ -72,7 +74,7 @@ function brightcove_deactivate() {

// Wireup actions.
if ( is_admin() ) {
add_action( 'admin_notices', array( 'BC_Setup', 'bc_admin_notices' ) );
add_action( 'admin_notices', array( 'BC_Setup', 'bc_admin_notices' ) );
}

add_action( 'init', array( 'BC_Setup', 'action_init' ) );
Expand All @@ -83,8 +85,6 @@ function brightcove_deactivate() {
add_action( 'init', array( 'BC_Setup', 'action_init_all' ), 9 ); // Ensures the menu is loaded on all pages.
add_action( 'init', array( 'BC_Notification_API', 'setup' ), 9 );

// add_action( 'brightcove_upgrade', array( 'BC_Notification_API', 'maybe_backport_subscriptions' ) ); // @TODO Verify API as errors don't seem to match the documentation

if ( ! defined( 'WPCOM_IS_VIP_ENV' ) || ! WPCOM_IS_VIP_ENV ) {

// Activation / Deactivation.
Expand Down
5 changes: 5 additions & 0 deletions cli/class-brightcove-cli.php
Original file line number Diff line number Diff line change
@@ -1,4 +1,9 @@
<?php
/**
* Brightcove WP CLI commands file.
*
* @package Brightcove_Video_Connect
*/

/**
* Implements a set of commands to interact with the Brightcove Video Connect plugin
Expand Down
103 changes: 67 additions & 36 deletions includes/admin/api/class-bc-admin-media-api.php
Original file line number Diff line number Diff line change
@@ -1,37 +1,60 @@
<?php

/**
* BC_Admin_Media_API class file.
*
* @package Brightcove_Video_Connect
*/

/**
* BC_Admin_Media_API class.
*/
class BC_Admin_Media_API {

/**
* BC_CMS_API object
*
* @var BC_CMS_API
*/
protected $cms_api;

/**
* BC_Player_Management_API object
*
* @var BC_Player_Management_API
*/
protected $player_api;

/**
* BC_Experiences_API object
*
* @var BC_Experiences_API
*/
protected $experiences_api;

/**
* BC_Playlists object
*
* @var BC_Playlists
*/
protected $playlists;

/**
* BC_Video_Upload object
*
* @var BC_Video_Upload
*/
protected $video_upload;

/**
* BC_Videos Object
*
* @var BC_Videos
*/
protected $videos;

/**
* Constructor method
*/
public function __construct() {

global $bc_accounts;
Expand Down Expand Up @@ -62,6 +85,9 @@ public function __construct() {
add_action( 'wp_ajax_bc_resolve_shortcode', array( $this, 'resolve_shortcode' ) );
}

/**
* Sends shortcode over.
*/
public function resolve_shortcode() {

$video_id = ( ! empty( $_POST['video_id'] ) ) ? sanitize_text_field( wp_unslash( $_POST['video_id'] ) ) : 0;
Expand All @@ -75,6 +101,9 @@ public function resolve_shortcode() {
wp_send_json_success( do_shortcode( $shortcode ) );
}

/**
* Helper function for validating Ajax requests
*/
protected function bc_helper_check_ajax() {

if ( ! defined( 'DOING_AJAX' ) || ! DOING_AJAX || ! isset( $_POST['nonce'] ) ) {
Expand Down Expand Up @@ -167,7 +196,7 @@ public function bc_ajax_update_video_or_playlist() {
$fields = $this->cms_api->video_fields();
$use_history = false;
foreach ( $fields['custom_fields'] as $item ) {
if ( '_change_history' == $item['id'] ) {
if ( '_change_history' === $item['id'] ) {
$use_history = true;
break;
}
Expand All @@ -187,10 +216,10 @@ public function bc_ajax_update_video_or_playlist() {
$user = wp_get_current_user();
$history[] = array(
'user' => $user->user_login,
'time' => date( 'Y-m-d H:i:s', time() ),
'time' => gmdate( 'Y-m-d H:i:s', time() ),
);

$custom['_change_history'] = json_encode( $history );
$custom['_change_history'] = wp_json_encode( $history );
}

$updated_data['custom_fields'] = $custom;
Expand All @@ -200,7 +229,7 @@ public function bc_ajax_update_video_or_playlist() {

$status = false;

if ( ! in_array( $_POST['type'], array( 'playlists', 'videos' ) ) ) {
if ( ! in_array( $_POST['type'], array( 'playlists', 'videos' ), true ) ) {
wp_send_json_error( __( 'Type is not specified', 'brightcove' ) );
}

Expand All @@ -225,11 +254,11 @@ public function bc_ajax_update_video_or_playlist() {
} else {
$status = $this->videos->update_bc_video( $updated_data );

if ( isset( $_POST['folderId'] ) && isset( $_POST['oldFolderId'] ) ) {
$folderId = sanitize_text_field( $_POST['folderId'] );
$oldFolderId = sanitize_text_field( $_POST['oldFolderId'] );
if ( isset( $_POST['folder_id'] ) && isset( $_POST['oldfolder_id'] ) ) {
$folder_id = sanitize_text_field( $_POST['folder_id'] );
$old_folder_id = sanitize_text_field( $_POST['oldfolder_id'] );
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@oscarssanchez I am probably missing it but did you also change the place where oldFolderId is sent in the POST request?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are right @felipeelia , I've fixed it here: 552364b


$this->cms_api->add_folder_to_video( $oldFolderId, $folderId, $updated_data['video_id'] );
$this->cms_api->add_folder_to_video( $old_folder_id, $folder_id, $updated_data['video_id'] );
}

// Maybe update poster
Expand Down Expand Up @@ -302,7 +331,7 @@ public function bc_ajax_delete_video_or_playlist() {
$id = BC_Utility::sanitize_id( $_POST['id'] );

// Get type brightcove-playlist or brightcove-video.
if ( ! in_array( $type, array( 'playlists', 'videos' ) ) ) {
if ( ! in_array( $type, array( 'playlists', 'videos' ), true ) ) {
wp_send_json_error( esc_html__( 'Type is not specified!', 'brightcove' ) );
}

Expand Down Expand Up @@ -458,7 +487,9 @@ public function fetch_all( $type, $posts_per_page = 100, $page = 1, $query_strin

$account_ids = array_unique( $account_ids );

while ( count( $results ) <= ( $page * $posts_per_page ) ) {
$count_results = count( $results );

while ( $count_results <= ( $page * $posts_per_page ) ) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As $results is changed inside the while, are you sure this will keep the same behavior it had before?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This code is actually not used anymore, so I just removed the whole method db94cfc


if ( 0 === count( $account_ids ) ) {

Expand Down Expand Up @@ -548,7 +579,7 @@ public function brightcove_media_query() {
$query = ( isset( $_POST['search'] ) && '' !== $_POST['search'] ) ? sanitize_text_field( $_POST['search'] ) : false;
$tag_name = ( isset( $_POST['tagName'] ) && '' !== $_POST['tagName'] ) ? sanitize_text_field( $_POST['tagName'] ) : false;
$dates = ( isset( $_POST['dates'] ) && 'all' !== $_POST['dates'] ) ? BC_Utility::sanitize_date( $_POST['dates'] ) : false;
$folder_id = ( isset( $_POST['folderId'] ) && '' !== $_POST['folderId'] ) ? sanitize_text_field( $_POST['folderId'] ) : false;
$folder_id = ( isset( $_POST['folder_id'] ) && '' !== $_POST['folder_id'] ) ? sanitize_text_field( $_POST['folder_id'] ) : false;
Comment on lines -551 to +503
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as before: did we also change the place where that POST field is sent?

$label = ( isset( $_POST['labelPath'] ) && '' !== $_POST['labelPath'] ) ? sanitize_text_field( $_POST['labelPath'] ) : false;
$state = ( isset( $_POST['state'] ) && '' !== $_POST['state'] ) ? sanitize_text_field( $_POST['state'] ) : apply_filters( 'brightcove_state_filter', false );

Expand All @@ -565,7 +596,7 @@ public function brightcove_media_query() {

$type = isset( $_POST['type'] ) ? sanitize_key( $_POST['type'] ) : false;

if ( ! $type || ! in_array( $type, array( 'videos', 'playlists', 'videoexperience', 'playlistexperience', 'inpageexperiences' ) ) ) {
if ( ! $type || ! in_array( $type, array( 'videos', 'playlists', 'videoexperience', 'playlistexperience', 'inpageexperiences' ), true ) ) {

wp_send_json_error( esc_html__( 'Invalid Search Type', 'brightcove' ) );
exit; // Type can only be videos or playlists.
Expand Down Expand Up @@ -742,7 +773,7 @@ public function brightcove_media_query() {
if ( isset( $result['custom_fields'] ) ) {
foreach ( $result['custom_fields'] as $id => $value ) {
// Extract the change tracking item explicitly
if ( $id == '_change_history' ) {
if ( '_change_history' === $id ) {
$result['history'] = $value;
continue;
}
Expand Down Expand Up @@ -846,11 +877,11 @@ public function ajax_players() {
*
* @global BC_Accounts $bc_accounts
*
* @param string $account_hash
* @param int $video_id
* @param string $url
* @param int $width
* @param int $height
* @param string $account_hash The account hash for the account.
* @param int $video_id The ID of the video to associate the image with.
* @param string $url The URL of the image to upload.
* @param int $width The width of the image.
* @param int $height The height of the image.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we align these descriptions? :)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed here 1069e77

*/
public function ajax_poster_upload( $account_hash, $video_id, $url, $width, $height ) {
global $bc_accounts;
Expand Down Expand Up @@ -887,11 +918,11 @@ public function ajax_poster_upload( $account_hash, $video_id, $url, $width, $hei
*
* @global BC_Accounts $bc_accounts
*
* @param string $account_hash
* @param int $video_id
* @param string $url
* @param int $width
* @param int $height
* @param string $account_hash The account hash for the account.
* @param int $video_id The video ID.
* @param string $url The URL of the thumbnail attachment.
* @param int $width The width of the thumbnail.
* @param int $height The height of the thumbnail.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here. It is possible that GitHub preview is showing it wrong but it seems some spaces are lacking to make them all start in the same column.

*/
public function ajax_thumb_upload( $account_hash, $video_id, $url, $width, $height ) {
global $bc_accounts;
Expand Down Expand Up @@ -928,8 +959,8 @@ public function ajax_thumb_upload( $account_hash, $video_id, $url, $width, $heig
*
* @global BC_Accounts $bc_accounts
*
* @param string $account_hash
* @param int $video_id
* @param string $account_hash The account hash to which the video belongs.
* @param int $video_id The ID of the video from which to delete captions.
*/
public function ajax_caption_delete( $account_hash, $video_id ) {
global $bc_accounts;
Expand All @@ -953,9 +984,9 @@ public function ajax_caption_delete( $account_hash, $video_id ) {
*
* @global BC_Accounts $bc_accounts
*
* @param string $account_hash
* @param int $video_id
* @param array $raw_captions
* @param string $account_hash The account hash for the account.
* @param int $video_id The video ID
* @param array $raw_captions The raw captions data.
*/
public function ajax_caption_upload( $account_hash, $video_id, $raw_captions ) {
global $bc_accounts;
Expand Down Expand Up @@ -986,7 +1017,7 @@ public function ajax_caption_upload( $account_hash, $video_id, $raw_captions ) {
$label = isset( $caption['label'] ) ? sanitize_text_field( $caption['label'] ) : '';
$default = ( isset( $caption['default'] ) && 'checked' === $caption['default'] );

$source = parse_url( $caption['source'] );
$source = wp_parse_url( $caption['source'] );
if ( 0 === strpos( $source['host'], 'brightcove' ) ) {
// If the hostname starts with "brightcove," assume this media has already been ingested and add to old captions.
$old_captions[] = new BC_Text_Track( $url, $lang, 'captions', $label, $default );
Expand Down Expand Up @@ -1014,8 +1045,8 @@ public function ajax_caption_upload( $account_hash, $video_id, $raw_captions ) {
/**
* Return a set of the most recent videos for the specified account.
*
* @param string $account_id
* @param int $count
* @param string $account_id The account ID to retrieve videos for.
* @param int $count The number of videos to return.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same comment about alignment here too.

*
* @global BC_Accounts $bc_accounts
*
Expand Down Expand Up @@ -1058,7 +1089,7 @@ protected function fetch_videos( $account_id, $count = 10 ) {

foreach ( $result['custom_fields'] as $id => $value ) {
// Extract the change tracking item explicitly
if ( $id == '_change_history' ) {
if ( '_change_history' === $id ) {
$result['history'] = $value;
continue;
}
Expand Down Expand Up @@ -1098,8 +1129,8 @@ protected function fetch_videos( $account_id, $count = 10 ) {
/**
* When a WP heartbeat is received with an account hash, respond with the most recent 10 videos available.
*
* @param array $response
* @param array $data
* @param array $response WP heartbeat response.
* @param array $data Data received from heartbeat
*
* @return array
*/
Expand Down Expand Up @@ -1127,7 +1158,7 @@ public function add_in_process_videos( $videos ) {

foreach ( $video_post_ids as $video_post_id ) {
$in_process_video_id = BC_Utility::get_sanitized_video_id( $video_post_id );
if ( in_array( $in_process_video_id, $video_ids ) ) {
if ( in_array( $in_process_video_id, $video_ids, true ) ) {
wp_delete_post( $video_post_id, true );
} else {
$videos[] = get_post_meta( $video_post_id, '_brightcove_video_object', true );
Expand Down
18 changes: 15 additions & 3 deletions includes/admin/class-bc-admin-labels-page.php
Original file line number Diff line number Diff line change
@@ -1,10 +1,22 @@
<?php
/**
* BC_Admin_Labels_Page class.
*
* @package Brightcove_Video_Connect
*/

/**
* The Labels Page class.
*
* Class BC_Admin_Labels_Page
*/
class BC_Admin_Labels_Page {

/**
* BC_Labels object
*
* @var BC_Labels
*/
protected $bc_labels;

/**
Expand Down Expand Up @@ -34,13 +46,13 @@ public function render_edit_label_page() {
<form method="post" action="<?php echo esc_url( admin_url( 'admin-post.php' ) ); ?>" class="validate">
<?php wp_nonce_field( 'brightcove-edit-label', 'brightcove-edit-label-nonce' ); ?>
<input type="hidden" name="action" value="brightcove-edit-label">
<input type="hidden" name="label-path" value="<?php echo ! empty( $_GET['update_label'] ) ? esc_attr( $_GET['update_label'] ) : ''; ?>">
<input type="hidden" name="label-path" value="<?php echo ! empty( $_GET['update_label'] ) ? esc_attr( $_GET['update_label'] ) : ''; // phpcs:ignore ?>">
<table class="form-table">
<tbody>
<tr class="form-field form-required term-name-wrap">
<th scope="row"><label for="name">Label</label></th>
<td>
<input name="label-update" id="name" type="text" value="<?php echo esc_attr( end( array_filter( explode( '/', $_GET['update_label'] ) ) ) ); ?>" size="40" aria-required="true">
<input name="label-update" id="name" type="text" value="<?php echo esc_attr( end( array_filter( explode( '/', $_GET['update_label'] ) ) ) ); ?>" size="40" aria-required="true"> <?php // phpcs:ignore ?>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd recommend ignoring the specific rule here with // phpcs:ignore WordPress.Security.NonceVerification. That can save some time in the future if any of those other functions being called lacks a parameter or something.

<p class="description"><?php esc_html_e( 'Enter the new label name.', 'brightcove' ); ?></p>
</td>
</tr>
Expand All @@ -58,7 +70,7 @@ public function render_edit_label_page() {
* Generates an HTML table with all configured sources
*/
public function render_labels_page() {
$maybe_refresh = isset( $_GET['refresh_labels'] ) ? (bool) $_GET['refresh_labels'] : false;
$maybe_refresh = isset( $_GET['refresh_labels'] ) && (bool) $_GET['refresh_labels']; // phpcs:ignore
$labels = $this->bc_labels->fetch_all( $maybe_refresh );
?>
<div class="wrap">
Expand Down
Loading