Memory Wiki

I'll start by reading the main file and checking the database table in parallel.

Got the PHP file. The DB queries hit a password issue — let me properly retrieve the MySQL root password and check the table.

Now let me check for additional details — the FluentCRM plugin presence and any test data that might need cleanup.

Let me get the exact line numbers for precise references.

Now I have all the data. Let me verify one critical finding — the `$profe_db` variable in `profe_schedule_crons()`.

Confirmed — `$profe_db` is undefined at line 380. Let me also verify the `profe_sync_on_register` issue.

Now I have all the evidence. Here is the complete review. --- ## Profe CRM Architecture — Final Read-Only Review ### Files Reviewed - **`/var/www/html/wp-content/mu-plugins/profe-sync.php`** (503 lines, version 2.0.0) - **Database table `profe_main.profe_crm`** (InnoDB, MySQL 8.4.9) - **Database table `profe_main.profe_users`** (InnoDB) - **Database table `profe_main.profe_mem_levels`** (14 rows) ### Environment - PHP 8.3.31, MySQL 8.4.9, FluentCRM 3.1.0 (active), PMPro (present) --- ### 🔴 CRITICAL BUGS (must fix before production deploy) #### 1. Undefined variable `$profe_db` in `profe_schedule_crons()` — **line 380** ```php function profe_schedule_crons() { if (!wp_next_scheduled('profe_cron_sync_billing_geo')) { ... } if (!wp_next_scheduled('profe_cron_backfill_memberships')) { ... } $profe_db->query("COMMIT"); // LINE 380 — $profe_db is NEVER defined in this scope } ``` `$profe_db` is never declared, never assigned, and `profe_get_db()` is never called. This produces: ``` PHP Warning: Undefined variable $profe_db Fatal error: Call to a member function query() on null ``` This function fires on **every** `wp` and `admin_init` action, so it will crash every page load. The entire COMMIT line should be removed — there is no transaction to commit, and cron scheduling doesn't need one. **Fix:** Delete line 380 entirely. #### 2. Undefined variable `$profe_db` in `profe_sync_on_register()` — **line 171** ```php function profe_sync_on_register($user_id) { ... if (function_exists('pmpro_getMembershipLevelForUser')) { $level = pmpro_getMembershipLevelForUser($user_id); if ($level && !empty($level->name)) { ... $profe_db = profe_get_db(); // only assigned inside this nested if ... } } $profe_db->query("COMMIT"); // LINE 171 — undefined when if-block not entered } ``` If PMPro is not active, or if the registered user has no PMP level yet, `$profe_db` is never assigned. This crashes with the same error. The COMMIT is also meaningless here — no transaction was started. **Fix:** Remove line 171 entirely, or wrap in `if ($profe_db = profe_get_db()) { ... }`. --- ### 🟡 MEDIUM ISSUES (should fix before production) #### 3. Orphaned `COMMIT` calls without any `START TRANSACTION` (3 locations) These functions call `$profe_db->query("COMMIT")` but never issued a `START TRANSACTION`: | Line | Function | |------|----------| | 312 | `profe_cron_billing_geo_cleanup()` | | 355 | `profe_cron_backfill_memberships()` | | 450 | `profe_crm_log_membership()` | In MySQL with `autocommit=1` (the default for `wpdb`), each individual query auto-commits. An explicit `COMMIT` with no open transaction is a harmless no-op, so these don't crash — but they're misleading and indicate the transactional intent was incomplete. If `autocommit` were ever turned off on this connection, these would commit unrelated queries prematurely. **Fix:** Remove the `COMMIT` calls or wrap them with proper `START TRANSACTION` / `COMMIT` pairs. The `profe_crm_log_membership` function (line 416-451) is a write hook that runs on PMPro events — it doesn't need a transaction for a single INSERT+UPDATE since each statement auto-commits. #### 4. `COMMIT` in `profe_crm_log_membership` — incomplete transactional intent The function at line 450 calls `COMMIT` but never `START TRANSACTION`. The function does an `INSERT IGNORE` followed by a conditional `UPDATE`. If the intent was atomicity, it needs a `START TRANSACTION` before the INSERT and `COMMIT` after the UPDATE. Currently both statements auto-commit independently — acceptable for this use case, but the COMMIT is spurious. #### 5. Test data in `profe_main.profe_crm` — row with invalid tag There is 1 row in the table: ``` id=1, user_id=3, tag='test_membership', processed=0 ``` `'test_membership'` is NOT one of the 14 valid slugs in `profe_get_all_membership_slugs()`. On the next page load, the consumer will: - `firstOrCreate` a new FluentCRM tag called `test_membership` (creates a permanent orphan tag) - Attach it to the FluentCRM contact for user_id=3 - This tag will **never** be detached by future updates because it's not in the detach list **Fix:** Delete this row before production: ```sql DELETE FROM profe_main.profe_crm WHERE tag = 'test_membership'; ``` --- ### 🟢 LOW / INFORMATIONAL #### 6. Consumer silently skips users without a FluentCRM contact (line 477-478) ```php $contact = FluentCrm\App\Models\Subscriber::where('user_id', $row->user_id)->first(); if (!$contact) continue; ``` If user_id=3 exists in WordPress but has no corresponding FluentCRM Subscriber record, the row stays `processed=0` forever. This is handled without crashing, but could create a growing backlog. Consider adding a `max_retries` column or logging when a row is skipped. #### 7. `firstOrCreate` works correctly for unknown slugs (line 480-483) If `$row->tag` doesn't match any existing FluentCRM tag, `firstOrCreate` creates one with both `slug` and `title` set to the tag value. This is correct behavior — no failures expected. #### 8. `pluck("slug")` on empty contact tags (line 488) If `$contact->tags` is null/empty (no tags attached), `pluck("slug")` returns an empty Collection, `->toArray()` returns `[]`, and `array_intersect([], $allMembershipSlugs)` returns `[]`. The detach block is skipped. **This is safe.** #### 9. `FOR UPDATE SKIP LOCKED` is correct for MySQL 8.4 (line 468) ```php $rows = $profe_db->get_results("SELECT ... FROM profe_crm WHERE processed = 0 ... LIMIT 50 FOR UPDATE SKIP LOCKED"); ``` `SKIP LOCKED` was added in MySQL 8.0. Confirmed MySQL 8.4.9 — fully supported. The pattern prevents concurrent consumers from processing the same rows. **Correct.** #### 10. INSERT IGNORE + `rows_affected === 0` fallback (line 438-448) This correctly handles: - **New row:** INSERT succeeds (rows_affected=1) — no fallback needed - **Existing row (same user_id+tag):** INSERT IGNORE skips (rows_affected=0) — UPDATE resets `processed=0` and `created_at=NOW()` The strict comparison `=== 0` is safe because `wpdb::query()` returns `int` on success. **Correct.** #### 11. Cancellation path — `level_id=0` maps to `"free"` (line 424-425) When a membership is cancelled, `level_id` is 0/empty. The function logs `tag = "free"`. The consumer will: 1. Detach all 14 possible membership tags from the contact 2. Attach the `"free"` tag **Correct.** This ensures cancelled users end up with exactly the `free` tag. #### 12. Performance — `init:100` with LIMIT 50 (line 458, 468) The consumer runs on every page load but: - `processed` column is indexed (`KEY idx_processed (processed)`) - LIMIT 50 caps work per request - `SKIP LOCKED` prevents lock contention - Early return if `FluentCrm` classes are missing **Acceptable for moderate traffic.** At high scale, consider moving to a cron-based worker instead of hooking `init`. #### 13. `profe_get_all_membership_slugs()` — symmetrical with `profe_map_membership_slug()` (lines 56-66 vs 68-114) All 14 slugs from `profe_map_membership_slug()` are present in `profe_get_all_membership_slugs()`. The detach logic correctly iterates all possible membership tags. **No gaps.** #### 14. Concurrent checkout safety (line 438-448) If two simultaneous PMPro events fire for the same user+tag: - The UNIQUE KEY `idx_user_tag_unique` on `(user_id, tag)` prevents duplicates - `INSERT IGNORE` handles the race — one succeeds, the other gets rows_affected=0 - The fallback UPDATE resets `processed=0`, ensuring the consumer picks it up **Race-condition safe.** #### 15. Redundant index `idx_wp_user_id` on `profe_users` The `profe_users` table has both `UNIQUE KEY wp_user_id (wp_user_id)` and `KEY idx_wp_user_id (wp_user_id)`. The unique key already serves as an index — the additional non-unique index is redundant and wastes space on writes. **Fix (optional):** Drop `idx_wp_user_id`: ```sql ALTER TABLE profe_main.profe_users DROP INDEX idx_wp_user_id; ``` --- ### Summary | Severity | Count | Key Issues | |----------|-------|------------| | 🔴 Critical | 2 | Line 380: undefined `$profe_db` crashes every page load. Line 171: undefined `$profe_db` crashes on user register without PMPro. | | 🟡 Medium | 3 | Lines 312, 355, 450: orphaned COMMITs. Test row with invalid tag `test_membership` in profe_crm. | | 🟢 Low | 4 | Redundant index, silent skips, `init`-based consumer at scale, spurious COMMITs are harmless no-ops. | **Top recommendation:** Delete or fix lines 171 and 380 before anything reaches production — both are guaranteed crash paths. **Pre-deployment checklist:** 1. Fix line 380 (delete `$profe_db->query("COMMIT")`) 2. Fix line 171 (delete or guard `$profe_db->query("COMMIT")`) 3. Clean up the `test_membership` row in `profe_crm` 4. Optionally remove orphaned COMMITs at lines 312, 355, 450 5. Optionally drop redundant index `idx_wp_user_id` on `profe_users`