claimrevolution/oe-module-claimrev-connect

Fix — the Claims and ERA tabs broke when the client ID / secret were not configured:

• ModuleNotConfiguredException extended \RuntimeException directly rather than ClaimRevException, so it fell outside every catch (ClaimRevException) fault boundary in the module. ClaimRevApi::makeFromGlobals() throws it for a missing client ID, which meant an unconfigured install fatalled instead of degrading. The Claims tab died on plain page load — claims.php calls ClaimsPage::getClaimStatuses() before emitting any HTML, and that method's own catch (ClaimRevException) never matched — and the ERA tab died on Submit, where the boundary in era.php was narrower still (ClaimRevApiException only, which also misses authentication failures from a wrong secret).
• Fix: ModuleNotConfiguredException now extends ClaimRevException, so the existing guards throughout the module cover the unconfigured case. This is the root fix; the rest is defense in depth.
• claims.php and era.php check GlobalConfig::isConfigured() up front, skip the API calls when credentials are missing, and render an explanatory banner pointing at Administration → Globals → ClaimRev Connect. The ERA banner also mentions Enable Test Mode, since that page can run on simulated data.
• Widen the search fault boundaries in era.php and payment_advice.php from ClaimRevApiException to ClaimRevException so authentication failures (a wrong or undecryptable secret) surface as the page's error alert rather than a fatal.
• Verified on the OpenEMR 8.3.0 test container with both credentials blank: both tabs render, ERA search returns cleanly, and test mode still produces mock ERA rows.

Testability — the module's first test suite, and a dependency-injection seam for ClaimRevApi (#24):

• The module shipped no tests, no require-dev, and no CI. ClaimRevApi already supported constructor injection, but no consumer used it: twelve classes called the static ClaimRevApi::makeFromGlobals() inline at eighteen call sites, so none of their logic could run without a live OpenEMR bootstrap, real global configuration, and a real OAuth token exchange.
• Add a PHPUnit 11 harness with hand-written stubs for the slice of OpenEMR core the module touches, and a GitHub Actions workflow running the suite on PHP 8.2 and 8.3. Tests drive a real Guzzle client over a MockHandler, so URL building, auth headers, and JSON encoding are genuinely exercised rather than mocked away. 17 tests, 37 assertions.
• Convert ClaimSearch, EraSearch, and ReportDownload to constructor-injected ClaimRevApi. Each keeps its existing static method as a thin wrapper that resolves the client from globals and delegates, so no call site changes. The remaining nine consumers need QueryUtils, Bootstrap, GlobalConfig, and the OpenEMR\Billing\* classes stubbed first and follow in a later release.
• Behaviour fix: ClaimSearch::search() documented "returns false on error" but caught nothing, so it could never return false and a failure propagated as an uncaught exception. It now catches ModuleNotConfiguredException, which makes the previously unreachable ($raw === false) branches in ClaimsPage live. The catch is deliberately narrow rather than on the ClaimRevException base class: because that base extends \RuntimeException, catching it would have swallowed genuine outages into an empty result set and rendered "No results found" on the Claims tab — telling a biller their claim did not exist when ClaimRev was merely unreachable.
• ReportDownload no longer extends BaseService. It used nothing from the parent, whose constructor queries the database and calls OEGlobalsBag::getKernel() — the method absent on OpenEMR 8.0.x that caused the 2.1.7 outage — so an injection constructor could not safely call it.
• Packaging: declare guzzlehttp/guzzle, which ClaimRevApi imports but the manifest never required (it resolved only because OpenEMR core provides it), and raise the PHP floor from >=7.1 to >=8.2, which ClaimRevApi's readonly class has required since it landed. Both were latent defects that only surfaced when the module was installed standalone for testing.

Testability, part two — the rest of the seam (#24):

• ClaimsPage, PaymentAdvicePage, ClaimTrackingService, ReconciliationService, NotificationPollService, ClaimUpload, and PaymentAdvicePostingService's ClaimRev notification step now take ClaimRevApi by constructor. Every entry point keeps its exact signature and, more importantly, its exact error contract. Three wrapper shapes were needed rather than one: several methods have no catch at all today and their wrappers must have none, several catch around client construction so construction has to stay inside the try, and one is a private helper whose caller owns the try (ReconciliationService::lookupClaimRev, called from inside reconcile()'s catch, which is what lets the reconciliation page still show OpenEMR data behind a warning banner when ClaimRev is unreachable). Applying a single template is what caused the regression fixed earlier in this release.
• ClaimUpload no longer extends BaseService. Its own constructor was dead code — every entry point is static and nothing ever instantiated the class — so applying the injection pattern would have run BaseService::__construct for the first time ever, querying the database and calling OEGlobalsBag::getKernel(), which does not exist on OpenEMR 8.0.x. Removing the inheritance is what makes the change safe, not a tidy-up.
• Coverage for eight previously untested pure helpers that needed no seam at all: normalizeAdvice, parsePatientControlNumber, buildIdempotencyReference, getClaimStatusLabel, sumServiceAmounts, parsePcn, computeDiscrepancy, and htmlToPlainText. normalizeAdvice is the shared normalisation point for both the live API path and the mock service, so one test covers both producers.
• Suite is now 61 tests, 108 assertions, green on PHP 8.2 and 8.3.
• Deliberately not converted, because injection would not make them meaningfully testable: ConnectivityInfo, whose constructor resolves Bootstrap/GlobalConfig before any API call, and EligibilityTransfer's sendWaitingEligibility()/sendImmediate(), where the API call sits deep inside DB-write sequences and the class extends BaseService. EligibilityTransfer::retryEligibility() and ::sendEligibility() already accept a ClaimRevApi parameter and needed no change. PaymentAdvicePostingService's posting pipeline, its lock, and its idempotency guard are untouched: none of them call ClaimRev, so the refactor stayed away from the billing writes entirely.

