Skip to content

Commit

Permalink
Refactor notes controller and add ordering params (woocommerce#1878)
Browse files Browse the repository at this point in the history 
* Separate notes controller request args and add collection params

* Fix page param var

* Fix collection params sanitization

* Allow array of note types to be queried

* Add order and orderby collection params to notes controller

* Allow sorting by order and orderby params in notes data store

* Make sure notes are ordered by date and unread matches inbox note types

* Allow status or type params to use an array of items

* Add tests for notes ordering

* Remove unnecessary query arg assignment in the notes data store

* Update get_notes_count to use array for status and type
  • Loading branch information
@joshuatf
joshuatf authored Apr 1, 2019
1 parent de8a1fe commit f93c8e4
Show file tree
Hide file tree
Showing 7 changed files with 149 additions and 57 deletions.
2 changes: 2 additions & 0 deletions client/header/activity-panel/panels/inbox.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,8 @@ export default compose(
page: 1,
per_page: QUERY_DEFAULTS.pageSize,
type: 'info,warning',
orderby: 'date',
order: 'desc',
};

const notes = getNotes( inboxQuery );
Expand Down
3 changes: 3 additions & 0 deletions client/header/activity-panel/unread-indicators.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,9 @@ export function getUnreadNotes( select ) {
const notesQuery = {
page: 1,
per_page: 1,
type: 'info,warning',
orderby: 'date',
order: 'desc',
};

const latestNote = getNotes( notesQuery );
Expand Down
126 changes: 90 additions & 36 deletions includes/api/class-wc-admin-rest-admin-notes-controller.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ public function register_routes() {
'methods' => WP_REST_Server::READABLE,
'callback' => array( $this, 'get_items' ),
'permission_callback' => array( $this, 'get_items_permissions_check' ),
'args' => $this->get_collection_params(),
),
'schema' => array( $this, 'get_public_item_schema' ),
)
Expand Down Expand Up @@ -108,34 +109,9 @@ public function get_item( $request ) {
* @return WP_REST_Response
*/
public function get_items( $request ) {
$per_page = isset( $request['per_page'] ) ? intval( $request['per_page'] ) : 10;
if ( $per_page <= 0 ) {
$per_page = 10;
}

$page = isset( $request['page'] ) ? intval( $request['page'] ) : 1;
if ( $page <= 0 ) {
$page = 1;
}

$args = array(
'per_page' => $per_page,
'page' => $page,
);
$query_args = $this->prepare_objects_query( $request );

$type = isset( $request['type'] ) ? $request['type'] : '';
$type = sanitize_text_field( $type );
if ( ! empty( $type ) ) {
$args['type'] = $type;
}

$status = isset( $request['status'] ) ? $request['status'] : '';
$status = sanitize_text_field( $status );
if ( ! empty( $status ) ) {
$args['status'] = $status;
}

$notes = WC_Admin_Notes::get_notes( 'edit', $args );
$notes = WC_Admin_Notes::get_notes( 'edit', $query_args );

$data = array();
foreach ( (array) $notes as $note_obj ) {
Expand All @@ -145,11 +121,44 @@ public function get_items( $request ) {
}

$response = rest_ensure_response( $data );
$response->header( 'X-WP-Total', WC_Admin_Notes::get_notes_count( $type, $status ) );
$response->header( 'X-WP-Total', WC_Admin_Notes::get_notes_count( $query_args['type'], $query_args['status'] ) );

return $response;
}

/**
* Prepare objects query.
*
* @param WP_REST_Request $request Full details about the request.
* @return array
*/
protected function prepare_objects_query( $request ) {
$args = array();
$args['order'] = $request['order'];
$args['orderby'] = $request['orderby'];
$args['per_page'] = $request['per_page'];
$args['page'] = $request['page'];
$args['type'] = isset( $request['type'] ) ? $request['type'] : array();
$args['status'] = isset( $request['status'] ) ? $request['status'] : array();

if ( 'date' === $args['orderby'] ) {
$args['orderby'] = 'date_created';
}

/**
* Filter the query arguments for a request.
*
* Enables adding extra arguments or setting defaults for a post
* collection request.
*
* @param array $args Key value array of query var to query value.
* @param WP_REST_Request $request The request used.
*/
$args = apply_filters( 'woocommerce_rest_admin_notes_object_query', $args, $request );

return $args;
}

/**
* Check whether a given request has permission to read a single note.
*
Expand Down Expand Up @@ -300,20 +309,65 @@ public function prepare_item_for_response( $data, $request ) {
* @return array
*/
public function get_collection_params() {
$params = array();
$params['context'] = $this->get_context_param( array( 'default' => 'view' ) );
$params['type'] = array(
'description' => __( 'Type of note.', 'woocommerce-admin' ),
$params = array();
$params['context'] = $this->get_context_param( array( 'default' => 'view' ) );
$params['order'] = array(
'description' => __( 'Order sort attribute ascending or descending.', 'woocommerce-admin' ),
'type' => 'string',
'enum' => WC_Admin_Note::get_allowed_types(),
'default' => 'desc',
'enum' => array( 'asc', 'desc' ),
'validate_callback' => 'rest_validate_request_arg',
);
$params['status'] = array(
'description' => __( 'Status of note.', 'woocommerce-admin' ),
$params['orderby'] = array(
'description' => __( 'Sort collection by object attribute.', 'woocommerce-admin' ),
'type' => 'string',
'enum' => WC_Admin_Note::get_allowed_statuses(),
'default' => 'date',
'enum' => array(
'note_id',
'date',
'type',
'title',
'status',
),
'validate_callback' => 'rest_validate_request_arg',
);
$params['page'] = array(
'description' => __( 'Current page of the collection.', 'woocommerce-admin' ),
'type' => 'integer',
'default' => 1,
'sanitize_callback' => 'absint',
'validate_callback' => 'rest_validate_request_arg',
'minimum' => 1,
);
$params['per_page'] = array(
'description' => __( 'Maximum number of items to be returned in result set.', 'woocommerce-admin' ),
'type' => 'integer',
'default' => 10,
'minimum' => 1,
'maximum' => 100,
'sanitize_callback' => 'absint',
'validate_callback' => 'rest_validate_request_arg',
);
$params['type'] = array(
'description' => __( 'Type of note.', 'woocommerce-admin' ),
'type' => 'array',
'sanitize_callback' => 'wp_parse_slug_list',
'validate_callback' => 'rest_validate_request_arg',
'items' => array(
'enum' => WC_Admin_Note::get_allowed_types(),
'type' => 'string',
),
);
$params['status'] = array(
'description' => __( 'Status of note.', 'woocommerce-admin' ),
'type' => 'array',
'sanitize_callback' => 'wp_parse_slug_list',
'validate_callback' => 'rest_validate_request_arg',
'items' => array(
'enum' => WC_Admin_Note::get_allowed_statuses(),
'type' => 'string',
),
);
return $params;
}

Expand Down
31 changes: 13 additions & 18 deletions includes/data-stores/class-wc-admin-notes-data-store.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -250,24 +250,21 @@ private function save_actions( &$note ) {
public function get_notes( $args = array() ) {
global $wpdb;

$per_page = isset( $args['per_page'] ) ? intval( $args['per_page'] ) : 10;
if ( $per_page <= 0 ) {
$per_page = 10;
}

$page = isset( $args['page'] ) ? intval( $args['page'] ) : 1;
if ( $page <= 0 ) {
$page = 1;
}

$offset = $per_page * ( $page - 1 );
$defaults = array(
'per_page' => get_option( 'posts_per_page' ),
'page' => 1,
'order' => 'DESC',
'orderby' => 'date_created',
);
$args = wp_parse_args( $args, $defaults );

$offset = $args['per_page'] * ( $args['page'] - 1 );
$where_clauses = $this->get_notes_where_clauses( $args );

$query = $wpdb->prepare(
"SELECT note_id, title, content FROM {$wpdb->prefix}wc_admin_notes WHERE 1=1{$where_clauses} ORDER BY note_id DESC LIMIT %d, %d",
"SELECT note_id, title, content FROM {$wpdb->prefix}wc_admin_notes WHERE 1=1{$where_clauses} ORDER BY {$args['orderby']} {$args['order']} LIMIT %d, %d",
$offset,
$per_page
$args['per_page']
); // WPCS: unprepared SQL ok.

return $wpdb->get_results( $query ); // phpcs:ignore WordPress.DB.PreparedSQL.NotPrepared
Expand All @@ -280,7 +277,7 @@ public function get_notes( $args = array() ) {
* @param string $status Comma separated list of statuses.
* @return array An array of objects containing a note id.
*/
public function get_notes_count( $type = '', $status = '' ) {
public function get_notes_count( $type = array(), $status = array() ) {
global $wpdb;

$where_clauses = $this->get_notes_where_clauses(
Expand Down Expand Up @@ -308,8 +305,7 @@ public function get_notes_where_clauses( $args = array() ) {
$allowed_types = WC_Admin_Note::get_allowed_types();
$where_type_array = array();
if ( isset( $args['type'] ) ) {
$args_types = explode( ',', $args['type'] );
foreach ( (array) $args_types as $args_type ) {
foreach ( $args['type'] as $args_type ) {
$args_type = trim( $args_type );
if ( in_array( $args_type, $allowed_types, true ) ) {
$where_type_array[] = "'" . esc_sql( $args_type ) . "'";
Expand All @@ -320,8 +316,7 @@ public function get_notes_where_clauses( $args = array() ) {
$allowed_statuses = WC_Admin_Note::get_allowed_statuses();
$where_status_array = array();
if ( isset( $args['status'] ) ) {
$args_statuses = explode( ',', $args['status'] );
foreach ( (array) $args_statuses as $args_status ) {
foreach ( $args['status'] as $args_status ) {
$args_status = trim( $args_status );
if ( in_array( $args_status, $allowed_statuses, true ) ) {
$where_status_array[] = "'" . esc_sql( $args_status ) . "'";
Expand Down
4 changes: 2 additions & 2 deletions includes/notes/class-wc-admin-notes.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ public static function get_note( $note_id ) {
* @param string $status Comma separated list of statuses.
* @return int
*/
public static function get_notes_count( $type = '', $status = '' ) {
public static function get_notes_count( $type = array(), $status = array() ) {
$data_store = WC_Data_Store::load( 'admin-note' );
return $data_store->get_notes_count( $type, $status );
}
Expand All @@ -112,7 +112,7 @@ public static function unsnooze_notes() {
$data_store = WC_Data_Store::load( 'admin-note' );
$raw_notes = $data_store->get_notes(
array(
'status' => WC_Admin_Note::E_WC_ADMIN_NOTE_SNOOZED,
'status' => array( WC_Admin_Note::E_WC_ADMIN_NOTE_SNOOZED ),
)
);
$now = new DateTime();
Expand Down
2 changes: 1 addition & 1 deletion lib/client-assets.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -194,7 +194,7 @@ function wc_admin_print_script_settings() {
'weekdaysShort' => array_values( $wp_locale->weekday_abbrev ),
),
'currentUserData' => $current_user_data,
'alertCount' => WC_Admin_Notes::get_notes_count( 'error,update', 'unactioned' ),
'alertCount' => WC_Admin_Notes::get_notes_count( array( 'error', 'update' ), array( 'unactioned' ) ),
'reviewsEnabled' => get_option( 'woocommerce_enable_reviews' ),
'manageStock' => get_option( 'woocommerce_manage_stock' ),
'commentModeration' => get_option( 'comment_moderation' ),
Expand Down
38 changes: 38 additions & 0 deletions tests/api/admin-notes.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -228,6 +228,44 @@ public function test_note_unsnoozing() {
$this->assertEmpty( $notes );
}

/**
* Test ordering of notes.
*/
public function test_order_notes() {
wp_set_current_user( $this->user );

$request = new WP_REST_Request( 'GET', $this->endpoint );
$request->set_query_params(
array(
'orderby' => 'title',
'order' => 'asc',
)
);
$response = $this->server->dispatch( $request );
$notes = $response->get_data();

$this->assertEquals( 200, $response->get_status() );
$this->assertEquals( 3, count( $notes ) );
$this->assertEquals( $notes[0]['title'], 'PHPUNIT_TEST_NOTE_1_TITLE' );
$this->assertEquals( $notes[1]['title'], 'PHPUNIT_TEST_NOTE_2_TITLE' );
$this->assertEquals( $notes[2]['title'], 'PHPUNIT_TEST_NOTE_3_TITLE' );

$request = new WP_REST_Request( 'GET', $this->endpoint );
$request->set_query_params(
array(
'orderby' => 'status',
'order' => 'desc',
)
);
$response = $this->server->dispatch( $request );
$notes = $response->get_data();

$this->assertEquals( 3, count( $notes ) );
$this->assertEquals( $notes[0]['status'], WC_Admin_Note::E_WC_ADMIN_NOTE_UNACTIONED );
$this->assertEquals( $notes[1]['status'], WC_Admin_Note::E_WC_ADMIN_NOTE_SNOOZED );
$this->assertEquals( $notes[2]['status'], WC_Admin_Note::E_WC_ADMIN_NOTE_ACTIONED );
}

/**
* Test getting lots of notes without permission. It should fail.
*
Expand Down

0 comments on commit f93c8e4

Please sign in to comment.