Track when message settings changed#2232
Conversation
WalkthroughThe pull request modifies the Changes
Possibly related PRs
Suggested labels
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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 (
|
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
classes/models/FrmUsage.php (1)
261-261: Consider improving readability of the comparison logic.While the current implementation is correct, the ternary operator could be made more readable.
Consider this alternative:
- $message_settings[ 'changed-' . $message ] = $settings_list->$message === $default[ $message ] ? 0 : 1; + $is_changed = $settings_list->$message !== $default[ $message ]; + $message_settings[ 'changed-' . $message ] = $is_changed ? 1 : 0;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
classes/models/FrmUsage.php(1 hunks)
🔇 Additional comments (1)
classes/models/FrmUsage.php (1)
257-261: LGTM! Implementation aligns well with tracking objectives.The changes effectively track message setting modifications by comparing against default values. This implementation cleanly addresses the PR's goal of tracking when message settings are changed.
This was reported by Mike via private message in Slack.
In Lite and Pro plugins, we send the
Message Defaultssettings to the usage endpoint. But in the new usage tracking endpoint, I skipped those settings because they are long and I didn't think we wanted to track them.Our purpose is to track when those settings are changed. So I created this PR. I will send
changed-{$message_key}to the endpoint. It has the value0(not changed) or1(changed).In the usage endpoint, I removed the code that skips the messages: https://github.com/Strategy11/ff-usage-2/pull/2
The similar PR in Pro: https://github.com/Strategy11/formidable-pro/pull/5603