Fix — the eligibility card on the patient dashboard now matches the cards around it (#19):

• The card's interior still used core's expand_collapse_widget(), which emits legacy table and section-header markup. Core stopped calling that helper on the demographics page some time ago — it now survives only in third-party modules — so our section rendered with a modern card shell wrapped around old-style contents and looked out of place next to Vitals, Labs and the rest.
• The section now mirrors core's templates/patient/card/card_base.html.twig directly: card-body / card-title / card-text collapse, with Bootstrap's own collapse attributes driving the toggle. That also drops a dependency on a core function we no longer want to rely on, which is one less thing to break across OpenEMR versions. The \Throwable fault boundary around the include is unchanged.
• The original report was that the injected card was missing its card class entirely; that part was fixed in 2023 (commit 685e8e3, landed under three minutes after the issue was filed) but the issue was never closed. This finishes the job.

Known issues found while building the above, not yet fixed:

• ClaimRevApi never sets Guzzle's http_errors, so Guzzle throws on any 4xx/5xx before the code reaches its own status check. The "ClaimRev API returned HTTP {code}" branch is therefore unreachable, and ClaimRevApiException always carries httpStatusCode = 0 and responseBody = ''. No caller reads those fields today, so the effect is latent; the status and body remain readable from the exception message.
• The ERA tab reports an outage as "No results found". EraSearch::search() swallows the failure before public/era.php can show the error message it already has written for exactly this case.
• ClaimsPage::getClaimStatuses() catches the ClaimRevException base class, so an outage silently yields an empty status dropdown on the Claims tab. Preserved verbatim rather than narrowed, since changing it alters what public/claims.php renders.
• PaymentAdvicePage::getPaymentAdviceById() has no catch at any layer, and public/payment_advice_post.php calls it without one either, so an outage during payment posting produces a PHP fatal rather than that endpoint's JSON error shape.
• ClaimUpload constructs X12RemoteTracker, which is OpenEMR core code extending BaseService — whose constructor calls OEGlobalsBag::getKernel(), absent on 8.0.x. If that reading is right, auto-send of claim files is already broken on 8.0.x independently of this module, and cannot be fixed from here. Worth confirming on the 8.0.x test container.
• ClaimUpload calls file_get_contents() unsuppressed on a claim file it then checks for false, so a missing file emits a PHP warning into the error log on a path the code already handles.
• The static entry-point wrappers are not directly covered by tests. Reaching one means calling ClaimRevApi::makeFromGlobals(), which resolves KernelCompat, Bootstrap, and GlobalConfig — a chain the test stubs do not yet fake. The instance methods behind each wrapper are covered; what is untested is the wrapper's own catch, most notably ClaimsPage::getClaimStatuses() returning an empty list on outage. Closing this needs a Kernel stub and ServiceContainer::getCrypto() support.

Fix — enabling the module blanked the Patient Portal / API panel on the patient dashboard (and fatalled other pages) on OpenEMR 8.0.x:

• 2.1.4 moved kernel resolution to Compat\KernelCompat::resolve() to avoid the direct OEGlobalsBag::getInstance()->getKernel() call that fatals on 8.0.x (core's OEGlobalsBag there has no getKernel()), but that migration only covered the src/ service layer. Eleven call sites still called getKernel() directly: templates/eligibility.php and ten public/*.php entry points (claims, claim_requeue, claim_status, claim_sync_status, eligibility_chat, era, EraDownload, payment_advice, payment_advice_post, reconciliation). On 8.0.x these still threw "Call to undefined method OEGlobalsBag::getKernel()". The eligibility template renders inside the core demographics page via a RenderEvent listener (Bootstrap::renderEligibilitySection), so its fatal aborted the whole page render and the Patient Portal / API panel — emitted after it — vanished; disabling the module made the panel reappear. Reported by a client on 8.0.x.
• Fix: route all eleven remaining call sites through KernelCompat::resolve()->getEventDispatcher(), completing the 2.1.4 migration. Verified on the 8.0.0.3 test container (patient dashboard renders, Portal/API panel present, eligibility section loads).
• Defense in depth: wrap the eligibility-section include in Bootstrap::renderEligibilitySection in a try/catch (\Throwable) fault boundary. A render fault in the module's demographics panel now logs via error_log and degrades to an inline notice instead of blanking adjacent core panels — the same isolation rationale as the 2.1.5 calendar fix. (\Throwable, not \Exception, since an undefined-method fatal is an Error.)

Fix — appointments invisible in calendar week/day view when eligibility indicators are enabled:

• A second, independent defect in the calendar eligibility indicator (distinct from the 2.1.5 crash fix). The day/week calendar template treats an event's eventViewClass as a full replacement for the core CSS class ($evtClass = $event['eventViewClass'] ?? $evtClass in views/day/ajax_template.html), not an addition. The core class for an appointment is event_appointment, which in ajax_calendar_sass.scss supplies z-index: 2 and background-color: white. CalendarEligibilityIndicator::filterCalendarEvents set eventViewClass to only the eligibility class (e.g. event_elig_active), so the appointment <div> lost event_appointment and dropped behind the calendar grid — appointments appeared to vanish in week and day view (month view uses a different template and was unaffected). Reported by a provider who traced it to the event div losing its z-index.
• Fix: mergeEventViewClass now preserves a base class — the existing eventViewClass if another listener set one, otherwise event_appointment — and appends the eligibility class to it, so the appointment keeps its stacking and fill while still getting the eligibility border.

Critical fix — installing the module could make all calendar appointments disappear:

• The calendar eligibility indicator (CalendarEligibilityIndicator::filterCalendarEvents, enabled by "Enable Calendar Eligibility Indicators") listens on OpenEMR's CalendarUserGetEventsFilter. Core dispatches that filter inside postcalendar_userapi_pcGetEvents() with no error handling, then returns whatever the listeners produce. The listener ran an unguarded SELECT ... FROM mod_claimrev_eligibility (referencing payer_responsibility, last_checked, create_date, individual_json, status). On an install whose mod_claimrev_eligibility schema was out of date (missing column) or otherwise unhappy, that query threw — and because core has no try/catch around the dispatch, the exception aborted the whole calendar event fetch, so every appointment vanished from the calendar. Disabling the module removed the listener and the appointments reappeared, which is why the symptom looked like the module was deleting appointments. It never touched appointment data.
• Fix: wrap the indicator logic in a fault boundary. A cosmetic color indicator must never be able to remove appointments, so filterCalendarEvents now delegates to applyEligibilityIndicators inside a try/catch (\Throwable); on any failure it logs via error_log and returns the events unmodified. The calendar always renders; at worst the eligibility colors are missing.

Cross-version compatibility fix:

• Resolve the OpenEMR kernel via a new Compat\KernelCompat::resolve() helper instead of calling OEGlobalsBag::getInstance()->getKernel() directly. getKernel() only exists on core's flex/master line; the 8.0.x patch releases ship an OEGlobalsBag without it. Because the real class is present on 8.0.x, the OEGlobalsBagShim swap never activates there, so the direct call fatalled with "Call to undefined method OEGlobalsBag::getKernel()" during login bootstrap — taking the whole site to a 500. The helper reads the kernel from the 'kernel' global (with a type guard), exactly as flex's getKernel() does internally, so the same binary works on 7.x, 8.0.x, and flex. Fixes the regression introduced in 2.1.3.

Cross-version compatibility and security hardening:

• Read stored credentials with CryptoGen::decryptStandard instead of decryptFromDatabase. The newer helper was added to OE core in the 8.x line but does not exist on OE 7.x; reverting to decryptStandard lets the same module binary work on both branches.
• Close findings from the external Aisle Analyzer security review of PR #11265: tighten IDOR and CSRF gaps on AJAX endpoints, refetch ERA and claim status server-side rather than trusting the browser to echo them back, and tighten property-access hardening on several response shapes.

Test mode coverage extended to every gated page:

• ERA tab and EraDownload short-circuit to mock data when test mode is enabled, gated by the global setting (the per-tab checkbox is removed).
• Payment Advice tab reads the test-mode global directly; per-tab checkbox removed.
• Reconciliation tab returns mock rows via ReconciliationMockService.
• Eligibility Chat returns mock AI answers in test mode.
• claim_sync_status and claim_requeue short-circuit cleanly without contacting the API.
• README's test-mode coverage row documents every gated page.

Maintenance:

• Rector/PHPStan cleanups across the module.
• Fix STATUS_UPLOAD_ERROR constant typo.
• Miscellaneous narrowing of array/object access patterns flagged by PHPStan.