add a sql db function that will retrieve a entry and its fields as ro…#2524
add a sql db function that will retrieve a entry and its fields as ro…#2524
Conversation
WalkthroughAdds a new static method FrmEntry::get_entries_with_fields that pivots entry meta values into columns per item_id for given field IDs and aliases, optionally filtered by a WHERE clause. It validates inputs, builds dynamic SELECT with MAX(CASE ...) expressions, groups by item_id, and returns query results. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Caller
participant FrmEntry
participant FrmDb
participant DB as wp_frm_item_metas
Caller->>FrmEntry: get_entries_with_fields(field_ids_and_aliases, where)
alt invalid input
FrmEntry-->>Caller: []
else valid input
FrmEntry->>FrmEntry: build SELECT with MAX(CASE...) AS aliases
FrmEntry->>FrmDb: prepend_and_or_where(where)
FrmDb-->>FrmEntry: normalized WHERE clause
FrmEntry->>DB: SELECT item_id, MAX(CASE ...) AS alias... WHERE field_id IN (...) AND <where> GROUP BY item_id
DB-->>FrmEntry: rows
FrmEntry-->>Caller: result rows
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
classes/models/FrmEntry.php (2)
541-606: Consider performance implications for large datasets.The method has no LIMIT clause or pagination support, which could return large result sets and cause performance issues. Additionally, the pivot query with multiple MAX(CASE WHEN) expressions can be expensive on large datasets.
Consider:
- Adding pagination parameters (limit, offset)
- Adding a cache layer for frequently accessed pivots
- Documenting performance characteristics in the PHPDoc
584-605: Document the double-prepare pattern for clarity
This two-stagewpdb->prepareapproach mirrors FrmDb.php (line 443); either refactor to a singlepreparecall for consistency or explicitly comment its intent where used.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
classes/models/FrmEntry.php(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
classes/models/FrmEntry.php (1)
classes/models/FrmDb.php (2)
FrmDb(6-759)prepend_and_or_where(570-590)
🔇 Additional comments (1)
classes/models/FrmEntry.php (1)
580-582: LGTM! Input validation is appropriate.The validation correctly handles empty input and non-array values, returning an empty array as a safe default.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (3)
classes/models/FrmEntry.php (3)
587-591: Verify WordPress version compatibility for %i placeholder.The
%iidentifier placeholder was introduced in WordPress 6.2. If this plugin supports earlier versions, this will cause prepare errors.#!/bin/bash # Check the minimum WordPress version requirement # Check readme files fd -e txt -e md 'readme' --exec grep -i "requires at least" {} \; # Check main plugin file fd -e php --max-depth 2 -x grep -n "Requires at least" {} \;If the plugin supports WP < 6.2, either update "Requires at least" to 6.2 or replace
%iwith manual escaping after validation.
560-560: Update the @SInCE version placeholder.Replace
x.xwith the actual version number.
573-573: Document WHERE clause sanitization requirements.The
$whereparameter is passed toFrmDb::prepend_and_or_where, which only sanitizes array values—string values are NOT automatically sanitized and must be pre-prepared by the caller to prevent SQL injection.Apply this diff:
- * @param string|array $where The where SQL clause to add to the query. + * @param string|array $where The WHERE SQL clause to add to the query. + * SECURITY: If passing a string, it must be pre-sanitized/prepared. + * If passing an array, use FrmDb format: array('field' => 'value'). + * String values are NOT automatically sanitized.
🧹 Nitpick comments (1)
classes/models/FrmEntry.php (1)
586-586: Consider rejecting invalid aliases instead of silently sanitizing.The current approach strips invalid characters, which could lead to unexpected column names (e.g.,
"my-column"becomes"mycolumn"). Consider validating and returning an empty array for invalid aliases to make errors more obvious.Optional refactor:
$select_case_parts = array(); foreach ( $field_ids_and_aliases as $fid => $alias ) { - $safe_alias = preg_replace( '/[^a-zA-Z0-9_]/', '', $alias ); + // Validate alias as a safe identifier (alphanumeric and underscores only) + if ( ! preg_match( '/^[a-zA-Z_][a-zA-Z0-9_]*$/', $alias ) ) { + return array(); + } $select_case_parts[] = $wpdb->prepare( 'MAX(CASE WHEN field_id = %d THEN meta_value END) AS %i', absint( $fid ), - $safe_alias + $alias ); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
classes/models/FrmEntry.php(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
classes/models/FrmEntry.php (1)
classes/models/FrmDb.php (2)
FrmDb(6-759)prepend_and_or_where(570-590)
🔇 Additional comments (2)
classes/models/FrmEntry.php (2)
580-582: LGTM!Input validation correctly guards against empty or non-array input.
605-607: LGTM!Query execution properly uses prepared statements, and returning results directly is safe as
$wpdb->get_resultsreturns an empty array on failure.
…gy11/formidable-forms into feature/new-sql-entry-db-function
2 participants
No description provided.