dwindown/WooNooW
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

docs: Update PROJECT_SOP and add customer data flow analysis

Browse Source 
1. Updated PROJECT_SOP.md:
✅ Added mobile card linkable pattern with full example
✅ Added submenu mobile hiding rules and behavior matrix
✅ Documented stopPropagation pattern for checkboxes
✅ Added ChevronRight icon requirement
✅ Documented active:scale animation for tap feedback
✅ Added spacing rules (space-y-3 for cards)

2. Created CUSTOMER_DATA_FLOW_ANALYSIS.md:
✅ Comprehensive analysis of customer data flow
✅ Documented 2 customer types: Guest vs Site Member
✅ Identified validation issues in OrdersController
✅ Found weak ! empty() checks allowing bad data
✅ Documented inconsistent validation between controllers
✅ Created action items for fixes
✅ Added test cases for all scenarios

Key Findings:
❌ OrdersController uses ! empty() - allows 'Indonesia' string
❌ No phone number sanitization in order creation
❌ No validation that phone is actually a phone number
✅ CustomersController has better validation (isset + sanitize)

Next: Investigate source of 'Indonesia' value and implement fixes
This commit is contained in:
dwindown
2025-11-20 23:52:23 +07:00
parent 97e24ae408
commit 275b045b5f
3 changed files with 424 additions and 8 deletions
Download Patch File Download Diff File Expand all files Collapse all files

345
CUSTOMER_DATA_FLOW_ANALYSIS.md Normal file
View File

