Update sample to latest recommendations: optional Application/DI, VF→LWC, and Agentforce actions by afawcett · Pull Request #61 · apex-enterprise-patterns/fflib-apex-common-samplecode

afawcett · 2026-08-25T10:32:11Z

Summary

Updates the sample to reflect current recommendations—making the Application class and interfaces optional unless using DI—along with modernized UI (VF→LWC) and Agentforce actions.

Remove Application.cls and service interfaces in favour of concrete services with newInstance() factories and test mocks — see Apex Enterprise Patterns: Recent Updates and Thoughts on the Application Class for background on this change
Replace Visualforce custom buttons with Lightning Web Components and quick actions
Add invocable actions and an AI authoring bundle for Agentforce
Move user mode Unit of Work wiring from Application.cls to UnitOfWork.cls (user mode enforcement unchanged from master)
Add polymorphic invoicing via InvoiceTargets__mdt
Update README and scratch org configuration for API 67+

This change is

CI deploy fails when __tests__ bundles reference @lwc/engine-dom, which is not valid org metadata.

daveespo

@daveespo reviewed 2 files and all commit messages, and made 5 comments.
Reviewable status: 2 of 186 files reviewed, 5 unresolved discussions (waiting on afawcett, ImJohnMDaniel, and john-storey-devops).

README.md line 33 at r1 (raw file):

------------------

This sample uses **concrete** Domain, Selector, and Service classes with static `newInstance()` factories and `@TestVisible` mock hooks for unit tests. There is no `Application.cls` dependency-injection factory — see [Apex Enterprise Patterns: Recent Updates and Thoughts on the Application Class](https://andyinthecloud.com/2026/04/13/apex-enterprise-patterns-recent-updates-and-thoughts-on-the-application-class/) for background on this approach.

Why keep the Interfaces for service/domain/selector in the project if you're taking the concrete implementation approach?

sfdx-source/apex-common-samplecode/main/classes/UnitOfWork.cls line 39 at r1 (raw file):

	 * @return Unit of Work instance.
	 */
	public static fflib_ISObjectUnitOfWork newInstance() {

Hmm. Seeing this in real life doesn't look like what I expected ... notably, you are hard-coding the list of SObjects that are part of UOW which doesn't lend itself well to being able to reuse this UOW across functional domains. Selectors and Domains are tied to their underlying SObjects but the UOW is used across the Services and by hardcoding this list here, it seems like we fell back toward the 'Application is the God class' problem .. we just moved it into UOW

I guess I wonder why we don't take the list of SObjects as the argument to newInstance()

sfdx-source/apex-common-samplecode/main/classes/actions/ApplyDiscount.cls line 45 at r1 (raw file):

		List<Result> results = new List<Result>();
		for (Request request : requests) {
			OpportunitiesService.newInstance().applyDiscounts(

Is this DML in a loop?

sfdx-source/apex-common-samplecode/main/classes/controllers/OpportunityCreateInvoicesController.cls line 36 at r1 (raw file):

	 * (for example 10 for 10%).
	 */
	public Decimal DiscountPercentage { get; set; }

lower case D

sfdx-source/apex-common-samplecode/main/classes/controllers/OpportunityApplyDiscountsController.cls line 35 at r1 (raw file):

	 * Discount percent entered on the list page (for example 10 for 10%).
	 */
	public Decimal DiscountPercentage { get; set; }

lowercase D

Aligns the sample factory with the concrete-class approach discussed in PR review.

afawcett · 2026-09-06T11:02:12Z

@daveespo Good catch — I meant the sample app’s own domain/selector/service interfaces, which this PR removes. The remaining interfaces are from fflib-apex-common (fflib_ISObjectUnitOfWork, etc.).

We can’t remove those without breaking the library contract, and they’re still useful where you want to mock at the framework boundary. The sample’s shift is to drop the extra sample-layer interfaces and Application.cls DI — concrete services with newInstance() + @TestVisible mocks instead.

On UnitOfWork.newInstance() returning fflib_ISObjectUnitOfWork vs fflib_SObjectUnitOfWork: agreed — for consistency with the concrete approach, UnitOfWork.newInstance() will return fflib_SObjectUnitOfWork (updated in latest commit).

afawcett · 2026-09-06T11:02:13Z

@daveespo I see the concern about the hardcoded SObject list feeling like a smaller Application class — but a per-service newInstance(List<SObjectType>) creates a different problem: you lose the ability to pass a single Unit of Work between services in one transaction (e.g. discounting and invoicing in the same commit).

The central registry does mean a missing type surfaces as a compile-time dependency that ripples through the codebase. I think that’s less common as the app evolves than the old Application.cls failure mode — different call sites quietly constructing incompatible UOW instances and only failing at runtime when a register/commit crosses service boundaries.

So the trade-off here is one app-level registry (now in UnitOfWork.cls) to support shared transactions, with the central list as the single place to register types. I've added a note on the class documenting the per-service alternative.

afawcett · 2026-09-06T11:02:14Z

@daveespo Good catch — the earlier version did call the service inside the loop.

It now builds a Map<Id, Decimal> from the requests, makes one applyDiscounts(...) call (one query, one commit), then maps results back for Flow. The loops are adapter-only; no DML in the loop.

afawcett · 2026-09-06T11:02:15Z

@daveespo Addressed — renamed DiscountPercentage to discountPercentage on both Visualforce controllers (OpportunityApplyDiscountsController, OpportunityCreateInvoicesController). The VF pages were already binding to {!discountPercentage}.

…flib_SObjectUnitOfWork consistently.

john-storey-devops · 2026-09-06T19:29:15Z

While considering current SOLID practices regarding this PR, two points have come to light: interfaces belong where variation genuinely exists, and constructor injection is the mechanism that actually satisfies dependency inversion.

My remaining reservation is how dependencies get substituted in tests. @TestVisible private static mock keeps the lookup inside the class, so a caller can't see or control what the service depends on. Constructor injection should expose those dependencies in the signature, while newInstance() is the default composition and the entry point — same testability, no ambient statics.

This also bears on the UnitOfWork.cls discussion above. Each service currently calls UnitOfWork.newInstance() internally and commits its own, so the shared-transaction case the class doc describes is met by combining operations in one method rather than by passing an instance. Constructor injection would let the entry point build one Unit of Work and hand it to both services — which is the capability the central registry is there to support.

Consider the following as an example of Apex following modern SOLID patterns:

// Supporting multiple processors 
// like Visa, American Express and Discover
// just as examples.
public interface ICardProcessor
{
    ProcessorResult charge(Payment__c payment);
}


public inherited sharing class PaymentService
{
    private final ICardProcessor processor;
    private final PaymentsSelector selector;
    private final fflib_ISObjectUnitOfWork uow;

    // Entry point for unit tests
    public PaymentService(ICardProcessor processor,
                          PaymentsSelector selector,
                          fflib_ISObjectUnitOfWork uow)
    {
        this.processor = processor;
        this.selector  = selector;
        this.uow       = uow;
    }

    // Entry point for business logic.
    public static PaymentService newInstance(ICardProcessor processor)
    {
        return new PaymentService(processor,
                                  PaymentsSelector.newInstance(),
                                  UnitOfWork.newInstance());
    }

    public void capture(Set<Id> paymentIds)
    {
        List<Payment__c> payments = selector.selectById(paymentIds);
        for (Payment__c payment : payments)
        {
            ProcessorResult result = processor.charge(payment);
            payment.Status__c = result.isApproved ? 'Captured' : 'Declined';
            uow.registerDirty(payment);
        }
        uow.commitWork();
    }
}


@IsTest
private class PaymentServiceTest
{
    @IsTest
    private static void captureMarksApprovedPaymentAsCaptured()
    {
        // Given
        ...

        // When
        new PaymentService(processorMock, selectorMock, uowMock).capture(paymentIds);

        // Then
        ...
    }
}

john-storey-devops

#61 (comment)

ImJohnMDaniel

I am assuming that if you wanted developers to use newInstance() methods in selectors and domains, then you would want the constructors to be private instead of public. Is that what you were thinking?

See README.md Architecture Notes

Make it clearer the connection between the two

'service' to 'opportunitiesSvc' improves readability of the code, e.g. opportunitiesSvc.applyDiscountsAndCreateInvoices

afawcett · 2026-09-17T10:24:56Z

@john-storey-devops thanks I made most of your changes here. Services and Controllers now use constructor injection. Domains and UnitOfWOrk still use property injection - as they are stateful (domain within a method, uow's within a service method) and we have no factories for them in this basic sample representation.

afawcett · 2026-09-17T10:28:19Z

sfdx-source/apex-common-samplecode/main/classes/controllers/OpportunityApplyDiscountsController.cls line 35 at r1 (raw file):