Add MODEL-FETCH-DATE-01 rule and migrate to client-side date formatting
🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
This commit is contained in:
root
237
app/RSpade/CodeQuality/Rules/Models/ModelFetchDateFormatting_CodeQualityRule.php
Executable file
237
app/RSpade/CodeQuality/Rules/Models/ModelFetchDateFormatting_CodeQualityRule.php
Executable file
@@ -0,0 +1,237 @@
|
||||
<?php
|
||||
|
||||
namespace App\RSpade\CodeQuality\Rules\Models;
|
||||
|
||||
use App\RSpade\CodeQuality\Rules\CodeQualityRule_Abstract;
|
||||
use App\RSpade\Core\Manifest\Manifest;
|
||||
|
||||
/**
|
||||
* Detects server-side date/datetime formatting in model fetch() methods
|
||||
*
|
||||
* Dates and datetimes should be passed to the client as-is (ISO strings for datetimes,
|
||||
* YYYY-MM-DD for dates) and formatted client-side using Rsx_Time and Rsx_Date JavaScript
|
||||
* classes. Server-side formatting creates implicit API contracts and prevents client-side
|
||||
* locale customization.
|
||||
*/
|
||||
class ModelFetchDateFormatting_CodeQualityRule extends CodeQualityRule_Abstract
|
||||
{
|
||||
/**
|
||||
* Get the unique rule identifier
|
||||
*/
|
||||
public function get_id(): string
|
||||
{
|
||||
return 'MODEL-FETCH-DATE-01';
|
||||
}
|
||||
|
||||
/**
|
||||
* Get the rule name
|
||||
*/
|
||||
public function get_name(): string
|
||||
{
|
||||
return 'Model Fetch Date Formatting';
|
||||
}
|
||||
|
||||
/**
|
||||
* Get the rule description
|
||||
*/
|
||||
public function get_description(): string
|
||||
{
|
||||
return 'Detects server-side date/datetime formatting in model fetch() methods';
|
||||
}
|
||||
|
||||
/**
|
||||
* Get file patterns this rule applies to
|
||||
*/
|
||||
public function get_file_patterns(): array
|
||||
{
|
||||
return ['*.php'];
|
||||
}
|
||||
|
||||
/**
|
||||
* Get default severity
|
||||
*/
|
||||
public function get_default_severity(): string
|
||||
{
|
||||
return 'high';
|
||||
}
|
||||
|
||||
/**
|
||||
* Whether this rule runs during manifest scan
|
||||
*/
|
||||
public function is_called_during_manifest_scan(): bool
|
||||
{
|
||||
return true;
|
||||
}
|
||||
|
||||
/**
|
||||
* Run the rule check
|
||||
*/
|
||||
public function check(string $file_path, string $contents, array $metadata = []): void
|
||||
{
|
||||
// Only check PHP files
|
||||
if (!str_ends_with($file_path, '.php')) {
|
||||
return;
|
||||
}
|
||||
|
||||
$class_name = $metadata['class'] ?? null;
|
||||
|
||||
if (!$class_name) {
|
||||
return;
|
||||
}
|
||||
|
||||
// Skip base abstract classes
|
||||
if ($class_name === 'Rsx_Model_Abstract' || $class_name === 'Rsx_Site_Model_Abstract') {
|
||||
return;
|
||||
}
|
||||
|
||||
// Check if this is a model (extends Rsx_Model_Abstract in the inheritance chain)
|
||||
if (!Manifest::php_is_subclass_of($class_name, 'Rsx_Model_Abstract')) {
|
||||
return;
|
||||
}
|
||||
|
||||
// Check if fetch() method exists (can be in static_methods or public_static_methods)
|
||||
$has_fetch = isset($metadata['static_methods']['fetch'])
|
||||
|| isset($metadata['public_static_methods']['fetch']);
|
||||
|
||||
if (!$has_fetch) {
|
||||
return;
|
||||
}
|
||||
|
||||
$lines = explode("\n", $contents);
|
||||
|
||||
// Extract fetch method body
|
||||
$fetch_body = $this->_extract_fetch_body($lines);
|
||||
if (empty($fetch_body)) {
|
||||
return;
|
||||
}
|
||||
|
||||
// Patterns that indicate server-side date formatting
|
||||
$patterns = [
|
||||
// Carbon format() calls - e.g., ->format('M d, Y')
|
||||
'/->format\s*\(\s*[\'"]/' => '->format() call',
|
||||
|
||||
// Carbon diffForHumans() calls - e.g., ->diffForHumans()
|
||||
'/->diffForHumans\s*\(/' => '->diffForHumans() call',
|
||||
|
||||
// Carbon::parse()->format() chain
|
||||
'/Carbon::parse\s*\([^)]+\)\s*->format\s*\(/' => 'Carbon::parse()->format() chain',
|
||||
|
||||
// Rsx_Time formatting (which is fine on its own, but suggests formatted values in response)
|
||||
// We still flag these because they shouldn't be in fetch() - format client-side
|
||||
'/Rsx_Time::format_date(time)?(_with_tz)?\s*\(/' => 'Rsx_Time formatting call',
|
||||
'/Rsx_Time::relative\s*\(/' => 'Rsx_Time::relative() call',
|
||||
|
||||
// Rsx_Date formatting
|
||||
'/Rsx_Date::format\s*\(/' => 'Rsx_Date::format() call',
|
||||
];
|
||||
|
||||
foreach ($fetch_body as $relative_line => $line_content) {
|
||||
// Check for exception comment
|
||||
if (str_contains($line_content, '@' . $this->get_id() . '-EXCEPTION')) {
|
||||
continue;
|
||||
}
|
||||
|
||||
foreach ($patterns as $pattern => $description) {
|
||||
if (preg_match($pattern, $line_content)) {
|
||||
$this->add_violation(
|
||||
$file_path,
|
||||
$relative_line,
|
||||
$this->_get_violation_message($description),
|
||||
trim($line_content),
|
||||
$this->_get_resolution_message(),
|
||||
'high'
|
||||
);
|
||||
break; // Only one violation per line
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Extract the fetch() method body with line numbers
|
||||
*
|
||||
* @param array $lines File lines
|
||||
* @return array [line_number => line_content]
|
||||
*/
|
||||
private function _extract_fetch_body(array $lines): array
|
||||
{
|
||||
$in_fetch = false;
|
||||
$brace_count = 0;
|
||||
$fetch_body = [];
|
||||
$started = false;
|
||||
|
||||
for ($i = 0; $i < count($lines); $i++) {
|
||||
$line = $lines[$i];
|
||||
$line_num = $i + 1;
|
||||
|
||||
// Look for fetch method declaration
|
||||
if (!$in_fetch) {
|
||||
if (preg_match('/\b(public\s+)?static\s+function\s+fetch\s*\(/', $line)) {
|
||||
$in_fetch = true;
|
||||
// Count opening brace if on same line
|
||||
if (str_contains($line, '{')) {
|
||||
$started = true;
|
||||
$brace_count = 1;
|
||||
}
|
||||
}
|
||||
continue;
|
||||
}
|
||||
|
||||
// We're inside fetch method
|
||||
if (!$started) {
|
||||
// Looking for opening brace
|
||||
if (str_contains($line, '{')) {
|
||||
$started = true;
|
||||
$brace_count = 1;
|
||||
}
|
||||
continue;
|
||||
}
|
||||
|
||||
// Track braces to find method end
|
||||
$brace_count += substr_count($line, '{') - substr_count($line, '}');
|
||||
|
||||
if ($brace_count <= 0) {
|
||||
break; // End of fetch method
|
||||
}
|
||||
|
||||
$fetch_body[$line_num] = $line;
|
||||
}
|
||||
|
||||
return $fetch_body;
|
||||
}
|
||||
|
||||
/**
|
||||
* Get the violation message
|
||||
*/
|
||||
private function _get_violation_message(string $pattern_description): string
|
||||
{
|
||||
return "Server-side date formatting detected in fetch() method ({$pattern_description}).\n\n" .
|
||||
"Dates and datetimes should not be pre-formatted for Ajax responses. " .
|
||||
"RSpade stores dates as 'YYYY-MM-DD' strings and datetimes as ISO 8601 UTC strings " .
|
||||
"(e.g., '2024-12-24T15:30:45.123Z'). These values should be passed directly to the " .
|
||||
"client and formatted using JavaScript.\n\n" .
|
||||
"Problems with server-side formatting:\n" .
|
||||
" 1. Creates implicit API contracts - frontend developers may expect these fields\n" .
|
||||
" to exist on all model responses, leading to confusion\n" .
|
||||
" 2. Prevents client-side locale customization\n" .
|
||||
" 3. Increases response payload size with redundant data\n" .
|
||||
" 4. Violates the principle that fetch() returns model data, not presentation";
|
||||
}
|
||||
|
||||
/**
|
||||
* Get the resolution message
|
||||
*/
|
||||
private function _get_resolution_message(): string
|
||||
{
|
||||
return "Remove the formatted date fields from fetch(). Format dates client-side:\n\n" .
|
||||
" JavaScript datetime formatting:\n" .
|
||||
" Rsx_Time.format_datetime(iso_string) // 'Dec 24, 2024 3:30 PM'\n" .
|
||||
" Rsx_Time.format_datetime_with_tz(iso) // 'Dec 24, 2024 3:30 PM CST'\n" .
|
||||
" Rsx_Time.relative(iso_string) // '2 hours ago'\n\n" .
|
||||
" JavaScript date formatting:\n" .
|
||||
" Rsx_Date.format(date_string) // 'Dec 24, 2024'\n\n" .
|
||||
" Example in jqhtml template:\n" .
|
||||
" <span><%= Rsx_Time.relative(this.data.model.created_at) %></span>\n\n" .
|
||||
"See: php artisan rsx:man time";
|
||||
}
|
||||
}
|
||||
312
app/RSpade/upstream_changes/datetime_strings_12_24.txt
Executable file
312
app/RSpade/upstream_changes/datetime_strings_12_24.txt
Executable file
@@ -0,0 +1,312 @@
|
||||
DATE/DATETIME STRING FORMAT - MIGRATION GUIDE
|
||||
Date: 2024-12-24
|
||||
|
||||
SUMMARY
|
||||
RSpade now stores and transmits dates and datetimes as strings throughout
|
||||
the application, not as Carbon objects. This eliminates timezone bugs with
|
||||
date columns and ensures PHP/JavaScript parity with identical formats on
|
||||
both sides.
|
||||
|
||||
- Dates: "YYYY-MM-DD" strings (e.g., "2024-12-24")
|
||||
- Datetimes: ISO 8601 UTC strings (e.g., "2024-12-24T15:30:45.123Z")
|
||||
|
||||
Model properties like $model->created_at now return strings, not Carbon
|
||||
instances. Calling Carbon methods like ->format() or ->diffForHumans() on
|
||||
these properties will cause errors. Use Rsx_Time and Rsx_Date helper classes
|
||||
instead.
|
||||
|
||||
A code quality rule (MODEL-FETCH-DATE-01) now detects server-side date
|
||||
formatting in model fetch() methods at manifest-time. Dates should be
|
||||
formatted client-side using the JavaScript Rsx_Time and Rsx_Date classes.
|
||||
|
||||
WHY THIS CHANGE
|
||||
|
||||
1. Timezone Safety
|
||||
Carbon objects assume a timezone, but DATE columns have no timezone.
|
||||
"2024-12-24" is just a calendar day - not midnight in any timezone.
|
||||
Treating it as Carbon caused subtle bugs when serializing to JSON.
|
||||
|
||||
2. PHP/JavaScript Parity
|
||||
With string-based dates, both PHP and JavaScript see identical values.
|
||||
No serialization surprises, no format mismatches.
|
||||
|
||||
3. Client-Side Formatting
|
||||
Formatting should happen in the browser where the user's locale and
|
||||
timezone are known. Server-side formatting creates implicit API
|
||||
contracts that confuse frontend developers.
|
||||
|
||||
AFFECTED CODE PATTERNS
|
||||
|
||||
1. Model Property Access
|
||||
-------------------------------------------------------------------------
|
||||
Model datetime/date properties now return strings, not Carbon.
|
||||
|
||||
Before (BROKEN):
|
||||
$user->created_at->format('M d, Y');
|
||||
$user->created_at->diffForHumans();
|
||||
$project->due_date->format('Y-m-d');
|
||||
|
||||
After (CORRECT):
|
||||
use App\RSpade\Core\Time\Rsx_Time;
|
||||
use App\RSpade\Core\Time\Rsx_Date;
|
||||
|
||||
Rsx_Time::format_date($user->created_at); // "Dec 24, 2024"
|
||||
Rsx_Time::relative($user->created_at); // "2 hours ago"
|
||||
Rsx_Date::format($project->due_date); // "Dec 24, 2024"
|
||||
|
||||
2. Formatted Fields in fetch() Methods
|
||||
-------------------------------------------------------------------------
|
||||
Model fetch() methods should NOT return pre-formatted date fields.
|
||||
The MODEL-FETCH-DATE-01 rule now blocks this at manifest-time.
|
||||
|
||||
Before (BLOCKED BY CODE QUALITY RULE):
|
||||
public static function fetch($id) {
|
||||
$data = $model->toArray();
|
||||
$data['created_at_formatted'] = $model->created_at->format('M d, Y');
|
||||
$data['created_at_human'] = $model->created_at->diffForHumans();
|
||||
return $data;
|
||||
}
|
||||
|
||||
After (CORRECT):
|
||||
public static function fetch($id) {
|
||||
$data = $model->toArray();
|
||||
// created_at is already included as ISO string from toArray()
|
||||
// DO NOT add formatted versions - format client-side
|
||||
return $data;
|
||||
}
|
||||
|
||||
3. Controller Ajax Responses
|
||||
-------------------------------------------------------------------------
|
||||
Controllers returning date data should pass raw values, not formatted.
|
||||
|
||||
Before:
|
||||
return [
|
||||
'created_at_formatted' => $user->created_at->format('F j, Y'),
|
||||
'updated_at_human' => $user->updated_at->diffForHumans(),
|
||||
];
|
||||
|
||||
After:
|
||||
return [
|
||||
'created_at' => $user->created_at, // ISO string automatically
|
||||
'updated_at' => $user->updated_at,
|
||||
];
|
||||
|
||||
4. Carbon::parse() in PHP
|
||||
-------------------------------------------------------------------------
|
||||
If you need Carbon for calculations, use Rsx_Time::parse() instead.
|
||||
It returns Carbon internally but validates the input format.
|
||||
|
||||
Before:
|
||||
$carbon = \Carbon\Carbon::parse($project->start_date);
|
||||
$days_until = $carbon->diffInDays(now());
|
||||
|
||||
After:
|
||||
$carbon = Rsx_Time::parse($user->created_at); // For datetimes
|
||||
$days_until = $carbon->diffInDays(Rsx_Time::now());
|
||||
|
||||
// For date comparisons:
|
||||
$days = Rsx_Date::diff_days($project->start_date, Rsx_Date::today());
|
||||
|
||||
CLIENT-SIDE FORMATTING
|
||||
|
||||
JavaScript templates should format dates using Rsx_Time and Rsx_Date:
|
||||
|
||||
1. Datetime Formatting (Rsx_Time)
|
||||
-------------------------------------------------------------------------
|
||||
For datetime columns (created_at, updated_at, timestamps):
|
||||
|
||||
Template (jqhtml):
|
||||
<%-- Display formatted date --%>
|
||||
<div><%= Rsx_Time.format_date(this.data.user.created_at) %></div>
|
||||
|
||||
<%-- Display with time --%>
|
||||
<div><%= Rsx_Time.format_datetime(this.data.event.start_time) %></div>
|
||||
|
||||
<%-- Display with timezone --%>
|
||||
<div><%= Rsx_Time.format_datetime_with_tz(this.data.event.start_time) %></div>
|
||||
|
||||
<%-- Relative time (e.g., "2 hours ago") --%>
|
||||
<small><%= Rsx_Time.relative(this.data.user.created_at) %></small>
|
||||
|
||||
2. Date Formatting (Rsx_Date)
|
||||
-------------------------------------------------------------------------
|
||||
For date-only columns (due_date, birth_date, start_date):
|
||||
|
||||
Template (jqhtml):
|
||||
<%-- Format date --%>
|
||||
<div><%= Rsx_Date.format(this.data.project.due_date) %></div>
|
||||
|
||||
<%-- Check if past due --%>
|
||||
<% if (Rsx_Date.is_past(this.data.project.due_date)) { %>
|
||||
<span class="text-danger">Overdue</span>
|
||||
<% } %>
|
||||
|
||||
3. Handling Null Values
|
||||
-------------------------------------------------------------------------
|
||||
Check for null before formatting:
|
||||
|
||||
<div><%= this.data.user.last_login_at
|
||||
? Rsx_Time.format_datetime(this.data.user.last_login_at)
|
||||
: 'Never' %></div>
|
||||
|
||||
MIGRATION STEPS
|
||||
|
||||
Step 1: Find Carbon method calls on model properties
|
||||
-------------------------------------------------------------------------
|
||||
Search for ->format( and ->diffForHumans( on model properties:
|
||||
|
||||
grep -rn "->format\s*(" rsx/ --include="*.php" | grep -v "Rsx_Time"
|
||||
grep -rn "->diffForHumans" rsx/ --include="*.php"
|
||||
|
||||
Step 2: Find formatted date fields in fetch() methods
|
||||
-------------------------------------------------------------------------
|
||||
Search for _formatted and _human field assignments:
|
||||
|
||||
grep -rn "_formatted\s*=" rsx/models/ --include="*.php"
|
||||
grep -rn "_human\s*=" rsx/models/ --include="*.php"
|
||||
|
||||
Step 3: Find Carbon::parse() calls
|
||||
-------------------------------------------------------------------------
|
||||
These may need conversion to Rsx_Time::parse() or Rsx_Date::parse():
|
||||
|
||||
grep -rn "Carbon::parse" rsx/ --include="*.php"
|
||||
|
||||
Step 4: Find template usages of _formatted fields
|
||||
-------------------------------------------------------------------------
|
||||
Templates referencing removed fields need updating:
|
||||
|
||||
grep -rn "_formatted\|_human" rsx/app/ --include="*.jqhtml"
|
||||
grep -rn "_formatted\|_human" rsx/app/ --include="*.js"
|
||||
|
||||
Step 5: Update JavaScript defaults
|
||||
-------------------------------------------------------------------------
|
||||
JS files with this.data defaults for _formatted fields:
|
||||
|
||||
grep -rn "created_at_formatted\|updated_at_formatted" rsx/ --include="*.js"
|
||||
|
||||
EXAMPLE MIGRATION
|
||||
|
||||
Complete example: Converting a view action
|
||||
|
||||
PHP Controller (before):
|
||||
return [
|
||||
'id' => $user->id,
|
||||
'name' => $user->name,
|
||||
'created_at_formatted' => $user->created_at->format('F j, Y'),
|
||||
];
|
||||
|
||||
PHP Controller (after):
|
||||
return [
|
||||
'id' => $user->id,
|
||||
'name' => $user->name,
|
||||
'created_at' => $user->created_at, // Already ISO string
|
||||
];
|
||||
|
||||
JavaScript (before):
|
||||
on_create() {
|
||||
this.data.user = {
|
||||
id: null,
|
||||
name: '',
|
||||
created_at_formatted: '',
|
||||
};
|
||||
}
|
||||
|
||||
JavaScript (after):
|
||||
on_create() {
|
||||
this.data.user = {
|
||||
id: null,
|
||||
name: '',
|
||||
created_at: null,
|
||||
};
|
||||
}
|
||||
|
||||
Template (before):
|
||||
<div><%= this.data.user.created_at_formatted %></div>
|
||||
|
||||
Template (after):
|
||||
<div><%= Rsx_Time.format_date(this.data.user.created_at) %></div>
|
||||
|
||||
CUSTOM ELOQUENT CASTS
|
||||
|
||||
The framework uses custom Eloquent casts to ensure string-based values:
|
||||
|
||||
- Rsx_DateTime_Cast: DATETIME/TIMESTAMP columns return ISO 8601 strings
|
||||
- Rsx_Date_Cast: DATE columns return YYYY-MM-DD strings
|
||||
|
||||
These casts are automatically applied via Rsx_Model_Abstract::getCasts().
|
||||
You do not need to configure anything - model properties automatically
|
||||
return strings.
|
||||
|
||||
If a model needs Laravel's default Carbon behavior for a specific column,
|
||||
override $casts in the model:
|
||||
|
||||
protected $casts = [
|
||||
'special_timestamp' => 'datetime', // Laravel default (Carbon)
|
||||
];
|
||||
|
||||
AVAILABLE HELPER METHODS
|
||||
|
||||
Rsx_Time (for datetimes)
|
||||
-------------------------------------------------------------------------
|
||||
PHP:
|
||||
Rsx_Time::now() // Current time as Carbon
|
||||
Rsx_Time::now_iso() // Current time as ISO string
|
||||
Rsx_Time::parse($input) // Parse to Carbon (for calculations)
|
||||
Rsx_Time::to_iso($time) // Convert to ISO string
|
||||
Rsx_Time::format_date($time) // "Dec 24, 2024"
|
||||
Rsx_Time::format_datetime($time) // "Dec 24, 2024 3:30 PM"
|
||||
Rsx_Time::relative($time) // "2 hours ago"
|
||||
Rsx_Time::add($time, $seconds) // Add seconds, return ISO string
|
||||
Rsx_Time::is_past($time) // Boolean
|
||||
Rsx_Time::is_future($time) // Boolean
|
||||
|
||||
JavaScript:
|
||||
Rsx_Time.now() // Current time as Date
|
||||
Rsx_Time.now_iso() // Current time as ISO string
|
||||
Rsx_Time.format_date(iso) // "Dec 24, 2024"
|
||||
Rsx_Time.format_datetime(iso) // "Dec 24, 2024 3:30 PM"
|
||||
Rsx_Time.format_datetime_with_tz(iso) // "Dec 24, 2024 3:30 PM CST"
|
||||
Rsx_Time.relative(iso) // "2 hours ago"
|
||||
Rsx_Time.countdown($el, target) // Live countdown
|
||||
Rsx_Time.countup($el, start) // Live elapsed time
|
||||
|
||||
Rsx_Date (for dates)
|
||||
-------------------------------------------------------------------------
|
||||
PHP:
|
||||
Rsx_Date::today() // Today as "YYYY-MM-DD"
|
||||
Rsx_Date::parse($input) // Validate and normalize
|
||||
Rsx_Date::format($date) // "Dec 24, 2024"
|
||||
Rsx_Date::is_past($date) // Boolean
|
||||
Rsx_Date::is_future($date) // Boolean
|
||||
Rsx_Date::diff_days($d1, $d2) // Days between dates
|
||||
|
||||
JavaScript:
|
||||
Rsx_Date.today() // Today as "YYYY-MM-DD"
|
||||
Rsx_Date.format(date) // "Dec 24, 2024"
|
||||
Rsx_Date.is_past(date) // Boolean
|
||||
Rsx_Date.is_future(date) // Boolean
|
||||
Rsx_Date.diff_days(d1, d2) // Days between dates
|
||||
|
||||
VERIFICATION
|
||||
|
||||
1. Run code quality check:
|
||||
php artisan rsx:check
|
||||
|
||||
The MODEL-FETCH-DATE-01 rule will detect any remaining server-side
|
||||
date formatting in fetch() methods.
|
||||
|
||||
2. Search for remaining Carbon method calls:
|
||||
grep -rn "->format\s*(" rsx/ --include="*.php" | grep -v vendor
|
||||
|
||||
3. Test pages that display dates:
|
||||
- Verify dates render correctly
|
||||
- Check relative times update properly
|
||||
- Confirm timezone handling is correct
|
||||
|
||||
REFERENCE
|
||||
|
||||
php artisan rsx:man time - Complete time/date API documentation
|
||||
Rsx_Time.php, Rsx_Date.php - PHP implementation
|
||||
Rsx_Time.js, Rsx_Date.js - JavaScript implementation
|
||||
Rsx_DateTime_Cast.php - Custom Eloquent cast for datetimes
|
||||
Rsx_Date_Cast.php - Custom Eloquent cast for dates
|
||||
Reference in New Issue
Block a user