@@ -0,0 +1,345 @@
# Customer Data Flow Analysis
## Issue Report
**Problem:** Customer `billing_phone` shows "Indonesia" instead of phone number
**Source:** Customer created via Order module
**Impact:** Incorrect customer data stored in database
## Data Flow Investigation
### A. Customer as Guest (Not Site Member)
#### 1. Order Creation Flow
**File:** `OrdersController.php` → `create_order()` method
**Steps:**
1. Order form submits billing data including `phone`
2. Data flows through `$billing['phone']` parameter
3. Order billing is set via `$order->set_billing_phone($billing['phone'])`
4. **No WC_Customer object created** - guest orders only store data in order meta
**Code Location:** Lines 780-1120
```php
// Guest customer - data only in order
$order->set_billing_first_name($billing['first_name'] ?? '');
$order->set_billing_last_name($billing['last_name'] ?? '');
$order->set_billing_email($billing['email'] ?? '');
$order->set_billing_phone($billing['phone'] ?? ''); // ← Data here
// ... more fields
```
**Issue:** Guest customers don't have WC_Customer records, so viewing them in Customer module will fail or show incorrect data.
---
### B. Customer as Site Member
#### 1. Existing Member - Order Creation
**File:** `OrdersController.php` → Lines 1020-1064
**Flow:**
1. User exists, found by email
2. **Upgrade subscriber to customer role** (if needed)
3. Create `WC_Customer` object
4. **Update customer data** from order billing/shipping
5. Save customer
6. Link order to customer
**Code:**
```php
if ($user) {
// Upgrade role if needed
if (in_array('subscriber', (array) $user->roles, true)) {
$user->set_role('customer');
}
// Update customer billing & shipping data
$customer = new \WC_Customer($user->ID);
// Update billing address
if (! empty($billing['first_name'])) $customer->set_billing_first_name($billing['first_name']);
if (! empty($billing['last_name'])) $customer->set_billing_last_name($billing['last_name']);
if (! empty($billing['email'])) $customer->set_billing_email($billing['email']);
if (! empty($billing['phone'])) $customer->set_billing_phone($billing['phone']); // ← HERE
// ... more fields
$customer->save();
}
```
**Validation:** Uses `! empty()` check
**Problem:** If `$billing['phone']` contains "Indonesia" (non-empty string), it will be saved!
---
#### 2. New Member - Auto-Registration
**File:** `OrdersController.php` → Lines 1065-1118
**Flow:**
1. User doesn't exist
2. Auto-register setting is ON
3. Create WordPress user
4. Create `WC_Customer` object
5. Set billing/shipping from order data
6. Save customer
7. Send welcome email
**Code:**
```php
elseif ($register_member) {
// Create user
$user_id = wp_insert_user($userdata);
if (!is_wp_error($user_id)) {
$customer = new \WC_Customer($user_id);
// Billing address
if (! empty($billing['first_name'])) $customer->set_billing_first_name($billing['first_name']);
// ...
if (! empty($billing['phone'])) $customer->set_billing_phone($billing['phone']); // ← HERE
// ...
$customer->save();
}
}
```
**Same Issue:** `! empty()` check allows "Indonesia" to be saved.
---
### C. Customer Module Direct Edit
**File:** `CustomersController.php` → `update_customer()` method (Lines 232-310)
**Flow:**
1. Receive customer data via PUT request
2. Update WordPress user meta
3. Update `WC_Customer` billing/shipping
4. Save customer
**Code:**
```php
$customer = new WC_Customer($id);
// Billing address
if (!empty($data['billing'])) {
$billing = $data['billing'];
if (isset($billing['first_name'])) $customer->set_billing_first_name(...);
// ...
if (isset($billing['phone'])) $customer->set_billing_phone(sanitize_text_field($billing['phone']));
// ...
}
$customer->save();
```
**Validation:** Uses `isset()` check (better than `! empty()`)
**Sanitization:** Uses `sanitize_text_field()` ✅
---
## Root Cause Analysis
### Possible Sources of "Indonesia" Value
1. **Frontend Default Value**
- Check `OrderForm.tsx` for default country/phone values
- Check if "Indonesia" is being set as placeholder or default
2. **Backend Fallback**
- Check if WooCommerce has default country settings
- Check if there's a fallback to country name instead of phone
3. **Data Validation Issue**
- `! empty()` check allows ANY non-empty string
- No validation that phone is actually a phone number
- No sanitization before saving
4. **Virtual Products Case**
- When cart has only virtual products, address fields are hidden
- Phone field might still be submitted with wrong value
---
## Issues Found
### 1. ❌ Weak Validation in OrdersController
**Problem:**
```php
if (! empty($billing['phone'])) $customer->set_billing_phone($billing['phone']);
```
- `! empty()` allows "Indonesia", "test", "abc", etc.
- No phone number format validation
- No sanitization
**Should Be:**
```php
if (isset($billing['phone']) && $billing['phone'] !== '') {
$customer->set_billing_phone(sanitize_text_field($billing['phone']));
}
```
---
### 2. ❌ No Data Sanitization in Order Creation
**Problem:**
- Direct assignment without sanitization
- Allows any string value
- No format validation
**Should Add:**
- `sanitize_text_field()` for all text fields
- Phone number format validation
- Empty string handling
---
### 3. ❌ Inconsistent Validation Between Controllers
**OrdersController:**
- Uses `! empty()` check
- No sanitization
**CustomersController:**
- Uses `isset()` check ✅
- Has `sanitize_text_field()` ✅
**Should:** Use same validation pattern everywhere
---
### 4. ❌ Virtual Products Address Handling
**Current Behavior:**
- Frontend hides address fields for virtual products
- But phone field is ALWAYS shown
- Backend might receive wrong data
**Check:**
- OrderForm.tsx line 1023: `setBPhone(data.billing.phone || '');`
- Does this fallback to something else?
---
## Action Items
### 1. Fix OrdersController Validation
**File:** `OrdersController.php`
**Lines:** 1039-1047, 1088-1096
**Change:**
```php
// OLD (Lines 1042)
if (! empty($billing['phone'])) $customer->set_billing_phone($billing['phone']);
// NEW
if (isset($billing['phone']) && trim($billing['phone']) !== '') {
$customer->set_billing_phone(sanitize_text_field($billing['phone']));
}
```
Apply to:
- Existing member update (lines 1039-1047)
- New member creation (lines 1088-1096)
---
### 2. Add Phone Number Validation
**Create Helper Function:**
```php
private static function sanitize_phone($phone) {
if (empty($phone)) {
return '';
}
// Remove non-numeric characters except + and spaces
$phone = preg_replace('/[^0-9+\s-]/', '', $phone);
// Trim whitespace
$phone = trim($phone);
// If result is empty or just symbols, return empty
if (empty($phone) || preg_match('/^[+\s-]+$/', $phone)) {
return '';
}
return $phone;
}
```
---
### 3. Check Frontend Data Source
**File:** `OrderForm.tsx`
**Line:** ~1023
**Check:**
- Where does `data.billing.phone` come from?
- Is there a default value being set?
- Is "Indonesia" coming from country field?
---
### 4. Test Cases
**A. Guest Customer:**
1. Create order with phone = "08123456789"
2. Check order billing_phone
3. Verify no WC_Customer created
**B. Existing Member:**
1. Create order with existing customer
2. Phone = "08123456789"
3. Check WC_Customer billing_phone updated
4. Create another order with same customer
5. Phone = "08198765432"
6. Verify WC_Customer phone updated to new value
**C. New Member (Auto-register):**
1. Create order with new email
2. Auto-register ON
3. Phone = "08123456789"
4. Verify WC_Customer created with correct phone
**D. Virtual Products:**
1. Create order with only virtual products
2. Verify phone field behavior
3. Check what value is submitted
---
## Expected Behavior
### Order Creation with Existing Member
1. Order billing data should update WC_Customer data
2. Phone should be validated and sanitized
3. Empty phone should clear WC_Customer phone (not set to country name)
### Order Creation with New Member
1. WC_Customer should be created with correct data
2. Phone should be validated and sanitized
3. No fallback to country name
### Virtual Products
1. Phone field should still work correctly
2. No address fields needed
3. Phone should not default to country name
---
## Next Steps
1. ✅ Update PROJECT_SOP.md with mobile UX patterns
2. 🔄 Find source of "Indonesia" value
3. ⏳ Fix validation in OrdersController
4. ⏳ Add phone sanitization helper
5. ⏳ Test all scenarios
6. ⏳ Document fix in commit message

