Conversation
WalkthroughThis update adds a user rating feature to a WordPress theme integrated with CiviCRM. It introduces a shortcode that renders a star rating interface linked to an activity, AJAX handlers to process rating submissions, and uses CiviCRM API4 to update related entity data accordingly. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Browser
participant WordPress
participant CiviCRM
User->>Browser: Loads page with [goonj_user_rating] shortcode
Browser->>WordPress: Requests shortcode content
WordPress->>CiviCRM: Fetches activity data by activity ID
CiviCRM-->>WordPress: Returns activity data
WordPress-->>Browser: Renders user-rating.php template
User->>Browser: Clicks a star rating (1-10)
Browser->>WordPress: AJAX POST (update_user_rating, rating, entity_id)
WordPress->>CiviCRM: Updates User_Rating field via API4
CiviCRM-->>WordPress: Returns update status
WordPress-->>Browser: Responds with JSON (success/error)
Browser-->>User: Displays message
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
✨ Finishing Touches
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: 5
🧹 Nitpick comments (2)
wp-content/themes/goonj-crm/templates/user-rating.php (1)
23-52: Inline<script>breaks caching & CSP; enqueue a dedicated file insteadPlace this code in
assets/js/user-rating.js, enqueue it viawp_enqueue_scripts, and localizeajaxurl,nonce, andentity_idwithwp_localize_script.
This keeps templates clean and respects WP asset-loading best practices.wp-content/themes/goonj-crm/functions.php (1)
858-869: Redundantwp_die()afterwp_send_json_*
wp_send_json_success()/wp_send_json_error()already callwp_die()internally, so the explicit call at the end is dead code.- wp_die(); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
wp-content/themes/goonj-crm/functions.php(1 hunks)wp-content/themes/goonj-crm/templates/user-rating.php(1 hunks)
| // Prepare data for AJAX request | ||
| var data = { | ||
| action: 'update_user_rating', | ||
| rating: rating, | ||
| entity_id: 7323 // Hardcoded ID as per requirement | ||
| }; | ||
|
|
||
| // AJAX request to call the API | ||
| jQuery.ajax({ | ||
| url: '<?php echo admin_url('admin-ajax.php'); ?>', | ||
| type: 'POST', | ||
| data: data, | ||
| success: function(response) { |
There was a problem hiding this comment.
AJAX request is missing a WordPress nonce – CSRF vulnerability
Any third-party site can silently POST to admin-ajax.php while the user is logged-in and alter ratings.
Add a nonce to the request and verify it in update_user_rating_callback.
- var data = {
- action: 'update_user_rating',
- rating: rating,
- entity_id: <?php echo (int) $entity_id; ?> // …
- };
+ var data = {
+ action: 'update_user_rating',
+ _ajax_nonce: '<?php echo wp_create_nonce( "goonj_user_rating" ); ?>',
+ rating: rating,
+ entity_id: <?php echo (int) $entity_id; ?>
+ };Then in PHP:
check_ajax_referer( 'goonj_user_rating' );🤖 Prompt for AI Agents
In wp-content/themes/goonj-crm/templates/user-rating.php around lines 28 to 40,
the AJAX request lacks a WordPress nonce, exposing a CSRF vulnerability. Fix
this by generating a nonce in PHP using wp_create_nonce('goonj_user_rating') and
passing it to the JavaScript. Then include this nonce in the AJAX data object.
Finally, in the PHP callback function update_user_rating_callback, verify the
nonce by calling check_ajax_referer('goonj_user_rating') before processing the
request.
| if (!isset($_POST['rating']) || !isset($_POST['entity_id'])) { | ||
| wp_send_json_error(['message' => 'Invalid input']); | ||
| wp_die(); | ||
| } | ||
|
|
||
| $rating = intval($_POST['rating']); | ||
| $entity_id = intval($_POST['entity_id']); | ||
|
|
||
| // Validate rating (1 to 10) | ||
| if ($rating < 1 || $rating > 10) { | ||
| wp_send_json_error(['message' => 'Rating must be between 1 and 10']); | ||
| wp_die(); | ||
| } | ||
|
|
There was a problem hiding this comment.
update_user_rating_callback lacks nonce & capability checks
Add check_ajax_referer( 'goonj_user_rating' ) to stop CSRF and optionally verify the current user can update the specified entity.
check_ajax_referer( 'goonj_user_rating' );
if ( ! current_user_can( 'read' ) ) {
wp_send_json_error( [ 'message' => 'Permission denied' ], 403 );
}🤖 Prompt for AI Agents
In wp-content/themes/goonj-crm/functions.php around lines 842 to 855, the
update_user_rating_callback function lacks security checks against CSRF and
unauthorized access. Add a call to check_ajax_referer('goonj_user_rating') at
the start of the function to verify the nonce. Then, add a capability check
using current_user_can('read') or a more specific capability to ensure the user
has permission to update the rating. If the check fails, respond with
wp_send_json_error including a 'Permission denied' message and a 403 status
code.
| $activities = \Civi\Api4\Activity::get(FALSE) | ||
| ->addSelect( | ||
| 'Material_Contribution.Collection_Camp', | ||
| 'Material_Contribution.Institution_Collection_Camp', | ||
| 'Material_Contribution.Dropping_Center', | ||
| 'Material_Contribution.Institution_Dropping_Center', | ||
| 'Material_Contribution.Goonj_Office', | ||
| 'Office_Visit.Goonj_Processing_Center', | ||
| 'Material_Contribution.Event' | ||
| ) | ||
| ->addWhere('id', '=', $activity_id) | ||
| ->execute() | ||
| ->first(); | ||
| error_log('Activities: ' . print_r($activities, true)); | ||
| include(get_template_directory() . '/templates/user-rating.php'); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Fetched $activities is discarded and $entity_id never reaches the template
You fetch activity data but never pass the resulting entity/collection-camp ID to the template; meanwhile the template hard-codes 7323.
Populate $entity_id from $activities and expose it to the template:
- $activities = \Civi\Api4\Activity::get(FALSE)
+ $activities = \Civi\Api4\Activity::get(FALSE)
/* … */
- ->first();
- error_log('Activities: ' . print_r($activities, true));
- include(get_template_directory() . '/templates/user-rating.php');
+ ->first();
+
+ if ( empty( $activities ) ) {
+ \Civi::log()->warning( 'Activity not found', [ 'id' => $activity_id ] );
+ return ob_get_clean();
+ }
+
+ // Derive the related Collection_Camp ID (example, adjust as per schema)
+ $entity_id = $activities['Material_Contribution.Collection_Camp'] ?? 0;
+
+ include get_template_directory() . '/templates/user-rating.php';🤖 Prompt for AI Agents
In wp-content/themes/goonj-crm/functions.php around lines 820 to 834, the
fetched $activities data is not used to set $entity_id, causing the template to
use a hard-coded value. Extract the relevant entity or collection camp ID from
$activities, assign it to $entity_id, and ensure $entity_id is available to the
included user-rating.php template so it uses dynamic data instead of a fixed ID.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
wp-content/themes/goonj-crm/templates/user-rating.php (1)
90-96:⚠️ Potential issueCSRF protection is still missing – nonce must be included
The AJAX payload is dispatched without a WordPress nonce, leaving the endpoint vulnerable to CSRF attacks. This is exactly the issue flagged in the earlier review and remains unfixed.var data = { action: 'update_user_rating', + _ajax_nonce: '<?php echo wp_create_nonce( "goonj_user_rating" ); ?>', rating: rating, entity_id: <?php echo $entity_id; ?> };Add
check_ajax_referer( 'goonj_user_rating' );at the top ofupdate_user_rating_callback.
🧹 Nitpick comments (2)
wp-content/themes/goonj-crm/templates/user-rating.php (2)
36-37: Avoiderror_login production templates
Rawerror_log()calls leak internal state to PHP error logs and clutter logs in production. Use\Civi::log()->debug()(with appropriate log level) or remove once debugging is complete.Also applies to: 66-66
68-83: Escape dynamic output in HTML
<?php echo $i; ?>is safe here, but follow WordPress convention and wrap withesc_html( $i )/esc_attr(). Future edits will inherit safe patterns.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
wp-content/themes/goonj-crm/functions.php(1 hunks)wp-content/themes/goonj-crm/templates/user-rating.php(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- wp-content/themes/goonj-crm/functions.php
🔇 Additional comments (1)
wp-content/themes/goonj-crm/templates/user-rating.php (1)
85-85: Ensure jQuery is enqueued
The script assumesjQueryis present. Addwp_enqueue_script( 'jquery' );when registering the shortcode to avoid “undefined jQuery” errors on themes that dequeue it.
| try { | ||
| // Fetch activity details. | ||
| $activities = Activity::get(FALSE) | ||
| ->addSelect( | ||
| 'Material_Contribution.Collection_Camp', | ||
| 'Material_Contribution.Institution_Collection_Camp', | ||
| 'Material_Contribution.Dropping_Center', | ||
| 'Material_Contribution.Institution_Dropping_Center', | ||
| 'Material_Contribution.Goonj_Office', | ||
| 'Office_Visit.Goonj_Processing_Center', | ||
| 'Material_Contribution.Event' | ||
| ) | ||
| ->addWhere('id', '=', $activity_id) | ||
| ->execute() | ||
| ->first(); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Template mixes heavy data access with presentation – violates SRP
The CiviCRM API query (including multiple joins) is executed directly inside the template. This couples database logic to markup and hampers testability.
Move the lookup into a helper (get_entity_id_from_activity( $activity_id )) in functions.php (or a separate service class) and pass the resolved $entity_id to the template.
🤖 Prompt for AI Agents
In wp-content/themes/goonj-crm/templates/user-rating.php around lines 20 to 34,
the code directly performs a complex data query inside the template, mixing data
access with presentation and violating the Single Responsibility Principle. To
fix this, move the entire data fetching logic into a helper function named
get_entity_id_from_activity($activity_id) located in functions.php or a
dedicated service class. Then, modify the template to accept the pre-fetched
$entity_id as a parameter, removing all database queries from the template file.
| success: function(response) { | ||
| if (response.success) { | ||
| document.getElementById('rating-message').innerHTML = '<p style="color: green;">Rating updated successfully!</p>'; | ||
| } else { | ||
| document.getElementById('rating-message').innerHTML = '<p style="color: red;">Error: ' + response.data.message + '</p>'; | ||
| } | ||
| }, |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Potential XSS – server message is written with innerHTML
response.data.message is interpolated directly into the DOM with innerHTML. If the server ever echoes unsanitised user-supplied data, you have an XSS vector.
- document.getElementById('rating-message').innerHTML =
- '<p style="color: red;">Error: ' + response.data.message + '</p>';
+ document.getElementById('rating-message').textContent =
+ 'Error: ' + response.data.message;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| success: function(response) { | |
| if (response.success) { | |
| document.getElementById('rating-message').innerHTML = '<p style="color: green;">Rating updated successfully!</p>'; | |
| } else { | |
| document.getElementById('rating-message').innerHTML = '<p style="color: red;">Error: ' + response.data.message + '</p>'; | |
| } | |
| }, | |
| success: function(response) { | |
| if (response.success) { | |
| document.getElementById('rating-message').innerHTML = '<p style="color: green;">Rating updated successfully!</p>'; | |
| } else { | |
| document.getElementById('rating-message').textContent = 'Error: ' + response.data.message; | |
| } | |
| }, |
🤖 Prompt for AI Agents
In wp-content/themes/goonj-crm/templates/user-rating.php around lines 101 to
107, the code inserts server response messages directly into the DOM using
innerHTML, which risks XSS if the message contains unsanitized user input. To
fix this, replace innerHTML with textContent or another safe method to insert
the message text, ensuring that any HTML tags in response.data.message are not
interpreted as code.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (3)
wp-content/themes/goonj-crm/templates/user-rating.php (3)
20-34: Heavy DB logic still embedded in template – violates SRPThe entire
Activity::get()query remains inside the presentation layer. This couples markup to data access and makes the template hard to test or reuse. Please move this lookup to a helper/ service infunctions.php(or similar) and pass the resolved$entity_idwhen rendering the template.
112-118: AJAX request is still missing a WordPress nonce → CSRF riskPrevious review already flagged this. A malicious site can POST to
admin-ajax.phpand change ratings.
Add a nonce to the data payload and verify it in the PHP callback.- const data = { - action: 'update_user_rating', - rating: rating, - entity_id: <?php echo $entity_id; ?> // Dynamically set from activity - }; + const data = { + action: 'update_user_rating', + _ajax_nonce: '<?php echo wp_create_nonce( "goonj_user_rating" ); ?>', + rating: rating, + entity_id: <?php echo $entity_id; ?> + };And in
update_user_rating_callback:check_ajax_referer( 'goonj_user_rating' );
126-134: Potential XSS – replaceinnerHTMLwithtextContent
response.data.messageis written viainnerHTML, allowing an attacker-supplied string to inject markup/JS. UsetextContentinstead.- document.getElementById('rating-message').innerHTML = - '<p style="color: green;">Rating updated successfully!</p>'; + document.getElementById('rating-message').textContent = + 'Rating updated successfully!'; - document.getElementById('rating-message').innerHTML = - '<p style="color: red;">Error: ' + response.data.message + '</p>'; + document.getElementById('rating-message').textContent = + 'Error: ' + response.data.message;
| error_log('Activities: ' . print_r($activities, TRUE)); | ||
|
|
There was a problem hiding this comment.
Verbose error_log() dumps may leak PII in production logs
print_r($activities, TRUE) and unconditional logging of $entity_id can expose sensitive donor/camp data. Remove or gate behind a debug flag.
-error_log('Activities: ' . print_r($activities, TRUE));
-...
-error_log('Entity ID: ' . $entity_id);
+// error_log(...) // Remove or wrap with WP_DEBUG checkAlso applies to: 66-66
🤖 Prompt for AI Agents
In wp-content/themes/goonj-crm/templates/user-rating.php at lines 36-37 and line
66, the error_log calls unconditionally output detailed data including
$activities and $entity_id, which may expose sensitive information. To fix this,
wrap these error_log statements inside a conditional check for a debug flag or
environment variable that is only enabled in development or debugging scenarios,
ensuring no sensitive data is logged in production.
| foreach ($fields as $field) { | ||
| if (!empty($activities[$field])) { | ||
| $entity_id = intval($activities[$field]); | ||
| break; | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
Possible fatal error when $activities is null
If no activity is found, $activities is null, so $activities[$field] triggers a warning. Bail out early:
-if (!empty($activities[$field])) {
+if ($activities && !empty($activities[$field])) {Or check $activities === null right after the query and handle gracefully.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| foreach ($fields as $field) { | |
| if (!empty($activities[$field])) { | |
| $entity_id = intval($activities[$field]); | |
| break; | |
| } | |
| } | |
| foreach ($fields as $field) { | |
| if ($activities && !empty($activities[$field])) { | |
| $entity_id = intval($activities[$field]); | |
| break; | |
| } | |
| } |
🤖 Prompt for AI Agents
In wp-content/themes/goonj-crm/templates/user-rating.php around lines 49 to 55,
the code accesses $activities[$field] without checking if $activities is null,
which can cause a fatal error. Fix this by adding a check before the foreach
loop to ensure $activities is not null, or handle the case where $activities is
null immediately after the query to avoid accessing it when null.
1 participant
Rating
Summary by CodeRabbit