feat: Implement admin dashboard api metrics. (PR-3) #2796
Conversation
# Conflicts: # includes/DependencyManagement/Providers/ServiceProvider.php
…dminDashboardController
…e data retrieval logic
…endor count by date range
…onal order statuses
…and enhance data retrieval with filters
… model and streamline data retrieval
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughThis change introduces a new Changes
Sequence Diagram(s)sequenceDiagram
participant Admin as Admin User
participant REST as AdminDashboardStatsController (REST API)
participant Model as AdminDashboardStats / VendorOrderStats
participant Store as AdminDashboardStatsStore / VendorOrderStatsStore
participant DB as Database/WooCommerce
Admin->>REST: Sends request to /dokan/v1/admin/dashboard/{endpoint}
REST->>Model: Calls static method for stats (e.g., get_customer_metrics)
Model->>Store: Delegates data retrieval (e.g., get_customer_metrics)
Store->>DB: Queries database/WooCommerce for metrics
DB-->>Store: Returns data
Store-->>Model: Returns processed data
Model-->>REST: Returns stats array
REST-->>Admin: Responds with WP_REST_Response
Assessment against linked issues
Assessment against linked issues: Out-of-scope changes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
✨ Finishing Touches🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
🔭 Outside diff range comments (9)
includes/Utilities/ReportUtil.php (1)
83-84: Fix the missing newline at end of file.The pipeline failure indicates a missing newline at the end of the file, which violates PHP coding standards.
} +includes/DependencyManagement/Providers/ModelServiceProvider.php (1)
40-41: Fix the missing newline at end of file.The pipeline failure indicates a missing newline at the end of the file, which violates PHP coding standards.
} +includes/DependencyManagement/Providers/ServiceProvider.php (1)
98-99: Fix the missing newline at end of file.The pipeline failure indicates a missing newline at the end of the file, which violates PHP coding standards.
} +includes/REST/Manager.php (1)
217-218: Fix the missing newline at end of file.The pipeline failure indicates a missing newline at the end of the file, which violates PHP coding standards.
} +includes/Models/AdminDashboardStats.php (1)
140-141: Add missing newline at end of file.} } +includes/Models/DataStore/VendorOrderStatsStore.php (1)
164-165: Add missing newline at end of file.} } +includes/Models/DataStore/AdminDashboardStatsStore.php (1)
384-385: Add missing newline at end of file.} } +includes/Models/VendorOrderStats.php (1)
423-424: Add missing newline at end of file.} } +includes/REST/AdminDashboardController.php (1)
659-660: Add missing newline at end of file.} } +
🧹 Nitpick comments (5)
includes/Utilities/ReportUtil.php (1)
59-82: Review the sanitization approach and ensure proper error handling.The method implementation looks generally correct, but consider these improvements:
- The use of
esc_sqlinarray_mapmight be overkill since these are order statuses, not SQL queries. Consider usingsanitize_text_fieldinstead.- The method doesn't handle potential null/empty returns from
WC_Admin_Settings::get_option().- $excluded_statuses = array_merge( - [ 'auto-draft', 'trash' ], - array_map( 'esc_sql', $excluded_statuses ) - ); + $excluded_statuses = array_merge( + [ 'auto-draft', 'trash' ], + array_map( 'sanitize_text_field', $excluded_statuses ) + );includes/Models/AdminDashboardStats.php (2)
28-33: Consider if the$idparameter is necessary for this stats model.The constructor accepts an
$idparameter but this appears to be a stats aggregation model that doesn't represent a single database entity. The ID parameter seems unused and potentially confusing.- /** - * Constructor. - * - * @since DOKAN_SINCE - * - * @param int $id ID to load from the DB (optional) or an AdminDashboardStats object. - */ - public function __construct( int $id = 0 ) { + /** + * Constructor. + * + * @since DOKAN_SINCE + */ + public function __construct() { $this->data_store = new AdminDashboardStatsStore(); - parent::__construct( $id ); + parent::__construct( 0 ); }
45-47: Consider implementing a singleton pattern or caching for efficiency.Each static method creates a new instance of the class and its data store. For better performance, consider implementing a singleton pattern or caching the instance.
class AdminDashboardStats extends BaseModel { + /** + * Singleton instance + * + * @var AdminDashboardStats|null + */ + private static $instance = null; + + /** + * Get singleton instance + * + * @return AdminDashboardStats + */ + private static function get_instance(): AdminDashboardStats { + if ( null === self::$instance ) { + self::$instance = new static(); + } + return self::$instance; + } + /** * This is the name of this object type.Then update the static methods to use the singleton:
public static function get_customer_metrics( string $start_date, string $end_date ): array { - return ( new static() )->get_data_store()->get_customer_metrics( $start_date, $end_date ); + return self::get_instance()->get_data_store()->get_customer_metrics( $start_date, $end_date ); }Also applies to: 58-60, 71-73, 84-86, 97-99, 110-112, 124-126, 137-139
includes/Models/DataStore/AdminDashboardStatsStore.php (1)
298-298: Fix variable alignment.// Use VendorOrderStatsStore for dokan_order_stats queries - $current_count = $this->vendor_order_stats_store->get_active_vendors_count( + $current_count = $this->vendor_order_stats_store->get_active_vendors_count( $date_range['current_month_start'],includes/REST/AdminDashboardController.php (1)
143-145: Consolidate date parameter sanitization.The date parameter handling is inconsistent across methods. Some use ternary operators with
sanitize_text_field, others use null coalescing without sanitization.- $date = $request->get_param( 'date' ) ? sanitize_text_field( $request->get_param( 'date' ) ) : date( 'Y-m' ); + $date = sanitize_text_field( $request->get_param( 'date' ) ?? date( 'Y-m' ) );Apply the same pattern to lines 223-224:
- $date = $request->get_param( 'date' ) ?? date( 'Y-m' ); + $date = sanitize_text_field( $request->get_param( 'date' ) ?? date( 'Y-m' ) );Also applies to: 223-224, 239-240, 255-256
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
includes/DependencyManagement/Providers/AdminDashboardServiceProvider.php(1 hunks)includes/DependencyManagement/Providers/ModelServiceProvider.php(1 hunks)includes/DependencyManagement/Providers/ServiceProvider.php(1 hunks)includes/Models/AdminDashboardStats.php(1 hunks)includes/Models/DataStore/AdminDashboardStatsStore.php(1 hunks)includes/Models/DataStore/VendorOrderStatsStore.php(1 hunks)includes/Models/VendorOrderStats.php(1 hunks)includes/REST/AdminDashboardController.php(1 hunks)includes/REST/Manager.php(1 hunks)includes/Utilities/ReportUtil.php(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
includes/Utilities/ReportUtil.php (1)
includes/Abstracts/Settings.php (1)
get_option(84-99)
🪛 GitHub Actions: Inspections
includes/DependencyManagement/Providers/ServiceProvider.php
[error] 1-1: PHP CodeSniffer: Expected 1 newline at end of file; 0 found.
includes/DependencyManagement/Providers/ModelServiceProvider.php
[error] 1-1: PHP CodeSniffer: Expected 1 newline at end of file; 0 found.
includes/REST/Manager.php
[error] 1-1: PHP CodeSniffer: Expected 1 newline at end of file; 0 found.
includes/Utilities/ReportUtil.php
[error] 1-1: PHP CodeSniffer: Expected 1 newline at end of file; 0 found.
includes/Models/AdminDashboardStats.php
[error] 1-1: PHP CodeSniffer: Expected 1 newline at end of file; 0 found.
includes/DependencyManagement/Providers/AdminDashboardServiceProvider.php
[error] 1-1: PHP CodeSniffer: Expected 1 newline at end of file; 0 found.
includes/Models/DataStore/VendorOrderStatsStore.php
[error] 1-1: PHP CodeSniffer: Expected 1 newline at end of file; 0 found.
includes/Models/DataStore/AdminDashboardStatsStore.php
[error] 1-1: PHP CodeSniffer: Expected 1 newline at end of file; 0 found.
includes/Models/VendorOrderStats.php
[error] 1-1: PHP CodeSniffer: Expected 1 newline at end of file; 0 found.
includes/REST/AdminDashboardController.php
[error] 1-1: PHP CodeSniffer: Expected 1 newline at end of file; 0 found.
🪛 GitHub Check: Run PHPCS inspection
includes/DependencyManagement/Providers/AdminDashboardServiceProvider.php
[failure] 43-43:
Expected 1 newline at end of file; 0 found
includes/Models/DataStore/VendorOrderStatsStore.php
[warning] 117-117:
Use placeholders and $wpdb->prepare(); found get_query_statement
[warning] 117-117:
Use placeholders and $wpdb->prepare(); found $this
includes/Models/DataStore/AdminDashboardStatsStore.php
[warning] 298-298:
Equals sign not aligned correctly; expected 1 space but found 2 spaces
[failure] 161-161:
Closing parenthesis of a multi-line function call must be on a line by itself
[failure] 157-157:
Whitespace found at end of line
[failure] 157-157:
Opening parenthesis of a multi-line function call must be the last content on the line
[failure] 156-156:
Closing parenthesis of a multi-line function call must be on a line by itself
[failure] 152-152:
Whitespace found at end of line
[failure] 152-152:
Opening parenthesis of a multi-line function call must be the last content on the line
[failure] 122-122:
Functions must not contain multiple empty lines in a row; found 2 empty lines
[failure] 121-121:
Closing parenthesis of a multi-line function call must be on a line by itself
[warning] 117-117:
Use placeholders and $wpdb->prepare(); found $exclude_order_statuses
[failure] 111-111:
Opening parenthesis of a multi-line function call must be the last content on the line
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: e2e tests (2, 3)
- GitHub Check: api tests (1, 1)
- GitHub Check: e2e tests (3, 3)
- GitHub Check: e2e tests (1, 3)
🔇 Additional comments (9)
includes/DependencyManagement/Providers/ModelServiceProvider.php (1)
15-17: LGTM! New vendor order statistics services properly registered.The addition of
VendorOrderStatsandVendorOrderStatsStoreservices to the dependency injection container is correctly implemented and aligns with the PR objectives.includes/DependencyManagement/Providers/ServiceProvider.php (1)
65-67: LGTM! Indentation formatting improvements.The indentation changes for
UtilsServiceProviderandIntelligenceServiceProviderimprove code consistency.includes/REST/Manager.php (1)
186-212: LGTM! Improved formatting and alignment.The formatting improvements to the REST API class map enhance code readability and maintainability.
includes/DependencyManagement/Providers/AdminDashboardServiceProvider.php (1)
5-42: LGTM! Clean refactoring to single service architecture.The refactoring from multiple services to a single
AdminDashboardControllerservice is a good architectural improvement that:
- Simplifies the service provider
- Uses a more descriptive tag name (
admin-dashboard-api-service)- Improves maintainability
The implementation correctly uses the base service provider methods and maintains proper documentation.
includes/Models/DataStore/VendorOrderStatsStore.php (2)
74-75: Excluded order statuses are safely sourced and escapedReportUtil::get_exclude_order_statuses() pulls values from WooCommerce settings (defaulting to pending, failed, cancelled), applies array_map('esc_sql', …) both before and after the
woocommerce_analytics_excluded_order_statuses/dokan_analytics_excluded_order_statusesfilters, and then injects them into theNOT INclause wrapped in quotes. Since every status is passed through esc_sql, there is no SQL‐injection risk here.
117-120: Fix incorrect null coalescing operator usage.The null coalescing operator
??on line 119 will return an empty array even if$vendorsisfalse(whichget_resultscan return on error). This masks database errors.- $vendors = $wpdb->get_results( $this->get_query_statement(), ARRAY_A ); - - return $vendors ?? []; + $vendors = $wpdb->get_results( $this->get_query_statement(), ARRAY_A ); + + return is_array( $vendors ) ? $vendors : [];Likely an incorrect or invalid review comment.
includes/Models/VendorOrderStats.php (1)
12-57: Well-structured model implementation!The model follows best practices with proper data encapsulation, type hints, and clear getter/setter methods. The use of filters for the data store is a nice touch for extensibility.
includes/REST/AdminDashboardController.php (2)
32-120: Excellent REST API design with well-structured endpoints!The refactored controller provides a clean separation of concerns with individual endpoints for each dashboard section. Good use of consistent naming conventions and proper permission callbacks.
613-637: Robust date parsing implementation!The
parse_date_rangemethod includes proper validation and handles edge cases well. The month range validation (line 621) prevents invalid month values.
includes/DependencyManagement/Providers/AdminDashboardServiceProvider.php
Outdated
Show resolved
Hide resolved
…-dashboard-metrics-api # Conflicts: # includes/Models/DataStore/VendorOrderStatsStore.php # includes/REST/AdminDashboardController.php
…-dashboard-metrics-api
…type filtering in Admin Dashboard
…-dashboard-metrics-api # Conflicts: # includes/Models/DataStore/VendorOrderStatsStore.php
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (5)
includes/Models/DataStore/VendorOrderStatsStore.php (1)
80-81: Critical: SQL injection vulnerability with order status exclusions.The
$exclude_order_statusesarray values are directly concatenated into SQL queries without proper escaping. This could lead to SQL injection if the values are not properly sanitized.- $this->add_sql_clause( 'where', " AND wos.status NOT IN ( '" . implode( "','", $exclude_order_statuses ) . "' )" ); + $placeholders = array_fill( 0, count( $exclude_order_statuses ), '%s' ); + $this->add_sql_clause( 'where', $wpdb->prepare( " AND wos.status NOT IN ( " . implode( ',', $placeholders ) . " )", $exclude_order_statuses ) );includes/Models/DataStore/AdminDashboardStatsStore.php (4)
122-123: Remove extra empty line.) ); - $query_statement = $this->get_query_statement();
152-161: Fix multi-line function call formatting.The opening parenthesis should be on the same line as the function name, and closing parenthesis should be on its own line.
$this->clear_all_clauses(); - $this->add_sql_clause( 'select', $wpdb->prepare( + $this->add_sql_clause( 'select', $wpdb->prepare( 'SUM(CASE WHEN u.user_registered BETWEEN %s AND %s THEN 1 ELSE 0 END) as current_count,', $date_range['current_month_start'], $date_range['current_month_end'] - ) ); - $this->add_sql_clause( 'select', $wpdb->prepare( + ) ); + $this->add_sql_clause( 'select', $wpdb->prepare( 'SUM(CASE WHEN u.user_registered BETWEEN %s AND %s THEN 1 ELSE 0 END) as previous_count', $date_range['previous_month_start'], $date_range['previous_month_end'] - ) ); + ) );
197-204: Fix multi-line function call formatting.$this->clear_all_clauses(); - $this->add_sql_clause( 'select', $wpdb->prepare( + $this->add_sql_clause( 'select', $wpdb->prepare( 'CASE WHEN post_date BETWEEN %s AND %s THEN \'current\' WHEN post_date BETWEEN %s AND %s THEN \'previous\' END as period,', $date_range['current_month_start'], $date_range['current_month_end'], $date_range['previous_month_start'], $date_range['previous_month_end'] - ) ); + ) );
109-110: Critical: SQL injection vulnerability with order status exclusions.Same issue as in VendorOrderStatsStore - the
$exclude_order_statusesvalues are directly concatenated into SQL.- $this->add_sql_clause( 'where', " AND p1.post_status NOT IN ( '" . implode( "','", $exclude_order_statuses ) . "' )" ); + $placeholders = array_fill( 0, count( $exclude_order_statuses ), '%s' ); + $this->add_sql_clause( 'where', $wpdb->prepare( " AND p1.post_status NOT IN ( " . implode( ',', $placeholders ) . " )", $exclude_order_statuses ) );Also fix within the subquery on line 117:
- AND p2.post_status NOT IN ( '" . implode( "','", $exclude_order_statuses ) . "' ) + AND p2.post_status NOT IN ( " . implode( ',', $placeholders ) . " )
🧹 Nitpick comments (1)
includes/REST/AdminDashboardController.php (1)
153-156: Use consistent null coalescing operator for date parameters.The code uses
??operator in line 155 but ternary operator in lines 171 and 187. For consistency and cleaner code, use the null coalescing operator throughout.public function get_monthly_overview_data( $request ) { // Get the selected month and year from the request $date = $request->get_param( 'date' ) ?? date( 'Y-m' ); public function get_sales_chart_data( $request ) { // Get the selected month and year from the request (same as monthly overview) - $date = $request->get_param( 'date' ) ? sanitize_text_field( $request->get_param( 'date' ) ) : date( 'Y-m' ); + $date = sanitize_text_field( $request->get_param( 'date' ) ?? date( 'Y-m' ) ); public function get_customer_metrics_data( $request ) { // Get the selected month and year from the request - $date = $request->get_param( 'date' ) ? sanitize_text_field( $request->get_param( 'date' ) ) : date( 'Y-m' ); + $date = sanitize_text_field( $request->get_param( 'date' ) ?? date( 'Y-m' ) );Also applies to: 169-172, 185-188
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
includes/Models/AdminDashboardStats.php(1 hunks)includes/Models/DataStore/AdminDashboardStatsStore.php(1 hunks)includes/Models/DataStore/VendorOrderStatsStore.php(1 hunks)includes/REST/AdminDashboardController.php(1 hunks)tests/php/src/REST/AdminDashboardControllerTest.php(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- includes/Models/AdminDashboardStats.php
🧰 Additional context used
🧬 Code Graph Analysis (1)
tests/php/src/REST/AdminDashboardControllerTest.php (2)
includes/REST/AdminDashboardController.php (11)
AdminDashboardController(18-591)get_to_do(118-142)get_monthly_overview_data(153-158)get_customer_metrics_data(185-190)get_all_time_stats_data(199-201)get_top_performing_vendors_data(210-212)get_most_reviewed_products_data(221-223)get_vendor_approvals_count(232-235)get_product_approvals_count(244-254)get_pending_withdrawals_count(263-271)parse_date_range(545-569)tests/php/src/DokanTestCase.php (1)
DokanTestCase(23-398)
🪛 PHPMD (2.15.0)
includes/Models/DataStore/VendorOrderStatsStore.php
142-142: Avoid unused local variables such as '$exclude_order_statuses'. (Unused Code Rules)
(UnusedLocalVariable)
tests/php/src/REST/AdminDashboardControllerTest.php
164-164: Avoid unused local variables such as '$request'. (Unused Code Rules)
(UnusedLocalVariable)
🪛 GitHub Check: Run PHPCS inspection
includes/Models/DataStore/VendorOrderStatsStore.php
[warning] 117-117:
Use placeholders and $wpdb->prepare(); found get_query_statement
[warning] 117-117:
Use placeholders and $wpdb->prepare(); found $this
includes/Models/DataStore/AdminDashboardStatsStore.php
[warning] 298-298:
Equals sign not aligned correctly; expected 1 space but found 2 spaces
[failure] 161-161:
Closing parenthesis of a multi-line function call must be on a line by itself
[failure] 157-157:
Whitespace found at end of line
[failure] 157-157:
Opening parenthesis of a multi-line function call must be the last content on the line
[failure] 156-156:
Closing parenthesis of a multi-line function call must be on a line by itself
[failure] 152-152:
Whitespace found at end of line
[failure] 152-152:
Opening parenthesis of a multi-line function call must be the last content on the line
[failure] 122-122:
Functions must not contain multiple empty lines in a row; found 2 empty lines
[failure] 121-121:
Closing parenthesis of a multi-line function call must be on a line by itself
[warning] 117-117:
Use placeholders and $wpdb->prepare(); found $exclude_order_statuses
[failure] 111-111:
Opening parenthesis of a multi-line function call must be the last content on the line
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: api tests (1, 1)
- GitHub Check: e2e tests (3, 3)
- GitHub Check: e2e tests (1, 3)
- GitHub Check: e2e tests (2, 3)
🔇 Additional comments (2)
tests/php/src/REST/AdminDashboardControllerTest.php (1)
84-480: Excellent test coverage!The test suite comprehensively covers:
- All REST endpoints registration and existence
- Permission checks for different user roles
- Data structure validation for each endpoint
- Helper method unit tests
- Date parsing edge cases (including leap year handling)
- Security testing with unauthorized/forbidden access scenarios
includes/REST/AdminDashboardController.php (1)
1-591: Excellent refactoring of the admin dashboard API!The refactoring from a feed-based system to a comprehensive metrics API is well-executed:
- Clean REST API design with modular endpoints
- Proper separation of concerns using models and data stores
- Good security practices with permission checks and input sanitization
- Comprehensive data aggregation for dashboard widgets
- Extensible architecture with WordPress filters throughout
The new architecture provides a solid foundation for the admin dashboard metrics feature.
…cs-api' into feat/check-admin-dashboard-metrics-api # Conflicts: # includes/Models/DataStore/VendorOrderStatsStore.php # includes/REST/AdminDashboardController.php
…dorOrderStatsStore
…rics-api Feat/optimized query and API admin dashboard
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (2)
includes/Models/DataStore/VendorOrderStatsStore.php (2)
83-84: Critical: SQL injection vulnerability with order status exclusions.The
$exclude_order_statusesarray values are directly concatenated into SQL queries without proper escaping. This could lead to SQL injection if the values are not properly sanitized.Apply this fix:
- $this->add_sql_clause( 'where', " AND wos.status NOT IN ( '" . implode( "','", $exclude_order_statuses ) . "' )" ); + $placeholders = array_fill( 0, count( $exclude_order_statuses ), '%s' ); + $this->add_sql_clause( 'where', $wpdb->prepare( " AND wos.status NOT IN ( " . implode( ',', $placeholders ) . " )", $exclude_order_statuses ) );
154-154: Critical: SQL injection vulnerability with order status exclusions.Same SQL injection vulnerability as in the
get_active_vendors_count()method.Apply this fix:
- $this->add_sql_clause( 'where', " AND wos.status NOT IN ( '" . implode( "','", ReportUtil::get_exclude_order_statuses() ) . "' )" ); + $exclude_order_statuses = ReportUtil::get_exclude_order_statuses(); + $placeholders = array_fill( 0, count( $exclude_order_statuses ), '%s' ); + $this->add_sql_clause( 'where', $wpdb->prepare( " AND wos.status NOT IN ( " . implode( ',', $placeholders ) . " )", $exclude_order_statuses ) );
🧹 Nitpick comments (3)
includes/Models/VendorOrderStats.php (2)
58-368: Consider reducing boilerplate code with magic methods or traits.The repetitive getter/setter pattern creates significant code duplication. Consider using magic methods or a trait to reduce this boilerplate.
Example implementation using magic methods:
/** * Magic getter for properties */ public function __get($name) { $method = 'get_' . $name; if (method_exists($this, $method)) { return $this->$method(); } if (array_key_exists($name, $this->data)) { $value = $this->get_prop($name); // Apply type casting based on property if (in_array($name, ['vendor_id', 'order_id', 'order_type'])) { return (int) $value; } if (strpos($name, '_fee') !== false || strpos($name, '_earning') !== false || strpos($name, '_commission') !== false || strpos($name, '_discount') !== false || strpos($name, '_subsidy') !== false) { return (float) $value; } return $value; } throw new Exception("Property {$name} does not exist"); }
410-422: Add missing PHPDoc parameter.The
$group_by_dayparameter is missing from the PHPDoc comment./** * Get sales chart data for a date range. * * @since DOKAN_SINCE * * @param string $start_date Start date in Y-m-d format. * @param string $end_date End date in Y-m-d format. + * @param bool $group_by_day Whether to group data by day. Default false. * * @return array Sales chart data with totals. */includes/REST/AdminDashboardController.php (1)
286-303: Handle missing vendors more gracefully.When a vendor is not found (e.g., deleted), the current implementation skips them but continues incrementing the rank, which could create gaps in the results.
Consider fetching extra vendors to account for missing ones:
- $vendors = AdminDashboardStats::get_top_performing_vendors(); + // Fetch extra vendors to account for potentially missing ones + $vendors = AdminDashboardStats::get_top_performing_vendors(10); // If vendors found, then populate the result array if ( ! empty( $vendors ) ) { $rank = 0; foreach ( $vendors as $vendor ) { + if ( count( $result ) >= 5 ) { + break; + } $vendor_info = dokan()->vendor->get( $vendor['vendor_id'] ); if ( ! $vendor_info->get_id() ) { continue; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
includes/Models/DataStore/VendorOrderStatsStore.php(1 hunks)includes/Models/VendorOrderStats.php(1 hunks)includes/REST/AdminDashboardController.php(1 hunks)
…t_datetime for consistency
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (3)
includes/REST/AdminDashboardController.php (3)
40-40: Permission callback allows shop managers in addition to admins.All endpoints use the same
check_permissioncallback which currently allows both administrators and shop managers due to themanage_woocommercecapability. If these routes should be admin-only, consider using a more restrictive capability.Also applies to: 50-50, 62-62, 73-73, 84-84, 95-95, 106-106
154-159: Add input validation for date parameters.The date parameters are sanitized but not validated for format before use. Consider adding validation to ensure the date format matches the expected 'Y-m' pattern.
Also applies to: 170-175, 186-195
371-381: Optimize product count query for better performance.Using
limit => -1to fetch all products and then counting them in PHP is inefficient for large stores. Consider using a direct count query instead.
🧹 Nitpick comments (3)
includes/REST/AdminDashboardController.php (3)
292-296: Add error handling for invalid vendor data.The code silently skips invalid vendors using
continuewithout logging or tracking these cases. Consider adding error logging to help with debugging.if ( ! $vendor_info->get_id() ) { + // Log invalid vendor for debugging + error_log( "Invalid vendor ID: " . $vendor['vendor_id'] ); continue; }
323-338: Consider optimizing the most reviewed products query.The current approach uses
meta_querywhich can be slower for large product catalogs. Consider using a more direct approach or caching the results.Alternative approach using direct WP_Query with better performance:
-$products = wc_get_products( - [ - 'limit' => 5, - 'orderby' => 'comment_count', - 'order' => 'DESC', - 'status' => 'publish', - 'meta_query' => [ // phpcs:ignore - [ - 'key' => '_wc_review_count', - 'value' => 0, - 'compare' => '>', - 'type' => 'NUMERIC', - ], - ], - ] -); +$products_query = new WP_Query([ + 'post_type' => 'product', + 'post_status' => 'publish', + 'posts_per_page' => 5, + 'orderby' => 'comment_count', + 'order' => 'DESC', + 'meta_query' => [ + [ + 'key' => '_wc_review_count', + 'value' => 0, + 'compare' => '>', + 'type' => 'NUMERIC', + ], + ], +]); +$products = array_map('wc_get_product', $products_query->posts);
154-159: Add consistent type hints and return type declarations.Several methods lack proper type hints for the
$requestparameter and return type declarations. This improves code clarity and IDE support.-public function get_monthly_overview_data( $request ) { +public function get_monthly_overview_data( WP_REST_Request $request ): WP_REST_Response { -public function get_sales_chart_data( $request ) { +public function get_sales_chart_data( WP_REST_Request $request ): WP_REST_Response { -public function get_customer_metrics_data( $request ) { +public function get_customer_metrics_data( WP_REST_Request $request ): WP_REST_Response { -public function get_monthly_overview( $date ) { +public function get_monthly_overview( string $date ): array {Also applies to: 170-177, 188-195, 523-533
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
includes/DependencyManagement/Providers/AdminDashboardServiceProvider.php(1 hunks)includes/REST/AdminDashboardController.php(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- includes/DependencyManagement/Providers/AdminDashboardServiceProvider.php
🧰 Additional context used
🧬 Code Graph Analysis (1)
includes/REST/AdminDashboardController.php (7)
includes/Models/AdminDashboardStats.php (4)
AdminDashboardStats(12-126)get_monthly_overview(110-112)get_customer_metrics(45-47)get_top_performing_vendors(123-125)includes/Models/VendorOrderStats.php (3)
VendorOrderStats(12-423)get_sales_chart_data(420-422)get_top_performing_vendors(406-408)includes/REST/DokanBaseAdminController.php (1)
DokanBaseAdminController(11-29)includes/functions.php (2)
dokan_current_datetime(3606-3612)dokan_get_seller_status_count(3384-3410)includes/Models/DataStore/AdminDashboardStatsStore.php (3)
get_monthly_overview(330-341)get_customer_metrics(98-137)get_top_performing_vendors(352-355)includes/Models/DataStore/VendorOrderStatsStore.php (2)
get_sales_chart_data(136-198)get_top_performing_vendors(107-123)includes/Withdraw/Manager.php (2)
all(357-376)get(399-439)
🔇 Additional comments (1)
includes/REST/AdminDashboardController.php (1)
19-26: Excellent architectural refactoring with proper separation of concerns.The refactoring successfully moves from a feed-based approach to modular REST endpoints with proper model separation. The use of dedicated model classes (
AdminDashboardStats,VendorOrderStats) and centralized date handling improves maintainability.Also applies to: 33-110, 285-312
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (3)
includes/Models/DataStore/AdminDashboardStatsStore.php (3)
109-110: Critical: SQL injection vulnerability with order status exclusions.The past review comment about this SQL injection vulnerability is still valid and needs to be addressed.
Also applies to: 119-119
154-161: Fix multi-line function call formatting.The past review comment about multi-line function call formatting is still valid and needs to be addressed.
Also applies to: 162-169
205-212: Fix multi-line function call formatting.The past review comment about multi-line function call formatting is still valid and needs to be addressed.
Also applies to: 217-221
🧹 Nitpick comments (2)
tests/php/src/REST/AdminDashboardStatsControllerTest.php (1)
164-164: Remove unused variable$request.The
$requestvariable is created but never used since theget_to_do()method doesn't accept any parameters.- $request = new WP_REST_Request( 'GET', "/{$this->namespace}/{$this->rest_base}/todo" ); $controller = new AdminDashboardStatsController();includes/REST/AdminDashboardStatsController.php (1)
293-296: Add null check for vendor info object.Consider adding a more defensive check to ensure
$vendor_infois a valid object before calling methods on it.$vendor_info = dokan()->vendor->get( $vendor['vendor_id'] ); - if ( ! $vendor_info->get_id() ) { + if ( ! $vendor_info || ! $vendor_info->get_id() ) { continue; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
includes/DependencyManagement/Providers/AdminDashboardServiceProvider.php(1 hunks)includes/Models/DataStore/AdminDashboardStatsStore.php(1 hunks)includes/REST/AdminDashboardController.php(2 hunks)includes/REST/AdminDashboardStatsController.php(1 hunks)includes/REST/Manager.php(1 hunks)tests/php/src/REST/AdminDashboardStatsControllerTest.php(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- includes/REST/AdminDashboardController.php
🚧 Files skipped from review as they are similar to previous changes (2)
- includes/REST/Manager.php
- includes/DependencyManagement/Providers/AdminDashboardServiceProvider.php
🧰 Additional context used
🪛 PHPMD (2.15.0)
tests/php/src/REST/AdminDashboardStatsControllerTest.php
164-164: Avoid unused local variables such as '$request'. (Unused Code Rules)
(UnusedLocalVariable)
🪛 GitHub Check: Run PHPCS inspection
includes/Models/DataStore/AdminDashboardStatsStore.php
[failure] 221-221:
Closing parenthesis of a multi-line function call must be on a line by itself
[failure] 217-217:
Whitespace found at end of line
[failure] 217-217:
Opening parenthesis of a multi-line function call must be the last content on the line
[failure] 212-212:
Closing parenthesis of a multi-line function call must be on a line by itself
[failure] 205-205:
Whitespace found at end of line
[failure] 205-205:
Opening parenthesis of a multi-line function call must be the last content on the line
[warning] 119-119:
Use placeholders and $wpdb->prepare(); found $exclude_order_statuses
includes/REST/AdminDashboardStatsController.php
[failure] 556-556:
date() is affected by runtime timezone changes which can cause date/time to be incorrectly displayed. Use gmdate() instead.
[failure] 555-555:
date() is affected by runtime timezone changes which can cause date/time to be incorrectly displayed. Use gmdate() instead.
[failure] 549-549:
date() is affected by runtime timezone changes which can cause date/time to be incorrectly displayed. Use gmdate() instead.
[failure] 548-548:
date() is affected by runtime timezone changes which can cause date/time to be incorrectly displayed. Use gmdate() instead.
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: e2e tests (3, 3)
- GitHub Check: e2e tests (1, 3)
- GitHub Check: e2e tests (2, 3)
- GitHub Check: api tests (1, 1)
…DashboardStatsController
…inDashboardStatsStore
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
includes/REST/AdminDashboardStatsController.php (1)
463-465: Add date parameter sanitization for consistency.The
get_sales_chartmethod doesn't sanitize the date parameter, while other similar methods do. Add sanitization for consistency and security.Apply this diff to add sanitization:
public function get_sales_chart( $date = '' ) { // Current month totals - $date_range = $this->parse_date_range( $date ); + $date_range = $this->parse_date_range( ! empty( $date ) ? sanitize_text_field( $date ) : '' );
🧹 Nitpick comments (2)
tests/php/src/REST/AdminDashboardStatsControllerTest.php (2)
86-86: Fix formatting issues to comply with WordPress coding standards.The static analysis detected formatting issues that should be addressed:
- Remove trailing whitespace
- Add commas after the last array items in multi-line arrays
Apply this diff to fix the formatting issues:
$routes = $this->server->get_routes( $this->namespace ); - + $expected_endpoints = [ 'todo', 'monthly-overview', 'sales-chart', 'customer-metrics', 'all-time-stats', 'top-performing-vendors', - 'most-reviewed-products' + 'most-reviewed-products', ];$routes = $this->server->get_routes( $this->namespace ); - + $expected_endpoints = [ 'todo', 'monthly-overview', 'sales-chart', 'customer-metrics', 'all-time-stats', 'top-performing-vendors', - 'most-reviewed-products' + 'most-reviewed-products', ];Also applies to: 94-95, 113-113, 121-122
164-166: Remove unused variable to improve code clarity.The
$requestvariable is created but never used sinceget_to_do()doesn't accept any parameters.Apply this diff to remove the unused variable:
- $request = new WP_REST_Request( 'GET', "/{$this->namespace}/{$this->rest_base}/todo" ); $controller = new AdminDashboardStatsController(); $response = $controller->get_to_do();
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
includes/DependencyManagement/Providers/ModelServiceProvider.php(1 hunks)includes/DependencyManagement/Providers/ServiceProvider.php(1 hunks)includes/REST/AdminDashboardStatsController.php(1 hunks)tests/php/src/REST/AdminDashboardStatsControllerTest.php(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- includes/DependencyManagement/Providers/ModelServiceProvider.php
🚧 Files skipped from review as they are similar to previous changes (1)
- includes/DependencyManagement/Providers/ServiceProvider.php
🧰 Additional context used
🪛 PHPMD (2.15.0)
tests/php/src/REST/AdminDashboardStatsControllerTest.php
164-164: Avoid unused local variables such as '$request'. (Unused Code Rules)
(UnusedLocalVariable)
🪛 GitHub Check: Run PHPCS inspection
tests/php/src/REST/AdminDashboardStatsControllerTest.php
[failure] 121-121:
There should be a comma after the last array item in a multi-line array.
[failure] 113-113:
Whitespace found at end of line
[failure] 94-94:
There should be a comma after the last array item in a multi-line array.
[failure] 86-86:
Whitespace found at end of line
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: e2e tests (2, 3)
- GitHub Check: e2e tests (3, 3)
- GitHub Check: e2e tests (1, 3)
- GitHub Check: api tests (1, 1)
🔇 Additional comments (2)
tests/php/src/REST/AdminDashboardStatsControllerTest.php (1)
45-481: Excellent test coverage!The test implementation is comprehensive and well-structured:
- Proper setup with different user roles (admin, vendor, customer)
- Thorough permission testing for all endpoints
- Response structure validation
- Edge case testing including date parameters and unauthorized access
- Correct handling of leap year in date range tests
includes/REST/AdminDashboardStatsController.php (1)
119-590: Well-implemented controller with good practices!The controller implementation demonstrates several good practices:
- Comprehensive PHPDoc documentation for all methods
- Consistent use of WordPress filters for extensibility
- Proper type casting for return values (int, float)
- Good separation of concerns by delegating to model classes
- Efficient use of WooCommerce and Dokan APIs
- Proper permission checks via parent class
…better readability and maintainability
MdAsifHossainNadim
changed the base branch from
develop
to
enhance/dokan-admin-dashboard-components
…mponents' into feat/admin-dashboard-metrics-api # Conflicts: # includes/DependencyManagement/Providers/AdminDashboardServiceProvider.php
…ate related tests
… in AdminDashboardStatsStore
…date range handling
MdAsifHossainNadim
changed the title
MdAsifHossainNadim
changed the base branch from
enhance/dokan-admin-dashboard-components
to
enhance/update-dashboard-menu-page-structure
MdAsifHossainNadim
changed the title
…/admin-dashboard-metrics-api
…' into feat/admin-dashboard-metrics-api
…/admin-dashboard-metrics-api
…/admin-dashboard-metrics-api
…/admin-dashboard-metrics-api
All Submissions:
Changes proposed in this Pull Request:
Related Pull Request(s)
Closes
How to test the changes in this Pull Request:
Changelog entry
Title
Detailed Description of the pull request. What was previous behaviour
and what will be changed in this PR.
Before Changes
Describe the issue before changes with screenshots(s).
After Changes
Describe the issue after changes with screenshot(s).
Feature Video (optional)
Link of detailed video if this PR is for a feature.
PR Self Review Checklist:
FOR PR REVIEWER ONLY:
Summary by CodeRabbit
New Features
Bug Fixes
Tests