Skip to content

Tweak cron log message#3146

Merged
Crabcyborg merged 2 commits into
masterfrom
tweak_cron_log_message
Jun 12, 2026
Merged

Tweak cron log message#3146
Crabcyborg merged 2 commits into
masterfrom
tweak_cron_log_message

Conversation

@Crabcyborg

@Crabcyborg Crabcyborg commented Jun 12, 2026

Copy link
Copy Markdown
Contributor

Summary by CodeRabbit

  • Chores
    • Updated subscription cron log messages for clearer wording.
  • New Features
    • Backend now retrieves subscriptions with "active" or "future_cancel" statuses when listing active subscriptions.

@coderabbitai

coderabbitai Bot commented Jun 12, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

Added FrmTransLiteSubscription::get_active_subscriptions() to fetch subscriptions with status 'active' or 'future_cancel', and updated run_payment_cron() log messages from "Stripe Cron Message" to "Overdue Subscription Cron Message" for the initial subscriptions-found line and each per-subscription log.

Changes

Subscription cron and query

Layer / File(s) Summary
Active subscriptions query
stripe/models/FrmTransLiteSubscription.php
Adds get_active_subscriptions() that returns rows from frm_subscriptions where status is active or future_cancel.
Cron logging message updates
stripe/controllers/FrmTransLiteAppController.php
Replaces "Stripe Cron Message" with "Overdue Subscription Cron Message" in run_payment_cron() for the initial count log and the per-subscription $log_message log.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐰
I dug a tidy tunnel through code and time,
Found active subs and made their logs rhyme.
From "Stripe" to "Overdue" the message hops—
A clearer trail where the cron job stops.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Title check ⚠️ Warning The title 'Tweak cron log message' is only partially related to the changeset; it accurately describes changes in FrmTransLiteAppController but omits the significant addition of the get_active_subscriptions() method in FrmTransLiteSubscription. Revise the title to reflect both major changes, such as 'Add get_active_subscriptions method and update cron log messages' to provide a complete picture of the changeset.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch tweak_cron_log_message

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.

❤️ Share

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

@deepsource-io

deepsource-io Bot commented Jun 12, 2026

Copy link
Copy Markdown

DeepSource Code Review

We reviewed changes in c2467d2...6bd5170 on this pull request. Below is the summary for the review, and you can see the individual issues we found as inline review comments.

See full review on DeepSource ↗

PR Report Card

Overall Grade   Security  

Reliability  

Complexity  

Hygiene  

Code Review Summary

Analyzer Status Updated (UTC) Details
PHP Jun 12, 2026 6:32p.m. Review ↗
JavaScript Jun 12, 2026 6:32p.m. Review ↗

Important

AI Review is run only on demand for your team. We're only showing results of static analysis review right now. To trigger AI Review, comment @deepsourcebot review on this thread.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🧹 Nitpick comments (1)
stripe/controllers/FrmTransLiteAppController.php (1)

66-68: ⚡ Quick win

Consider short-circuiting the active subscriptions query.

The current logic calls get_active_subscriptions() even when $overdue_subscriptions is non-empty. Since the early return only happens when both conditions are true, you could optimize by checking if overdue subscriptions exist first:

if ( ! $overdue_subscriptions && ! $frm_sub->get_active_subscriptions() ) {
    return;
}

Could be optimized to:

if ( ! $overdue_subscriptions ) {
    if ( ! $frm_sub->get_active_subscriptions() ) {
        return;
    }
}

This avoids querying active subscriptions when there are already overdue subscriptions to process.

⚡ Proposed optimization
-if ( ! $overdue_subscriptions && ! $frm_sub->get_active_subscriptions() ) {
-    return;
-}
+if ( ! $overdue_subscriptions ) {
+    if ( ! $frm_sub->get_active_subscriptions() ) {
+        return;
+    }
+}
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@stripe/controllers/FrmTransLiteAppController.php` around lines 66 - 68,
Change the conditional so you short-circuit the call to
FrmSubscription::get_active_subscriptions(): check $overdue_subscriptions first
and only call $frm_sub->get_active_subscriptions() when $overdue_subscriptions
is empty; i.e., replace the single combined if ( ! $overdue_subscriptions && !
$frm_sub->get_active_subscriptions() ) return; with a two-step check that
returns early if $overdue_subscriptions is empty and
$frm_sub->get_active_subscriptions() also returns false, thereby avoiding
unnecessary calls to get_active_subscriptions().
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@stripe/controllers/FrmTransLiteAppController.php`:
- Around line 66-68: Change the conditional so you short-circuit the call to
FrmSubscription::get_active_subscriptions(): check $overdue_subscriptions first
and only call $frm_sub->get_active_subscriptions() when $overdue_subscriptions
is empty; i.e., replace the single combined if ( ! $overdue_subscriptions && !
$frm_sub->get_active_subscriptions() ) return; with a two-step check that
returns early if $overdue_subscriptions is empty and
$frm_sub->get_active_subscriptions() also returns false, thereby avoiding
unnecessary calls to get_active_subscriptions().

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: d535c834-5953-4eae-894f-503fafb0d0ac

📥 Commits

Reviewing files that changed from the base of the PR and between 32c63a0 and 6bd5170.

📒 Files selected for processing (2)
  • stripe/controllers/FrmTransLiteAppController.php
  • stripe/models/FrmTransLiteSubscription.php

@Crabcyborg
Crabcyborg merged commit 005a462 into master Jun 12, 2026
21 of 22 checks passed
@Crabcyborg
Crabcyborg deleted the tweak_cron_log_message branch June 12, 2026 18:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant