Get in Touch

Tag: DML

  • What a good Salesforce technical lead actually does in code review

    Most code review comments on a Salesforce PR are really about formatting. Bracket placement, an unused variable, a missing test class. None of that is wrong to flag, but it’s not the job. The job is catching the thing that passes every static check and still takes the org down at 50,000 records, three releases from now.

    Reading for what scales, not what compiles

    A trigger that queries inside a for loop will pass code coverage. It’ll pass a linter. It might even pass a demo with ten test records. It fails the first time someone runs a data load, and by then the person who wrote it has usually moved to a different project. Governor limits don’t show up in a diff. You have to know what the code will do at volume, which means picturing the bulk case before you read a single line of the actual implementation.

    I ask four things before I look at logic: what triggers this, how many records can hit it in one transaction, what happens if it runs twice, and whether it depends on a specific order of operations somewhere else in the org. Most defects I catch in review live in the gap between those questions and what the developer assumed while writing the happy path.

    The bulkification checklist nobody writes down

    SOQL and DML inside loops are the obvious ones, and most experienced Apex developers catch those themselves now. The subtler failures are recursive trigger calls, callouts fired per record instead of batched, and collection logic that looks bulk-safe but still does an expensive comparison somewhere in the middle. None of that fails a unit test with 200 records. It fails a Tuesday morning data load with 40,000.

    Test coverage tells you almost nothing

    Seventy-five percent is a compliance number, not a quality signal. A test class can hit every line and still assert nothing beyond “no exception was thrown.” When I review test code, I’m checking for negative cases: a null field, a duplicate external ID, a user without the right permission set, or a picklist value that no longer exists. If every test in the class uses the same clean data setup, the coverage number is decorative.

    The better question in review isn’t “does this have tests.” It’s “would this test fail if the logic were wrong.” Write the bug in your head, then check whether any assertion in the class would catch it. Half the time it wouldn’t, and that’s the actual finding, not the coverage percentage sitting at 82%.

    Where governance debt actually gets created

    Every org I’ve worked in that’s been customised for years has the same pattern behind it. Nobody made one bad decision. A hundred small, reasonable-looking decisions got approved individually, and the org ended up with several ways to do the same thing: a validation rule here, a flow there, an Apex trigger doing the same check somewhere else. Each PR looked fine on its own.

    Catching that in review means asking a question most reviewers skip: does this already exist somewhere else in the org? Before approving a new flow that enforces a business rule, I check whether that rule is already sitting in a validation rule, a different flow, or a trigger nobody’s touched in two years. It takes an extra five minutes. It’s the difference between one source of truth and three that quietly disagree with each other by the time someone debugs a production issue.

    Naming and structure aren’t nitpicks

    A trigger handler named after the object instead of the intent. A flow with no description. An Apex class mixing unrelated responsibilities because it was faster to bolt on than to split out. These get waved through constantly because none of them break anything today. What they break is the next person’s ability to safely change the org without reading every line first, from scratch, with no context. I’ll push back on structure even when the logic is correct, because the logic being correct once isn’t the bar. The bar is someone else understanding it without asking the original author, who by then might not even be on the project anymore.

    Security review isn’t a separate step

    Field-level security, sharing model behaviour, CRUD and FLS enforcement on SOQL: these tend to get treated as a checklist item for a security audit months later, if at all. They should be part of every review, not a phase that happens afterward. Is this Apex class running without sharing when it should respect it? Does this LWC expose a method that skips permission checks a profile would otherwise enforce? Catching that in review costs one comment. Catching it in production costs an incident report and a client conversation nobody wants to have.

    What I actually leave as comments

    Not “nice work,” and not a list of style preferences dressed up as requirements. I leave the specific scenario that breaks it: this fails when the account has no owner, this recursion loop triggers if the same record updates twice in one transaction, this assumes a single currency and the org has four. Specific, reproducible, tied to a real condition that exists in this org. A developer can act on that immediately. “Please refactor for clarity” gives them nothing to act on and just adds a review cycle for its own sake.

    Good review also says yes fast. If a PR does what it needs to do, handles bulk correctly, respects sharing, and the tests would actually catch a regression, approve it. Slowing down good work just to leave a comment teaches people to dread review instead of using it.

    The habit that makes the biggest difference

    This isn’t about being harsher on people’s code. Good review protects the org the code will run in six months, not the sandbox it happened to be written in. The developer who wrote it is usually thinking about today’s ticket. The reviewer’s job is thinking about the org at scale, under load, with the security model intact, and readable by whoever inherits it next year. That gap is the whole difference between a review that catches typos and one that actually protects the platform.

  • Designing a Salesforce solution around one metric leadership will actually track

    Ask most delivery teams what a Salesforce enhancement is meant to achieve and you’ll get a feature description back. “We’re adding a custom object for renewals.” “We’re building an approval process for discounts over 15%.” Ask what number on a leadership dashboard is supposed to move because of it, and you’ll usually get a pause.

    That pause is the problem. A solution built without a target metric gets evaluated on whether it shipped, not whether it worked. Six months later nobody can say if the renewals object actually reduced churn, or if the approval process actually cut discount leakage, because nobody wired the build to a number in the first place.

    Why “it works” isn’t the same as “it moved something”

    I’ve sat in enough steering committees to know the difference between a project that’s technically complete and one that’s actually earning its budget. The first gets a green status and a demo. The second gets renewed funding next year because someone can point at a chart and say “that’s us.”

    Most orgs I’ve reviewed have dozens of automations, reports and custom fields that were built for a stakeholder request, went live, and then quietly stopped being anyone’s responsibility. Nobody’s watching whether they’re doing anything. The gap isn’t in testing. It sits earlier, at the requirements stage, long before a single flow gets built.

    Pick the metric before you open the object manager

    Before any real design work starts, force one question: what single number does this need to move, and who already looks at that number every week? Not a KPI you’ll invent for the project. One that already exists on someone’s dashboard, in someone’s board pack, or in someone’s head when they walk into a Monday meeting.

    For a contact-centre client I worked with, that number was average handle time on a specific case type. Not “improve agent efficiency.” Not “modernise the case workflow.” Handle time, on that case type, measured weekly, already tracked by the operations lead before we touched anything. Every design decision after that got tested against one question: does this reduce handle time on that case type, or doesn’t it?

    That constraint kills a lot of scope creep on its own. A nice-to-have field that doesn’t touch the metric gets deprioritised without an argument. A workaround that adds three clicks but saves the agent from a phone call gets fast-tracked, because someone can show the maths.

    What changes in the technical design

    Once you know the metric, it should show up in three places in the build, not just the final report.

    • The data model needs a field or relationship that captures the event driving the metric, at the point it happens, not reconstructed later from case comments or a separate spreadsheet.
    • The automation needs to fire close enough to that event that the data is accurate. A nightly batch job that recalculates handle time after the fact is a different design than a trigger or flow that stamps timestamps as the case moves through statuses.
    • The reporting layer needs to expose the metric in a way the person who owns it can actually check without asking IT for a custom report every Friday.

    Skip any of those three and you end up with a metric that technically exists but nobody trusts, because getting to it takes fifteen minutes and a pivot table.

    A composite example: the discount approval that didn’t cut leakage

    A typical enterprise org I’ve seen runs something like this. Finance flags discount leakage as a problem. The response is an approval process: discounts above a threshold route to a manager before the quote goes out. It ships. Everyone moves on.

    Eighteen months later, leakage hasn’t moved. The approval process captured whether a discount was approved, not what happened to margin as a result. Nobody built a field for the original list price versus the approved price at the line-item level. Nobody reported on override frequency by manager. The process existed, the metric didn’t, and finance is back asking for the same fix under a different name.

    More approval steps won’t fix that. Going back to the one number that actually matters, margin retained per deal, and checking whether the data model can produce it today, will. Usually it can’t, because the build was scoped around the workflow rather than the measurement.

    The scoping conversation this forces

    When a stakeholder brings a request, I ask them to finish this sentence: “I’ll know this worked when [metric] moves by [amount] by [date].” If they can’t finish it, the request isn’t ready for a solution design session yet. It’s a blunt filter, and a genuinely useful one for separating a real business problem from a feature someone saw in a demo.

    It also changes who signs off. A feature list gets approved by whoever asked for it. A metric commitment gets approved by whoever owns that number, and that person has a much stronger incentive to push back on scope that won’t move it.

    What this looks like in a solution design document

    Practically, add one section before the requirements table: the target metric, its current baseline, who owns it, and how the proposed build is expected to shift it. Keep it to a few lines. If you can’t fill it in, that’s the signal to go back and ask better questions before committing engineering time.

    I’ve started refusing to take a build past discovery without this filled in. It’s cut the number of “why did we build this again” conversations down to almost none, mostly because writing the metric down forces a conversation that usually gets skipped entirely.

    Where this earns its keep

    The payoff isn’t just cleaner requirements. It’s a much easier renewal conversation for whoever’s paying for the work. “We shipped the feature” competes for budget every year. “We moved handle time by 22% on the highest-volume case type” doesn’t need to compete. It gets funded again without a fight.

    That’s the actual argument for doing this on every enhancement, not just the big ones. Small requests accumulate into an org nobody can explain the value of. One metric, tracked from design through to the dashboard, is what turns a backlog of tickets into a track record.

  • Governor limits in bulk trigger design: patterns that actually scale

    Most Apex trigger bugs aren’t logic bugs. They’re volume bugs. Code that works fine in a dev org against three test records falls over the first time someone imports 400 leads or a nightly integration batches 2,000 case updates in one go. By then it’s live, and the failure looks like a mystery instead of what it actually is: a trigger that was never built to run more than once per transaction.

    I’ve reviewed enough trigger code across enterprise orgs to know the failures repeat. Same handful of anti-patterns, different object, different team. Here’s what actually holds up under volume, and where the line sits between “fix the trigger” and “this needs to be async.”

    Why governor limits catch people off guard

    Salesforce doesn’t limit how many records you can touch. It limits how many times you can call certain operations within a single transaction: 100 SOQL queries, 150 DML statements, 10,000 DML rows, 10 seconds of CPU time. A trigger that fires once per record, not once per batch, blows through those limits the moment more than a handful of records land in the same transaction.

    The trap is that single-record testing hides the problem completely. You insert one lead, the trigger fires, one SOQL query runs, everything’s fine. Nobody sees the SOQL-inside-a-for-loop until a data load or an API integration sends 200 records through at once and the transaction dies mid-way with a “Too many SOQL queries: 101” error that tells you almost nothing about which line caused it.

    The pattern that fixes 90% of it: bulkify first, always

    Bulkification isn’t an optimisation you add later. It’s the baseline shape of correct trigger code. The rule is simple: never put a query or a DML statement inside a loop over trigger records.

    Collect, then query once

    Instead of querying related records for each item in Trigger.new, pull every relevant ID into a Set<Id> first, run one SOQL query against the whole set, and build a map keyed by the field you need to match on. Every lookup after that is a map read, not a query. This one change is usually the difference between a trigger that handles 5 records and one that handles 5,000.

    Batch the DML the same way

    Same logic applies on the way out. Build a List of records to update or insert as you process the trigger set, and fire one DML statement at the end. Not one per record, and not one per sub-loop either. If you’re updating related records based on the trigger object, group them into a single list before the DML call, even if that means restructuring the loop to build up state first and act second.

    Use maps instead of nested loops

    Nested for-loops over related record lists look harmless until the data volume climbs. A loop inside a loop is O(n²), and at a few hundred records that’s suddenly tens of thousands of iterations for logic that should have been a single map lookup. If you find yourself looping through child records for every parent record, stop and build a Map<Id, List<SObject>> instead, keyed by the parent ID, populated in one pass.

    The pattern people skip: recursion control

    Bulkification gets the attention. Recursion is the one that actually causes production incidents. Code review rarely catches it, and the failure usually shows up months later, when someone adds a second trigger action that happens to update the same object.

    If your trigger updates the record it’s firing on, or updates a related object whose trigger updates back, you can end up re-entering the same trigger multiple times in one transaction without meaning to. The standard fix is a static boolean flag, set to true the first time the trigger body runs and checked at the top of every subsequent entry, so the second and third passes short-circuit. A cleaner version of the same idea uses a static Set<Id> of record IDs already processed rather than a blunt on/off switch, which matters when the trigger legitimately needs to fire again for records it hasn’t seen yet in the same transaction.

    Either way, don’t assume “it worked in the sandbox test” rules this out. Recursion usually needs a specific chain of updates to trigger, and that chain often doesn’t exist until someone builds a second automation six months after the first one shipped.

    One trigger, one handler, per object

    This is more of an architecture decision than a governor-limit fix directly, but it prevents the two problems above from compounding. Multiple triggers on the same object, each written independently, means multiple separate paths that can each query or DML inside a loop, and no single place to coordinate recursion control or execution order. A trigger handler pattern, one trigger per object that delegates to a handler class with clearly separated before/after insert/update/delete methods, gives you one place to enforce bulkification and one place to manage state across the whole transaction. It also means a new developer can find the logic instead of hunting through however many trigger files have accumulated on a Case object over the years.

    When bulkification isn’t enough

    Sometimes the honest answer isn’t a better trigger. It’s moving the work out of the trigger context entirely. If a trigger needs to make a callout, that can’t happen synchronously at all, callouts from triggers have to go through a Queueable or a future method. If the processing genuinely needs to touch tens of thousands of records, batch Apex with its own governor limit reset per chunk is the right tool, not a trigger straining to stay under 10,000 DML rows in one shot.

    A good rule of thumb: if you’re bulkifying correctly and still hitting limits on realistic data volumes, that’s a signal the logic belongs in an async context, not a sign you need a cleverer synchronous workaround.

    Test at the volume that actually breaks things

    A test class that inserts one record and asserts the trigger fired proves almost nothing about whether the trigger scales. Write at least one test method per trigger that inserts 200 records (the standard batch size Salesforce uses for bulk operations) and confirms it completes without hitting limits. That test will catch the SOQL-in-a-loop mistake in about the time it takes to run, long before it reaches a client’s production org during a data migration or an integration spike.

    None of this is exotic. Bulkify the queries, bulkify the DML, control recursion, keep one handler per object, and push genuinely large or callout-dependent work into async processing. The teams that get burned aren’t missing knowledge of these patterns. They’re skipping them under deadline pressure on the trigger that “only handles a couple of records anyway”, right up until it doesn’t.