Skip to content

bug fix and feature update for logs#97

Merged
alexsmithext merged 6 commits intomasterfrom
Log_Management
Oct 9, 2025
Merged

bug fix and feature update for logs#97
alexsmithext merged 6 commits intomasterfrom
Log_Management

Conversation

@jm-extend
Copy link
Copy Markdown
Collaborator

@jm-extend jm-extend commented Oct 2, 2025

added a toggle for logging. this way we don't saturate the logs and we have fine control on the process. changed the logic around checking for store ID and isolate the check for storeID instead of having 2 checks. Also fixed a decimal rounding issue.

Summary by CodeRabbit

  • New Features

    • Added a global logging toggle in Admin settings to enable/disable general logs.
    • Logging status is now passed to the frontend for consistent behavior across the app.
  • Bug Fixes

    • Improved price-to-cents conversion by rounding values, reducing penny discrepancies on orders.
  • Chores

    • Cleaned up unnecessary console output during post-purchase flows and adjusted messaging for missing configuration to be clearer.

added a toggle for logging. this way we don't saturate the logs and we have fine control on the process.
changed the logic around checking for store ID and isolate the check for storeID instead of having 2 checks.
Also fixed a decimal rounding issue.
@jm-extend jm-extend requested a review from alexsmithext October 2, 2025 19:22
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Oct 2, 2025

Important

Review skipped

Auto incremental reviews are disabled on this repository.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Note

Other AI code review bot(s) detected

CodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review.

Walkthrough

Introduces a global logging toggle across admin, backend, and frontend. Admin settings gain a new enable_helloextend_log option with a checkbox, default persisted and localized to JS. Global initialization removes console logging around leadToken and updates the store_id missing message. Logger class gains a settings store and guards all log methods and AJAX logging based on enable_helloextend_log (and debug on enable_helloextend_debug). Orders price-to-cents conversion now uses round((float)price * 100) before int cast. Frontend JS reads and exposes logEnabled via ExtendWooCommerce localization.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Poem

A whisker twitch, a toggle flips—hooray!
Logs now hop on only when we say.
Quiet console, carrot-bright control,
Cents are rounded, tidy as a hole.
I thump approval with soft delight—
Flags aligned, our burrow’s logging right.
🥕✨

Pre-merge checks and finishing touches