83
PROJECT_SOP.md
View File

@@ -263,6 +263,39 @@ When updating an existing module to follow this pattern:
</SubMenu>
```
**Submenu Mobile Behavior:**
To reduce clutter on mobile detail/new/edit pages, submenu MUST be hidden on mobile for these pages:
```typescript
// In SubmenuBar.tsx
const isDetailPage = /\/(orders|products|coupons|customers)\/(?:new|\d+(?:\/edit)?)$/.test(pathname);
const hiddenOnMobile = isDetailPage ? 'hidden md:block' : '';
return (
<div className={`border-b border-border bg-background ${hiddenOnMobile}`}>
{/* Submenu items */}
</div>
);
```
**Rules:**
1. ✅ **Hide on mobile** for detail/new/edit pages (has own tabs + back button)
2. ✅ **Show on desktop** for all pages (useful for quick navigation)
3. ✅ **Show on mobile** for index pages only (list views)
4. ✅ **Use regex pattern** to detect detail/new/edit pages
5. ❌ **Never hide on desktop** - always useful for navigation
6. ❌ **Never show on mobile detail pages** - causes clutter
**Behavior Matrix:**
| Page Type | Mobile | Desktop | Reason |
|-----------|--------|---------|--------|
| Index (`/orders`) | ✅ Show | ✅ Show | Main navigation |
| New (`/orders/new`) | ❌ Hide | ✅ Show | Has form tabs + back button |
| Edit (`/orders/123/edit`) | ❌ Hide | ✅ Show | Has form tabs + back button |
| Detail (`/orders/123`) | ❌ Hide | ✅ Show | Has detail tabs + back button |
**Toolbar (React):**
```typescript
<Toolbar>
@@ -394,18 +427,55 @@ All CRUD list pages MUST follow these consistent UI patterns:
</tr>
```
**Mobile Card Pattern:**
**Mobile Card Pattern (Linkable):**
Mobile cards MUST be fully tappable (whole card is a link) for better UX:
```tsx
<div className="md:hidden space-y-2">
<div className="md:hidden space-y-3">
{items.map(item => (
<Card key={item.id} className="p-4">
{/* Card content */}
</Card>
<Link
key={item.id}
to={`/entity/${item.id}/edit`}
className="block bg-card border border-border rounded-xl p-3 hover:bg-accent/50 transition-colors active:scale-[0.98] active:transition-transform shadow-sm"
>
<div className="flex items-center gap-3">
{/* Checkbox with stopPropagation */}
<div onClick={(e) => { e.preventDefault(); e.stopPropagation(); onSelect(item.id); }}>
<Checkbox checked={selected} className="w-5 h-5" />
</div>
{/* Content */}
<div className="flex-1 min-w-0">
<h3 className="font-bold text-base leading-tight mb-1">{item.name}</h3>
<div className="text-sm text-muted-foreground truncate mb-2">{item.description}</div>
<div className="flex items-center gap-3 text-xs text-muted-foreground mb-1">
<span>{item.stats}</span>
</div>
<div className="font-bold text-lg tabular-nums text-primary">{item.amount}</div>
</div>
{/* Chevron */}
<ChevronRight className="w-5 h-5 text-muted-foreground flex-shrink-0" />
</div>
</Link>
))}
</div>
```
**Rules:**
**Card Rules:**
1. ✅ **Whole card is Link** - Better mobile UX (single tap to view/edit)
2. ✅ **Use `space-y-3`** - Consistent spacing between cards
3. ✅ **Checkbox stopPropagation** - Prevent navigation when selecting
4. ✅ **ChevronRight icon** - Visual indicator card is tappable
5. ✅ **Active scale animation** - `active:scale-[0.98]` for tap feedback
6. ✅ **Hover effect** - `hover:bg-accent/50` for desktop hover
7. ✅ **Shadow** - `shadow-sm` for depth
8. ✅ **Rounded corners** - `rounded-xl` for modern look
9. ❌ **Never use separate edit button** - Whole card should be tappable
10. ❌ **Never use `space-y-2`** - Use `space-y-3` for consistency
**Table Rules:**
1. ✅ **Always use `p-3`** for table cells (NOT `px-3 py-2`)
2. ✅ **Always add `hover:bg-muted/30`** to body rows
3. ✅ **Always use `bg-muted/50`** for table headers
@@ -419,6 +489,7 @@ All CRUD list pages MUST follow these consistent UI patterns:
- Desktop: Show table with `hidden md:block`
- Mobile: Show cards with `md:hidden`
- Both views must support same actions (select, edit, delete)
- Cards must be linkable (whole card tappable)
#### Variable Product Handling in Order Forms

4
admin-spa/src/routes/Coupons/CouponForm.tsx
View File

@@ -80,8 +80,8 @@ export default function CouponForm({
const tabs = [
{ id: 'general', label: __('General'), icon: <Settings className="w-4 h-4" /> },
{ id: 'restrictions', label: __('Usage restrictions'), icon: <ShieldCheck className="w-4 h-4" /> },
{ id: 'limits', label: __('Usage limits'), icon: <BarChart3 className="w-4 h-4" /> },
{ id: 'restrictions', label: __('Restrictions'), icon: <ShieldCheck className="w-4 h-4" /> },
{ id: 'limits', label: __('Limits'), icon: <BarChart3 className="w-4 h-4" /> },
];
return (