Skip to content

Fix journal entry 500: PHP 8 method signature conflict in JournalController + business error handling#8

Draft
Copilot wants to merge 3 commits intomainfrom
copilot/resolve-journal-entry-failures
Draft

Fix journal entry 500: PHP 8 method signature conflict in JournalController + business error handling#8
Copilot wants to merge 3 commits intomainfrom
copilot/resolve-journal-entry-failures

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Apr 23, 2026

POST /journal/store (and all journal write endpoints) always returned a generic 500 because JournalController failed to load — its post(array $params) method conflicted with the inherited Controller::post(string $key) helper, which is a fatal error in PHP 8 (was only a warning in PHP 7). The error was caught exclusively by the global handler before any controller code ran.

Critical fix

  • JournalController::post()postEntry() — eliminates the PHP 8 fatal signature incompatibility
  • index.php route updated: JournalController@postEntry

Controller hardening (JournalController.php)

  • Moved requirePermission + verifyCsrf inside the try/catch block so DB errors during auth/CSRF checks are caught and return a structured 500 with debug_id instead of leaking to the global handler

Business errors → 422 instead of 500 (JournalService.php)

  • Fiscal period check in create() and update() moved before the transaction — returns ['success' => false, 'errors' => [...]] instead of throwing RuntimeException inside the transaction
  • All business-logic guards in post() / cancel() (double-post, closed period, imbalanced totals, etc.) converted from throw new \RuntimeException to structured return arrays — no more 500s for expected business rule violations

Global error handler (index.php)

  • Always generates a debug_id (e.g. SRV-A3F2C1D8), logs it, and includes it in JSON/HTML error responses — diagnosable without leaking sensitive details to end users

Account dropdown fix (main.php)

  • openNewJournalInline() made async — fetches /accounts/leaf when ERP_DATA.accounts is empty (fixes empty dropdown when opening "New Journal" from the dashboard or any page other than /journal)

Frontend resilience (erp.js)

  • submitJournalEntry: displays the errors[] array from 422 responses; shows debug_id on 500s; form is never closed on failure — state is preserved
  • initJournalForm: falls back to ERP_DATA.accounts if the caller passes an empty array
Original prompt

Create a comprehensive Pull Request in repository moded12/acc3 to fully resolve the journal entry failures and improve the application in two phases, based on the full debugging history below.

Repository:

  • moded12/acc3

Environment/context:

  • PHP 8.3 on Plesk
  • App path on server: /20/acc3
  • User has already deployed prior attempted fixes manually, but the problem persists on save.

Primary business problem:
The “New Journal Entry” flow had multiple failures. Account dropdowns were previously empty, later fixed to display accounts. Now users can select accounts and balance the journal, but saving still fails with POST /20/acc3/journal/store returning HTTP 500. Current JSON response is a generic error like:
{"success":false,"message":"حدث خطأ غير متوقع في الخادم. يرجى المحاولة لاحقاً."}

Root-cause investigation history and verified facts:

  1. Account dropdown issue:
  • Initially no accounts appeared in the journal line dropdown.
  • This was traced to how accounts were loaded/passed into the frontend, especially via views/layouts/main.php and public/assets/js/erp.js.
  • A later manual fix made accounts appear.
  1. Verified database facts:
  • Tables exist: journal_entry_headers, journal_entry_lines.
  • Manual INSERT into journal_entry_headers succeeded.
  • Manual INSERT into journal_entry_lines failed when account code was used instead of account primary key, confirming that journal_entry_lines.account_id must reference accounts.id, not accounts.code.
  • Verified mappings:
    • code 1111 => id 58
    • code 3101 => id 38
  • Therefore, any frontend/backend path that leaks account code instead of account id will break saving.
  1. Current observed UI state:
  • The dropdown now shows accounts such as صندوق الإدارة [1111] and رأس المال المدفوع [3101].
  • The journal can be filled so totals are balanced and the main description exists.
  • Yet Save Draft / Save and Post still fails.
  1. Files already identified as relevant:
  • app/controllers/JournalController.php
  • app/services/JournalService.php
  • app/repositories/JournalRepository.php
  • app/repositories/AccountRepository.php
  • app/services/AccountService.php
  • app/core/Database.php
  • app/core/Controller.php
  • views/layouts/main.php
  • public/assets/js/erp.js
  1. Changes already attempted manually before this PR:
  • JournalController methods store/update/post/cancel were changed to catch \Throwable instead of only RuntimeException.
  • Frontend erp.js was improved to handle empty/non-JSON responses and preserve UI on failure.
  • main.php was modified to expose ERP_DATA and to fetch/load accounts before opening New Journal.
  • Some manual fixes made the account dropdown functional.
  • Still, backend save continues to fail with a generic 500 JSON error.
  • Grepping for custom [acc3][journal.store] logs in common Plesk/system log locations did not reveal a clear root cause.

PR goals

Phase 1 — Fix the journal flow from the roots

Please perform full root-cause analysis and implement the complete fix so that:

  1. Account dropdown works reliably in New Journal.
  2. Selected accounts are persisted correctly in the frontend state and submitted as real account IDs (accounts.id), never account codes.
  3. Save Draft succeeds for valid journals.
  4. Save and Post succeeds for valid journals, or returns proper validation errors.
  5. No generic/opaque 500 remains for expected failures.
  6. Backend exceptions are surfaced in a diagnosable but safe way.
  7. UI does not reset or lose selected accounts/lines on failed save.

Specifically inspect and fix as needed:

  • views/layouts/main.php
    • data bootstrapping through ERP_DATA / window.ERP_DATA
    • openNewJournalInline()
  • public/assets/js/erp.js
    • initJournalForm
    • renderJeLines
    • updateJeLine
    • collectJournalLinesFromDom
    • submitJournalEntry
    • erpFetch / erpPost
  • app/controllers/JournalController.php
    • store/update/post/cancel error handling
    • JSON response consistency
  • app/services/JournalService.php
    • create/update/post validation and transaction flow
  • app/repositories/JournalRepository.php
    • createHeader / createLine / updateHeaderTotals correctness
  • app/core/Database.php
    • transaction/insert behavior if needed
  • app/core/Controller.php
    • inspect whether jsonError/jsonSuccess or global exception handling is masking the true error message

Also explicitly verify these possible causes and fix whichever are real:

  • lines_json malformed or not synced from DOM correctly
  • account_id in payload leaking code instead of PK id
  • createHeader succeeds but createLine fails
  • updateHeaderTotals throws
  • AuditLogger::log breaks successful saves afterward
  • Controller/global exception handling replaces specific exceptions with a generic message
  • schema mismatch between code and DB columns
  • transaction rollback due to a later exception after inserts succeed

Implementation expectations for Phase 1:

  • Ensure valid journals are saved succ...

This pull request was created from Copilot chat.

Copilot AI and others added 2 commits April 23, 2026 06:22
… account loading, error diagnostics

Agent-Logs-Url: https://github.com/moded12/acc3/sessions/8a7280b1-ac34-4071-b965-7e7c4c1479e7

Co-authored-by: moded12 <162719285+moded12@users.noreply.github.com>
Copilot AI changed the title [WIP] Fix journal entry save failures and improve application flow Fix journal entry 500: PHP 8 method signature conflict in JournalController + business error handling Apr 23, 2026
Copilot AI requested a review from moded12 April 23, 2026 06:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants