| @@ -290,18 +290,28 @@ | ||
| 290 | 290 | 'type' => 'string', |
| 291 | 291 | 'enum' => ['7d', '30d', '90d', 'all'], |
| 292 | 292 | 'sanitize_callback' => 'sanitize_key' |
| 293 | 293 | ], |
| 294 | - 'group_by' => [ | |
| 295 | - 'default' => 'day', | |
| 296 | - 'type' => 'string', | |
| 297 | - 'enum' => ['day', 'week', 'month'], | |
| 298 | - 'sanitize_callback' => 'sanitize_key' | |
| 299 | - ], | |
| 300 | 294 | 'user_id' => [ |
| 301 | 295 | 'default' => 0, |
| 302 | 296 | 'type' => 'integer', |
| 303 | 297 | 'sanitize_callback' => 'absint' |
| 298 | + ], | |
| 299 | + // Declared because the handler reads them. They were validated | |
| 300 | + // only by the handler's own clamping, so they had no type | |
| 301 | + // coercion and did not appear in the endpoint's schema. | |
| 302 | + 'page' => [ | |
| 303 | + 'default' => 1, | |
| 304 | + 'type' => 'integer', | |
| 305 | + 'minimum' => 1, | |
| 306 | + 'sanitize_callback' => 'absint' | |
| 307 | + ], | |
| 308 | + 'per_page' => [ | |
| 309 | + 'default' => 20, | |
| 310 | + 'type' => 'integer', | |
| 311 | + 'minimum' => 10, | |
| 312 | + 'maximum' => 100, | |
| 313 | + 'sanitize_callback' => 'absint' | |
| 304 | 314 | ] |
| 305 | 315 | ] |
| 306 | 316 | ]); |
| 307 | 317 | |
| @@ -380,9 +390,8 @@ | ||
| 380 | 390 | 'ai_actions' => $ai_metrics['total_actions'], |
| 381 | 391 | 'features_used_count' => $ai_metrics['features_used_count'], |
| 382 | 392 | 'most_used_feature' => $ai_metrics['most_used_feature'], |
| 383 | 393 | 'most_used_count' => $ai_metrics['most_used_count'], |
| 384 | - 'success_rate' => $ai_metrics['success_rate'], | |
| 385 | 394 | 'content_briefs' => $brief_metrics['total_briefs'], |
| 386 | 395 | 'feature_breakdown' => $ai_metrics['feature_breakdown'], |
| 387 | 396 | 'provider_breakdown' => $cost_data['by_provider'] |
| 388 | 397 | ], |
| @@ -446,8 +455,34 @@ | ||
| 446 | 455 | return true; |
| 447 | 456 | } |
| 448 | 457 | |
| 449 | 458 | /** |
| 459 | + * Bind the cache-invalidation listeners for the whole request lifecycle. | |
| 460 | + * | |
| 461 | + * The listeners used to be registered only by the constructor, which runs | |
| 462 | + * on rest_api_init — so usage logged during cron, WP-CLI or an admin-post | |
| 463 | + * request found no listener and the cached overview rode out its full TTL. | |
| 464 | + * Called from API\Manager::init() on every request instead. | |
| 465 | + * | |
| 466 | + * @since 2.2.1 | |
| 467 | + * @return void | |
| 468 | + */ | |
| 469 | + public static function boot_cache_invalidation(): void { | |
| 470 | + static $booted = false; | |
| 471 | + | |
| 472 | + if ($booted) { | |
| 473 | + return; | |
| 474 | + } | |
| 475 | + | |
| 476 | + $booted = true; | |
| 477 | + | |
| 478 | + // Constructing the endpoint registers the listeners; the guard in | |
| 479 | + // setup_cache_invalidation() keeps a later REST construction from | |
| 480 | + // double-binding them. | |
| 481 | + new self(); | |
| 482 | + } | |
| 483 | + | |
| 484 | + /** | |
| 450 | 485 | * Set up cache invalidation hooks |
| 451 | 486 | * |
| 452 | 487 | * @since 1.0.0 |
| 453 | 488 | * @return void |
| @@ -452,8 +487,23 @@ | ||
| 452 | 487 | * @since 1.0.0 |
| 453 | 488 | * @return void |
| 454 | 489 | */ |
| 455 | 490 | private function setup_cache_invalidation(): void { |
| 491 | + // The endpoint is constructed more than once per request — once on | |
| 492 | + // init via boot_cache_invalidation(), again on rest_api_init, and | |
| 493 | + // potentially by callers resolving it on demand. Bind once per | |
| 494 | + // request, or every event invalidates N times. | |
| 495 | + // | |
| 496 | + // A has_action() check cannot do this: the callback is [$this, ...] | |
| 497 | + // and each construction is a different instance, so it never matches. | |
| 498 | + static $bound = false; | |
| 499 | + | |
| 500 | + if ($bound) { | |
| 501 | + return; | |
| 502 | + } | |
| 503 | + | |
| 504 | + $bound = true; | |
| 505 | + | |
| 456 | 506 | // Invalidate analytics cache when AI usage is logged |
| 457 | 507 | add_action('thinkrank_ai_usage_logged', [$this, 'invalidate_analytics_cache']); |
| 458 | 508 | |
| 459 | 509 | // Invalidate analytics cache when SEO scores are updated |
| @@ -545,46 +595,50 @@ | ||
| 545 | 595 | ]; |
| 546 | 596 | |
| 547 | 597 | foreach ($usage_data as $usage) { |
| 548 | 598 | $tokens = (int) $usage['tokens_used']; |
| 549 | - $provider = $usage['provider']; | |
| 599 | + $provider = (string) $usage['provider']; | |
| 550 | 600 | |
| 551 | - // Estimate 70% input, 30% output tokens | |
| 552 | - $input_tokens = $tokens * 0.7; | |
| 553 | - $output_tokens = $tokens * 0.3; | |
| 601 | + // Unknown provider: no pricing table, so it cannot be costed. Skip | |
| 602 | + // rather than let `+=` invent a key that the total below misses. | |
| 603 | + if (!isset($costs[$provider])) { | |
| 604 | + continue; | |
| 605 | + } | |
| 554 | 606 | |
| 555 | - $cost = 0; | |
| 607 | + // Price at the model the request actually used. Reading only the | |
| 608 | + // provider meant every row was costed at that provider's default | |
| 609 | + // model, so this total disagreed with the per-record figures in | |
| 610 | + // the Usage Breakdown tab — by 4.5x on a gpt-4o-mini workload. | |
| 611 | + $metadata = !empty($usage['metadata']) ? json_decode((string) $usage['metadata'], true) : []; | |
| 612 | + $model = is_array($metadata) && !empty($metadata['actual_model']) | |
| 613 | + ? (string) $metadata['actual_model'] | |
| 614 | + : $this->get_default_model($provider); | |
| 556 | 615 | |
| 557 | - // Use the robust pricing helper for consistent cost calculation | |
| 558 | - $pricing = $this->get_model_pricing($provider); | |
| 559 | - if ($pricing) { | |
| 560 | - $cost = ($input_tokens * $pricing['input'] / 1000000) + | |
| 561 | - ($output_tokens * $pricing['output'] / 1000000); | |
| 562 | - $costs[$provider] += $cost; | |
| 563 | - } | |
| 616 | + // Single source of truth for per-row pricing, shared with | |
| 617 | + // get_detailed_usage_breakdown() so both tabs always agree. | |
| 618 | + $costs[$provider] += $this->calculate_record_cost($provider, $tokens, $model); | |
| 564 | 619 | } |
| 565 | 620 | |
| 566 | 621 | $costs['total'] = $costs['openai'] + $costs['claude'] + $costs['gemini'] + $costs['openrouter']; |
| 567 | 622 | |
| 568 | - // Format provider breakdown | |
| 569 | - $costs['by_provider'] = [ | |
| 570 | - 'openai' => [ | |
| 571 | - 'cost' => round($costs['openai'], 4), | |
| 572 | - 'percentage' => $costs['total'] > 0 ? round(($costs['openai'] / $costs['total']) * 100, 1) : 0 | |
| 573 | - ], | |
| 574 | - 'claude' => [ | |
| 575 | - 'cost' => round($costs['claude'], 4), | |
| 576 | - 'percentage' => $costs['total'] > 0 ? round(($costs['claude'] / $costs['total']) * 100, 1) : 0 | |
| 577 | - ], | |
| 578 | - 'gemini' => [ | |
| 579 | - 'cost' => round($costs['gemini'], 4), | |
| 580 | - 'percentage' => $costs['total'] > 0 ? round(($costs['gemini'] / $costs['total']) * 100, 1) : 0 | |
| 581 | - ], | |
| 582 | - 'openrouter' => [ | |
| 583 | - 'cost' => round($costs['openrouter'], 4), | |
| 584 | - 'percentage' => $costs['total'] > 0 ? round(($costs['openrouter'] / $costs['total']) * 100, 1) : 0 | |
| 585 | - ] | |
| 586 | - ]; | |
| 623 | + // Report only providers that actually incurred cost. Emitting all four | |
| 624 | + // unconditionally meant a site with no AI usage rendered four ranked | |
| 625 | + // rows at "$0.0000 (0%)" — reading as "four providers were used and | |
| 626 | + // each was free" — and made the panel's own "No provider cost data" | |
| 627 | + // empty state unreachable. | |
| 628 | + $costs['by_provider'] = []; | |
| 629 | + foreach (['openai', 'claude', 'gemini', 'openrouter'] as $provider) { | |
| 630 | + if ($costs[$provider] <= 0) { | |
| 631 | + continue; | |
| 632 | + } | |
| 633 | + | |
| 634 | + $costs['by_provider'][$provider] = [ | |
| 635 | + 'cost' => round($costs[$provider], 4), | |
| 636 | + 'percentage' => $costs['total'] > 0 | |
| 637 | + ? round(($costs[$provider] / $costs['total']) * 100, 1) | |
| 638 | + : 0 | |
| 639 | + ]; | |
| 640 | + } | |
| 587 | 641 | |
| 588 | 642 | return $costs; |
| 589 | 643 | } |
| 590 | 644 | |
| @@ -624,9 +678,9 @@ | ||
| 624 | 678 | // phpcs:ignore WordPress.DB.DirectDatabaseQuery.DirectQuery, WordPress.DB.DirectDatabaseQuery.NoCaching |
| 625 | 679 | $usage_data = $wpdb->get_results( |
| 626 | 680 | $wpdb->prepare( |
| 627 | 681 | // phpcs:ignore WordPress.DB.PreparedSQL.InterpolatedNotPrepared, PluginCheck.Security.DirectDB.UnescapedDBParameter -- $table_name is escaped via esc_sql(). |
| 628 | - "SELECT provider, action, tokens_used, created_at FROM `{$table_name}` WHERE user_id = %d AND created_at >= %s", | |
| 682 | + "SELECT provider, action, tokens_used, metadata, created_at FROM `{$table_name}` WHERE user_id = %d AND created_at >= %s", | |
| 629 | 683 | $user_id, |
| 630 | 684 | $cutoff |
| 631 | 685 | ), |
| 632 | 686 | ARRAY_A |
| @@ -635,9 +689,9 @@ | ||
| 635 | 689 | // phpcs:ignore WordPress.DB.DirectDatabaseQuery.DirectQuery, WordPress.DB.DirectDatabaseQuery.NoCaching |
| 636 | 690 | $usage_data = $wpdb->get_results( |
| 637 | 691 | $wpdb->prepare( |
| 638 | 692 | // phpcs:ignore WordPress.DB.PreparedSQL.InterpolatedNotPrepared, PluginCheck.Security.DirectDB.UnescapedDBParameter -- $table_name is escaped via esc_sql(). |
| 639 | - "SELECT provider, action, tokens_used, created_at FROM `{$table_name}` WHERE user_id = %d", | |
| 693 | + "SELECT provider, action, tokens_used, metadata, created_at FROM `{$table_name}` WHERE user_id = %d", | |
| 640 | 694 | $user_id |
| 641 | 695 | ), |
| 642 | 696 | ARRAY_A |
| 643 | 697 | ); |
| @@ -648,8 +702,16 @@ | ||
| 648 | 702 | // get_overview_metrics() reads every key unconditionally, so a |
| 649 | 703 | // short array here surfaces as undefined-key warnings and null |
| 650 | 704 | // fields for any user with no AI usage yet (i.e. a fresh install). |
| 651 | 705 | // The values mirror what the loop below produces for zero rows. |
| 706 | + // | |
| 707 | + // The change fields are COMPUTED here rather than hardcoded to 0. | |
| 708 | + // An empty current window does not mean "nothing changed": a user | |
| 709 | + // whose usage fell from five actions last month to none this month | |
| 710 | + // was shown a 0 — rendered as the same em-dash a genuinely flat | |
| 711 | + // period gets — instead of the -100% that actually happened. | |
| 712 | + $previous = $this->get_previous_period_data($user_id, $date_condition); | |
| 713 | + | |
| 652 | 714 | return [ |
| 653 | 715 | 'total_actions' => 0, |
| 654 | 716 | 'total_tokens' => 0, |
| 655 | 717 | 'feature_breakdown' => [], |
| @@ -655,12 +717,17 @@ | ||
| 655 | 717 | 'feature_breakdown' => [], |
| 656 | 718 | 'features_used_count' => 0, |
| 657 | 719 | 'most_used_feature' => '', |
| 658 | 720 | 'most_used_count' => 0, |
| 659 | - 'success_rate' => 0, | |
| 660 | 721 | 'usage_data' => [], |
| 661 | - 'cost_change' => 0, | |
| 662 | - 'time_saved_change' => 0 | |
| 722 | + 'cost_change' => $this->calculate_percentage_change( | |
| 723 | + array_key_exists('total_cost', $previous) ? $previous['total_cost'] : 0, | |
| 724 | + 0.0 | |
| 725 | + ), | |
| 726 | + 'time_saved_change' => $this->calculate_percentage_change( | |
| 727 | + array_key_exists('time_saved', $previous) ? $previous['time_saved'] : 0, | |
| 728 | + 0.0 | |
| 729 | + ) | |
| 663 | 730 | ]; |
| 664 | 731 | } |
| 665 | 732 | |
| 666 | 733 | // Calculate feature breakdown and new metrics |
| @@ -692,20 +759,24 @@ | ||
| 692 | 759 | $most_used_count = $count; |
| 693 | 760 | } |
| 694 | 761 | } |
| 695 | 762 | |
| 696 | - // Calculate success rate (assuming all logged actions are successful for now) | |
| 697 | - // In future, we could track failed attempts separately | |
| 698 | - $success_rate = $total_actions > 0 ? 100 : 0; | |
| 763 | + // No success rate here on purpose. It used to be | |
| 764 | + // `$total_actions > 0 ? 100 : 0` — a constant presented as a | |
| 765 | + // measurement, and one that could only ever read 100% or 0%. Failed | |
| 766 | + // AI calls are never written to this table, so there is nothing to | |
| 767 | + // compute a rate from; the KPI card is gone until there is. | |
| 699 | 768 | |
| 700 | 769 | // Calculate changes from previous period |
| 701 | 770 | $previous_period_data = $this->get_previous_period_data($user_id, $date_condition); |
| 771 | + // Note the lack of `?? 0`: a null here means "no previous period", | |
| 772 | + // and coalescing it to zero would turn that back into a fake 100%. | |
| 702 | 773 | $cost_change = $this->calculate_percentage_change( |
| 703 | - $previous_period_data['total_cost'] ?? 0, | |
| 774 | + array_key_exists('total_cost', $previous_period_data) ? $previous_period_data['total_cost'] : 0, | |
| 704 | 775 | $this->calculate_total_cost($usage_data) |
| 705 | 776 | ); |
| 706 | 777 | $time_saved_change = $this->calculate_percentage_change( |
| 707 | - $previous_period_data['time_saved'] ?? 0, | |
| 778 | + array_key_exists('time_saved', $previous_period_data) ? $previous_period_data['time_saved'] : 0, | |
| 708 | 779 | $this->calculate_time_saved($feature_breakdown) |
| 709 | 780 | ); |
| 710 | 781 | |
| 711 | 782 | return [ |
| @@ -714,9 +785,8 @@ | ||
| 714 | 785 | 'feature_breakdown' => $feature_breakdown, |
| 715 | 786 | 'features_used_count' => $features_used_count, |
| 716 | 787 | 'most_used_feature' => $most_used_feature, |
| 717 | 788 | 'most_used_count' => $most_used_count, |
| 718 | - 'success_rate' => $success_rate, | |
| 719 | 789 | 'usage_data' => $usage_data, |
| 720 | 790 | 'cost_change' => $cost_change, |
| 721 | 791 | 'time_saved_change' => $time_saved_change |
| 722 | 792 | ]; |
| @@ -758,24 +828,38 @@ | ||
| 758 | 828 | ); |
| 759 | 829 | } |
| 760 | 830 | |
| 761 | 831 | if (!$result || (int) $result['content_optimized'] === 0) { |
| 832 | + // Same reasoning as the empty branch in get_ai_usage_metrics(): | |
| 833 | + // an empty current window is not "no change". A user who | |
| 834 | + // optimized three posts last month and none this month should | |
| 835 | + // see -100%, not the em-dash a flat period gets — and for `all` | |
| 836 | + // there is no previous window, so the change is null. | |
| 837 | + $previous = $this->get_previous_seo_data($user_id, $date_condition); | |
| 838 | + | |
| 762 | 839 | return [ |
| 763 | 840 | 'content_optimized' => 0, |
| 764 | 841 | 'average_seo_score' => 0, |
| 765 | - 'content_optimized_change' => 0, | |
| 766 | - 'seo_score_change' => 0 | |
| 842 | + 'content_optimized_change' => $this->calculate_percentage_change( | |
| 843 | + array_key_exists('content_optimized', $previous) ? $previous['content_optimized'] : 0, | |
| 844 | + 0.0 | |
| 845 | + ), | |
| 846 | + 'seo_score_change' => $this->calculate_percentage_change( | |
| 847 | + array_key_exists('average_seo_score', $previous) ? $previous['average_seo_score'] : 0, | |
| 848 | + 0.0 | |
| 849 | + ) | |
| 767 | 850 | ]; |
| 768 | 851 | } |
| 769 | 852 | |
| 770 | 853 | // Calculate changes from previous period |
| 771 | 854 | $previous_seo_data = $this->get_previous_seo_data($user_id, $date_condition); |
| 855 | + // As above: no `?? 0`, so a null "no previous period" survives. | |
| 772 | 856 | $content_optimized_change = $this->calculate_percentage_change( |
| 773 | - $previous_seo_data['content_optimized'] ?? 0, | |
| 857 | + array_key_exists('content_optimized', $previous_seo_data) ? $previous_seo_data['content_optimized'] : 0, | |
| 774 | 858 | (int) $result['content_optimized'] |
| 775 | 859 | ); |
| 776 | 860 | $seo_score_change = $this->calculate_percentage_change( |
| 777 | - $previous_seo_data['average_seo_score'] ?? 0, | |
| 861 | + array_key_exists('average_seo_score', $previous_seo_data) ? $previous_seo_data['average_seo_score'] : 0, | |
| 778 | 862 | round((float) $result['average_score'], 1) |
| 779 | 863 | ); |
| 780 | 864 | |
| 781 | 865 | return [ |
| @@ -832,9 +916,14 @@ | ||
| 832 | 916 | * @return WP_REST_Response|WP_Error Response object |
| 833 | 917 | */ |
| 834 | 918 | public function get_usage_breakdown(WP_REST_Request $request) { |
| 835 | 919 | try { |
| 836 | - $user_id = get_current_user_id(); | |
| 920 | + // Mirrors get_overview_metrics(). The two endpoints declared the | |
| 921 | + // same `user_id` argument but only overview honoured it, so the | |
| 922 | + // same query string described two different users depending on | |
| 923 | + // which one you asked. check_permissions() already requires | |
| 924 | + // manage_options before another user's id is accepted. | |
| 925 | + $user_id = $request->get_param('user_id') ?: get_current_user_id(); | |
| 837 | 926 | $period = $request->get_param('period') ?? '30d'; |
| 838 | 927 | // `(int)` binds tighter than `??`, so `(int) null` is 0 and the |
| 839 | 928 | // `?? 20` fallback was unreachable — per_page silently defaulted to |
| 840 | 929 | // the max(10, 0) floor of 10 rather than the 20 it advertises, and |
| @@ -857,9 +946,10 @@ | ||
| 857 | 946 | 'pagination' => [ |
| 858 | 947 | 'page' => $page, |
| 859 | 948 | 'per_page' => $per_page, |
| 860 | 949 | 'total_records' => $total_records, |
| 861 | - 'total_pages' => ceil($total_records / $per_page) | |
| 950 | + // (int) so it serialises as 2, not 2.0. | |
| 951 | + 'total_pages' => $per_page > 0 ? (int) ceil($total_records / $per_page) : 0 | |
| 862 | 952 | ], |
| 863 | 953 | 'period' => $period |
| 864 | 954 | ] |
| 865 | 955 | ], 200); |
| @@ -907,14 +997,22 @@ | ||
| 907 | 997 | * @param float $old_value Previous period value |
| 908 | 998 | * @param float $new_value Current period value |
| 909 | 999 | * @return float Percentage change |
| 910 | 1000 | */ |
| 911 | - private function calculate_percentage_change(float $old_value, float $new_value): float { | |
| 1001 | + private function calculate_percentage_change($old_value, float $new_value): ?float { | |
| 1002 | + // No previous period at all (the 'all' range). | |
| 1003 | + if (null === $old_value) { | |
| 1004 | + return null; | |
| 1005 | + } | |
| 1006 | + | |
| 912 | 1007 | if ((float) $old_value === 0.0) { |
| 913 | - return $new_value > 0 ? 100 : 0; | |
| 1008 | + // Growth from nothing has no percentage. Reporting a flat 100% | |
| 1009 | + // dressed it up as a measured change; null lets the UI say "new" | |
| 1010 | + // (or say nothing) instead of inventing a number. | |
| 1011 | + return $new_value > 0 ? null : 0.0; | |
| 914 | 1012 | } |
| 915 | 1013 | |
| 916 | - return round((($new_value - $old_value) / $old_value) * 100, 1); | |
| 1014 | + return round((($new_value - (float) $old_value) / (float) $old_value) * 100, 1); | |
| 917 | 1015 | } |
| 918 | 1016 | |
| 919 | 1017 | /** |
| 920 | 1018 | * Get previous period data for comparison |
| @@ -931,8 +1029,14 @@ | ||
| 931 | 1029 | |
| 932 | 1030 | // Extract the interval from current date condition to calculate previous period |
| 933 | 1031 | $previous_date_condition = $this->get_previous_period_condition($current_date_condition); |
| 934 | 1032 | |
| 1033 | + // No preceding window: report "not comparable" rather than querying a | |
| 1034 | + // made-up one. | |
| 1035 | + if (null === $previous_date_condition) { | |
| 1036 | + return ['total_cost' => null, 'time_saved' => null]; | |
| 1037 | + } | |
| 1038 | + | |
| 935 | 1039 | // Prepare and execute query with proper parameter binding to prevent SQL injection |
| 936 | 1040 | // phpcs:disable WordPress.DB.PreparedSQL.InterpolatedNotPrepared -- Table name is properly escaped, date condition is from controlled source |
| 937 | 1041 | // phpcs:ignore WordPress.DB.DirectDatabaseQuery.DirectQuery, WordPress.DB.DirectDatabaseQuery.NoCaching, PluginCheck.Security.DirectDB.UnescapedDBParameter -- Analytics data is real-time and shouldn't be cached |
| 938 | 1042 | $usage_data = $wpdb->get_results( |
| @@ -939,9 +1043,10 @@ | ||
| 939 | 1043 | $wpdb->prepare(" |
| 940 | 1044 | SELECT |
| 941 | 1045 | provider, |
| 942 | 1046 | action, |
| 943 | - tokens_used | |
| 1047 | + tokens_used, | |
| 1048 | + metadata | |
| 944 | 1049 | FROM `{$table_name}` |
| 945 | 1050 | WHERE user_id = %d |
| 946 | 1051 | {$previous_date_condition} |
| 947 | 1052 | ", $user_id), |
| @@ -1159,8 +1264,13 @@ | ||
| 1159 | 1264 | $table_name = esc_sql($this->database->get_table('seo_scores')); |
| 1160 | 1265 | |
| 1161 | 1266 | $previous_date_condition = $this->get_previous_period_condition($current_date_condition); |
| 1162 | 1267 | |
| 1268 | + // See get_previous_period_data(): no preceding window, no comparison. | |
| 1269 | + if (null === $previous_date_condition) { | |
| 1270 | + return ['content_optimized' => null, 'average_seo_score' => null]; | |
| 1271 | + } | |
| 1272 | + | |
| 1163 | 1273 | // Prepare and execute query with proper parameter binding to prevent SQL injection |
| 1164 | 1274 | // phpcs:disable WordPress.DB.PreparedSQL.InterpolatedNotPrepared -- Table name is properly escaped, date condition is from controlled source |
| 1165 | 1275 | // phpcs:ignore WordPress.DB.DirectDatabaseQuery.DirectQuery, WordPress.DB.DirectDatabaseQuery.NoCaching, PluginCheck.Security.DirectDB.UnescapedDBParameter -- Analytics data is real-time and shouldn't be cached |
| 1166 | 1276 | $result = $wpdb->get_row( |
| @@ -1207,12 +1317,18 @@ | ||
| 1207 | 1317 | |
| 1208 | 1318 | /** |
| 1209 | 1319 | * Convert current period condition to previous period condition |
| 1210 | 1320 | * |
| 1321 | + * Returns null when there is no preceding window to compare against. | |
| 1322 | + * `all` produces an empty date condition, which used to fall through to a | |
| 1323 | + * hardcoded 30–60 day fallback — so "all time" was compared against an | |
| 1324 | + * arbitrary month and reported a large, meaningless increase. "No | |
| 1325 | + * comparison" is now representable instead of being a parse failure. | |
| 1326 | + * | |
| 1211 | 1327 | * @param string $current_condition Current period SQL condition |
| 1212 | - * @return string Previous period SQL condition | |
| 1328 | + * @return string|null Previous period SQL condition, or null when none exists | |
| 1213 | 1329 | */ |
| 1214 | - private function get_previous_period_condition(string $current_condition): string { | |
| 1330 | + private function get_previous_period_condition(string $current_condition): ?string { | |
| 1215 | 1331 | // Extract interval from conditions like "AND created_at >= DATE_SUB(NOW(), INTERVAL 30 DAY)" |
| 1216 | 1332 | if (preg_match('/INTERVAL (\d+) (\w+)/', $current_condition, $matches)) { |
| 1217 | 1333 | $interval = (int) $matches[1]; |
| 1218 | 1334 | $unit = $matches[2]; |
| @@ -1224,9 +1340,9 @@ | ||
| 1224 | 1340 | return "AND created_at >= DATE_SUB(NOW(), INTERVAL {$start_interval} {$unit}) |
| 1225 | 1341 | AND created_at < DATE_SUB(NOW(), INTERVAL {$end_interval} {$unit})"; |
| 1226 | 1342 | } |
| 1227 | 1343 | |
| 1228 | - // Fallback for unknown conditions | |
| 1229 | - return "AND created_at >= DATE_SUB(NOW(), INTERVAL 60 DAY) | |
| 1230 | - AND created_at < DATE_SUB(NOW(), INTERVAL 30 DAY)"; | |
| 1344 | + // No interval means no window — 'all'. Comparing every record ever | |
| 1345 | + // against a fabricated 30-day slice is not a trend. | |
| 1346 | + return null; | |
| 1231 | 1347 | } |
| 1232 | 1348 | } |