❌ Failed checks (2 warnings)
Check name Status Explanation Resolution
Title Check ⚠️ Warning The title focuses solely on logging changes, but the PR also alters store ID validation logic and fixes decimal rounding in price conversion, so it misrepresents and omits significant parts of the changeset. Rename the PR to concisely summarize all key changes, for example “Add global log toggle, refine store ID check, and fix price rounding” so the title accurately reflects the full scope.
Docstring Coverage ⚠️ Warning Docstring coverage is 40.00% which is insufficient. The required threshold is 80.00%. You can run `@coderabbitai generate docstrings` to improve docstring coverage.
✅ Passed checks (1 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit's high-level summary is enabled.

Comment @coderabbitai help to get the list of available commands and usage tips.

cursor[bot]

This comment was marked as outdated.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Disabled knowledge base sources:

  • Jira integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 6c56798 and fc0c687.

📒 Files selected for processing (5)
  • helloextend-protection/admin/class-helloextend-protection-admin.php (5 hunks)
  • helloextend-protection/includes/class-helloextend-global.php (2 hunks)
  • helloextend-protection/includes/class-helloextend-protection-logger.php (4 hunks)
  • helloextend-protection/includes/class-helloextend-protection-orders.php (1 hunks)
  • helloextend-protection/js/helloextend-global.js (2 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
helloextend-protection/includes/class-helloextend-protection-logger.php (1)
helloextend-protection/includes/class-helloextend-global.php (2)
  • HelloExtend_Protection_Global (18-560)
  • helloextend_get_settings (136-263)
helloextend-protection/includes/class-helloextend-global.php (1)
helloextend-protection/includes/class-helloextend-protection-logger.php (2)
  • HelloExtend_Protection_Logger (17-600)
  • helloextend_log_error (28-69)
🪛 PHPMD (2.15.0)
helloextend-protection/includes/class-helloextend-global.php

421-421: Avoid unused local variables such as '$safe_lead_token'. (undefined)

(UnusedLocalVariable)

🔇 Additional comments (14)
helloextend-protection/includes/class-helloextend-protection-orders.php (1)

159-159: LGTM! Correct rounding fix.

The change from direct cast to rounding before int conversion prevents precision loss from floating-point arithmetic (e.g., 10.995 * 100 = 1099.5 now correctly rounds to 1100 instead of truncating to 1099).

helloextend-protection/includes/class-helloextend-global.php (2)

189-190: LGTM! New logging toggle added correctly.

The enable_helloextend_log setting follows the same pattern as existing settings with an appropriate default of 0 (disabled).


411-429: LGTM! Control flow simplified and console logging removed.

The refactored logic correctly isolates the store_id check and removes inline console logging of leadToken values, which aligns with the PR objective to prevent log saturation. The post-purchase flow is preserved when a leadToken is present.

helloextend-protection/js/helloextend-global.js (1)

7-7: LGTM! Logging flag properly exposed to frontend.

The new logEnabled flag is correctly extracted from localized data and exposed on the global ExtendWooCommerce object, following the same pattern as debugLogEnabled.

Also applies to: 94-95

helloextend-protection/includes/class-helloextend-protection-logger.php (5)

30-32: LGTM! Logging guard correctly implemented.

The early-return guard respects the enable_helloextend_log setting and prevents error logs from being written when logging is disabled.


77-79: LGTM! Logging guard correctly implemented.

The early-return guard prevents notice logs from being written when general logging is disabled.


126-131: LGTM! Debug logging guards correctly implemented.

The double guard ensures debug logs are only written when both general logging (enable_helloextend_log) and debug logging (enable_helloextend_debug) are enabled, which is the correct behavior.


585-587: LGTM! AJAX logging guard correctly implemented.

The early-return guard in the AJAX handler respects the enable_helloextend_log setting, preventing logs from being written via AJAX when logging is disabled.


19-19: Initialization order is correct. The logger is instantiated via new HelloExtend_Protection_Logger() (line 602 of includes/class-helloextend-protection-logger.php) during plugin startup (helloextend_run()) before any hooked callbacks invoke its static methods, so self::$settings is always initialized.

Likely an incorrect or invalid review comment.

helloextend-protection/admin/class-helloextend-protection-admin.php (5)

164-164: LGTM! Logging flag localized to frontend.

The enable_helloextend_log setting is correctly extracted and passed to the frontend via script localization, following the same pattern as enable_helloextend_debug.

Also applies to: 169-169


485-491: LGTM! Settings field registered correctly.

The enable_helloextend_log field is properly added to the general settings section, following the plugin's settings registration pattern.


581-581: LGTM! Default value set correctly.

The default value of '0' (disabled) is appropriate for the new logging feature, requiring explicit opt-in.


696-698: LGTM! Input sanitization added correctly.

The sanitization logic for enable_helloextend_log follows the same pattern as other checkbox settings in the plugin.


1243-1250: LGTM! Callback renders checkbox correctly.

The enable_helloextend_log_callback method properly renders the checkbox control, following the same pattern as other toggle settings.

Comment thread helloextend-protection/includes/class-helloextend-global.php Outdated
added an initialize function to make sure the settings is never undefined.
added log_enabled and debug_log_enabled to the compact function
cursor[bot]

This comment was marked as outdated.

removed unused variable
coderabbitai[bot]
coderabbitai Bot previously approved these changes Oct 2, 2025
removed a circular reference
coderabbitai[bot]
coderabbitai Bot previously approved these changes Oct 2, 2025
Comment thread helloextend-protection/js/helloextend-global.js
@jm-extend jm-extend requested a review from alexsmithext October 2, 2025 20:36
alexsmithext
alexsmithext previously approved these changes Oct 7, 2025
@alexsmithext alexsmithext dismissed stale reviews from coderabbitai[bot] and themself via 665a5ad October 9, 2025 20:10
@alexsmithext alexsmithext merged commit 3f82f22 into master Oct 9, 2025
6 checks passed
@alexsmithext alexsmithext deleted the Log_Management branch October 9, 2025 20:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants