Skip to content

describeDestination(): purify to BMO calls instead of direct DB reads #27

@mwtcmi

Description

@mwtcmi

Context

Surfaced while fixing #15. Tools/AbstractTool.php's describeDestination() helper resolves a FreePBX dialplan destination string (e.g. ivr-8,s,1) into a friendly label (IVR: Main-Menu). It's the helper used by fm_list_all_dids and fm_did_destination_map.

Every branch is a direct DB read on another module's table:

Branch Current query Target BMO (likely)
from-did-direct / ext-local SELECT name FROM users WHERE extension = ? \FreePBX::Core()->getUser($ext)
ext-group SELECT description FROM ringgroups WHERE grpnum = ? \FreePBX::Ringgroups()->getRinggroup($id)
ext-queues SELECT descr FROM queues_config WHERE extension = ? \FreePBX::Queues()->getQueue($ext)
ivr- SELECT name FROM ivr_details WHERE id = ? \FreePBX::Ivr()->getDetails($id)
timeconditions SELECT displayname FROM timeconditions WHERE timeconditions_id = ? \FreePBX::Timeconditions()->getOne($id)
app-announcement SELECT description FROM announcement WHERE announcement_id = ? \FreePBX::Announcement()->getOne($id)

The reads are individually allowed by the BMO-first hierarchy ("reads of other modules' tables are OK when no BMO method exposes the data efficiently"), but the cumulative effect is six direct-DB-read branches sitting under a single helper. If any of those module schemas drift in a FreePBX update, the helper silently mislabels destinations — exactly the failure mode that produced #15.

Why this isn't urgent

  • Every branch is a parameterized read of a stable schema column. Low blast radius.
  • The output is a display label only — no security implications.
  • This is the v1.4.0 refactor that consolidated 5 inlined call sites; the surrounding helper is solid.

What to do

  1. For each branch above, verify the BMO method actually exists with the expected signature (use PHP reflection on a live dev box — don't trust the table above without checking).
  2. Replace the direct DB query with the BMO call.
  3. Where no BMO exists, keep the DB read but comment why ("BMO check: no \FreePBX::Foo()->getX() in FreePBX 17.0.x as of YYYY-MM").
  4. Add a small unit-style smoke test that calls describeDestination() for each branch type so future schema/method drift surfaces in CI.

Related

Metadata

Metadata

Assignees

No one assigned

    Labels

    enhancementNew feature or request

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions