# Audit Report — magento-shippingfields

> **Audit date:** 2026-02-26 (Europe/London)
> **Scope:** Read-only discovery audit of source code and documentation
> **Auditor:** AI-assisted engineering review
> **Status:** Complete — no code changes made

---

## Project Overview

| Attribute | Value |
|-----------|-------|
| Package | `portalpeople/magento-shippingfields` |
| Version | Conflicted: v1.0.18 (`composer.json`), `setup_version="1.0.0"` (`module.xml`) |
| Purpose | Adds `shipping_date` and `shipping_message` custom fields to Magento 2 checkout, quote, and order |
| Language | PHP >= 7.4 + JavaScript (RequireJS/KnockoutJS) |
| Framework | Magento 2 |
| Tests | None — PHPUnit not configured |

---

## Documentation Inventory

| File | Status |
|------|--------|
| `README.md` | Complete (updated) |
| `AI_GUIDELINES.md` | Complete (updated) |
| `docs/architecture.md` | Complete |
| `docs/API.md` | Complete |
| `docs/BOUNDARIES.md` | Complete |
| `docs/DATA_OWNERSHIP.md` | Complete |
| `docs/GDPR.md` | Pre-existing — kept |
| `docs/INTEGRATIONS.md` | Complete |
| `docs/MERMAID_DIAGRAM_STANDARDS.md` | Pre-existing — kept |
| `docs/OBSERVABILITY.md` | Complete |
| `docs/WORKFLOW.md` | Complete |
| `docs/AUDIT.md` | This file |
| `docs/SECURITY.md` | Complete |
| `docs/TODO.md` | Complete |
| `docs/SYSTEM_MAP.md` | Complete |
| `docs/adr/ADR-000-discovery-baseline.md` | Complete |

---

## Findings

### F1 — HIGH: Synchronous AJAX in place-order-mixin.js

| Attribute | Detail |
|-----------|--------|
| Severity | HIGH |
| File | `src/view/frontend/web/js/action/place-order-mixin.js` |
| Impact | `async: false` blocks the browser main thread during order placement; deprecated by browsers; may cause UI freezes or "unresponsive script" dialogs |

The `place-order-mixin.js` makes an AJAX call to save shipping fields with `async: false`. Synchronous XMLHttpRequests on the main thread are deprecated in browsers and produce console warnings. They can cause browser UI to freeze during the request, degrading the checkout experience.

**Recommendation:** Convert to async with Promise chaining or `async/await`.

---

### F2 — MEDIUM: Missing composer.json dependency for magento-disableddates

| Attribute | Detail |
|-----------|--------|
| Severity | MEDIUM |
| File | `composer.json`, `src/Model/Checkout/DeliveryDateConfigProvider.php` |
| Impact | `portalpeople/magento-disableddates` is not in `require` — Composer will not install it automatically; fresh installs may fail silently or with a fatal error |

`DeliveryDateConfigProvider` uses the `PortalPeople\DisabledDates` collection directly. If the module is not installed, this will throw a class not found error at runtime.

**Recommendation:** Add `portalpeople/magento-disableddates` to `composer.json` require, or add an existence check in `DeliveryDateConfigProvider`.

---

### F3 — MEDIUM: Dual save paths — ambiguous write authority

| Attribute | Detail |
|-----------|--------|
| Severity | MEDIUM |
| Files | `src/Model/ShippingFieldsManagement.php`, `src/Plugin/ShippingInformationManagementPlugin.php` |
| Impact | Both paths write `shipping_date` and `shipping_message` to the quote; the last-write wins — undefined if both fire in the same request |

The REST API path (`ShippingFieldsManagement`) and the plugin path (`ShippingInformationManagementPlugin`) both save to the quote. If both fire during a checkout session, the result depends on execution order. This creates maintenance confusion about which path is the true source of truth.

**Recommendation:** Document which path is authoritative. Consider removing the redundant path.

---

### F4 — MEDIUM: Dual transfer paths — quote to order

| Attribute | Detail |
|-----------|--------|
| Severity | MEDIUM |
| Files | `src/Observer/QuoteSubmitBefore.php`, `src/Plugin/QuoteToOrderPlugin.php` |
| Impact | Both the observer and the plugin transfer fields from quote to order; may result in redundant writes |

**Recommendation:** Confirm which path is authoritative. Document in `BOUNDARIES.md`.

---

### F5 — MEDIUM: Admin Update controller ACL not confirmed

| Attribute | Detail |
|-----------|--------|
| Severity | MEDIUM |
| File | `src/Controller/Adminhtml/Order/Update.php` |
| Impact | If the ACL resource is not properly defined or checked, any admin user could update shipping fields regardless of role |

`UNKNOWN — Requires Clarification`: ACL resource protecting this controller was not found in source review.

**Recommendation:** Confirm ACL resource in code and document in `API.md`.

---

### F6 — LOW: Version mismatch

| Attribute | Detail |
|-----------|--------|
| Severity | LOW |
| Files | `composer.json` (v1.0.18), `src/etc/module.xml` (`setup_version="1.0.0"`) |
| Impact | Unclear what version is deployed; schema version may be inconsistent |

**Recommendation:** Align version strings.

---

### F7 — LOW: Backup and stray files in repository

| Attribute | Detail |
|-----------|--------|
| Severity | LOW |
| Files | `src/view/adminhtml/layout/sales_order_edit_index.xml.bak`, `src/view/frontend/web/js/action/place-order-mixin.js.bak`, `src/view/adminhtml/Untitled-1.html` |
| Impact | Confuses developers about active vs inactive files; `.bak` files may be deployed to production |

**Recommendation:** Remove all `.bak` files and the stray `Untitled-1.html` from the repository.

---

### F8 — LOW: Emoji tags in production logging

| Attribute | Detail |
|-----------|--------|
| Severity | LOW |
| Files | `src/Model/ShippingFieldsManagement.php`, `src/Observer/QuoteSubmitBefore.php`, multiple plugins |
| Impact | Emoji characters may not render correctly in all log aggregation tools; makes grep/regex matching harder |

**Recommendation:** Replace emoji prefixes with structured log context or standard text prefixes.

---

### F9 — LOW: No PHPUnit tests

| Attribute | Detail |
|-----------|--------|
| Severity | LOW |
| Impact | No automated regression protection for save logic, observer, or plugin conditions |

**Recommendation:** Add PHPUnit tests.

---

## Risk Summary

| Severity | Count | Items |
|----------|-------|-------|
| HIGH | 1 | F1 — Synchronous AJAX |
| MEDIUM | 4 | F2 — Missing dependency, F3 — Dual save path, F4 — Dual transfer path, F5 — ACL gap |
| LOW | 4 | F6 — Version mismatch, F7 — Backup files, F8 — Emoji logging, F9 — No tests |

---

## Audit Metadata

- **Method:** Static source code analysis — all source files read
- **Code changes:** None
- **Git actions:** None
- **Tools:** Claude Code AI-assisted review
- **Date:** 2026-02-26 (Europe/London)
