Class Name: PracticeSiteController¶

Last Updated: 2025-10-22 Source Code: https://github.com/AANP-IT/I2C.Salesforce.Metadata/blob/STAGING/force-app/main/default/classes/PracticeSiteController.cls

API Name: PracticeSiteController Type: Controller (LWC) Test Coverage: To be determined

Business Purpose¶

The PracticeSiteController class manages practice site information for nurse practitioners in the NP Finder directory. This LWC controller enables: - Viewing practice sites associated with a user's account - Creating, updating, and deleting practice site records - Managing visibility in the public NP Finder directory - Retrieving picklist values for practice site fields - Handling composite address fields for geolocation

This supports the NP Finder feature where members can list their practice locations for patient discovery.

Class Overview¶

Author: Antoneac Victor
Created: 09-Apr-2025
Last Modified: 15-Apr-2025
Scope/Sharing: without sharing - CRITICAL SECURITY CONCERN
Key Responsibilities:
Query and display practice sites for logged-in user
CRUD operations on PracticeSite__c records
Toggle NP Finder visibility
Dynamic picklist value retrieval
Address field concatenation

This class uses without sharing which bypasses all record-level security. This means: - Any user can access ANY practice site regardless of ownership - No OWD or sharing rules are enforced - Potential security vulnerability if not properly controlled

Justification (Assumed): Likely intended to allow users to manage their own practice sites regardless of org-wide defaults. However, this should be carefully reviewed.

Public Methods¶

getPicklistValues¶

@AuraEnabled(cacheable=true)
public static Map<String, List<String>> getPicklistValues(String objectName, String fieldName)

Purpose: Retrieves active picklist values for a specified field on any object.

Parameters: - objectName (String) - API name of the object (e.g., 'PracticeSite__c') - fieldName (String) - API name of the picklist field

Returns: Map<String, List<String>> - Map with fieldName as key, list of active picklist values

Business Logic: 1. Uses Schema describe to access object metadata (line 17) 2. Retrieves field describe for specified field 3. Iterates through picklist entries 4. Filters to only active entries (line 22) 5. Returns map with fieldName → values

Issues/Concerns: - ⚠️ No Input Validation: Doesn't validate objectName or fieldName exist - ⚠️ No Error Handling: Will throw NullPointerException if field doesn't exist - ⚠️ Unnecessary Map Structure: Returns map with single key (could return List directly) - ⚠️ Debug Statement in Production: Line 28 has System.debug (should be removed) - ✅ Cacheable: Properly marked for LWC caching - ✅ Active Only: Filters to active picklist values

Recommended Improvements:

public

name="__codelineno-1-1" href="#__codelineno-1-1">@AuraEnabled(cacheable=true) static List<String> getPicklistValues(String objectName, String fieldName) { try { Schema.DescribeFieldResult fieldDescribe = Schema.getGlobalDescribe() .get(objectName) .getDescribe() .fields.getMap() .get(fieldName) .getDescribe(); List<String> picklistValues = new List<String>(); for (Schema.PicklistEntry entry : fieldDescribe.getPicklistValues()) { if (entry.isActive()) { picklistValues.add(entry.getValue()); } } return picklistValues; } catch (Exception e) { throw new AuraHandledException('Unable to retrieve picklist values: ' + e.getMessage()); } }

getPracticeSites¶

@AuraEnabled(cacheable=true)
public static List<PracticeSite__c> getPracticeSites(String accountId)

Purpose: Retrieves up to 3 practice sites for a specified account.

Parameters: - accountId (String) - Account record ID to query practice sites for

Returns: List<PracticeSite__c> - Up to 3 practice sites ordered by Name

Business Logic: 1. Queries PracticeSite__c where Account__c = accountId (line 52) 2. Filters to non-null Account__c (line 53) 3. Orders by Name (line 54) 4. Limits to 3 results (line 54)

Issues/Concerns: - ⚠️ Hardcoded LIMIT 3: Why only 3? Should be configurable or removed - ⚠️ No FLS Check: Doesn't validate field-level security - ⚠️ No Error Handling: No try-catch for query failures - ⚠️ WITHOUT SHARING: Can query ANY account's practice sites - ⚠️ Redundant Filter: Line 53 filters Account__c != null but accountId parameter is used in WHERE (unnecessary) - ✅ Cacheable: Properly marked for LWC caching

Security Concern: With without sharing, any user can query practice sites for ANY account by passing different accountIds.

updatePracticeSiteDisplay¶

@AuraEnabled
public static Boolean updatePracticeSiteDisplay(Id siteId)

Purpose: Toggles the Display_On_Np_Finder__c field for a practice site.

Parameters: - siteId (Id) - Practice site record ID

Returns: Boolean - New value of Display_On_Np_Finder__c

Business Logic: 1. Queries PracticeSite__c by Id (lines 61-65) 2. Reads current Display_On_Np_Finder__c value (line 66) 3. Toggles the value (lines 70-76) 4. Updates the record (line 77) 5. Returns new value (line 81)

Issues/Concerns: - 🚨 CRITICAL BUG (line 66): Accesses sites[0] BEFORE checking if list is empty - Will throw IndexOutOfBoundsException if site not found - Lines 68-80 check isEmpty AFTER already accessing [0] - ⚠️ WITHOUT SHARING: Can toggle ANY practice site's visibility - ⚠️ Misleading Exception: Line 79 says "no permission" but real issue is site not found - ⚠️ Variable Redundancy: isDisplayed and isDisplayedOnNpFinder both track same value - ⚠️ No FLS Check: Doesn't validate user can update Display_On_Np_Finder__c field

Correct Logic:

@AuraEnabled
public static Boolean updatePracticeSiteDisplay(Id siteId) {
    List<PracticeSite__c> sites = [
        SELECT Id, Display_On_Np_Finder__c
        FROM PracticeSite__c
        WHERE Id = :siteId
        LIMIT 1
    ];

    if (sites.isEmpty()) {
        throw new AuraHandledException('Practice site not found.');
    }

    PracticeSite__c site = sites[0];
    site.Display_On_Np_Finder__c = !site.Display_On_Np_Finder__c;
    update site;

    return site.Display_On_Np_Finder__c;
}

updatePracticeSite¶

@AuraEnabled
public static void updatePracticeSite(Map<String, Object> updatedSite)

Purpose: Updates a practice site record from LWC form data, handling address field concatenation.

Parameters: - updatedSite (Map) - Field name → value map from LWC

Returns: void

Business Logic: 1. Creates new PracticeSite__c sobject (line 87) 2. Iterates through provided fields (line 92) 3. Extracts Address_Line_1/2/3__c values separately (lines 93-98) 4. Skips Address__c field (lines 99-100) 5. Sets all other fields on sobject (line 102) 6. Concatenates address lines (line 106) 7. Sets Address__Street__s field (lines 107-108) 8. Updates record (line 109)

Issues/Concerns: - 🚨 CRITICAL BUG: No Id field set on siteToUpdate - Line 87 creates new sobject without Id - Line 109 update will fail or create duplicate - Must extract Id from updatedSite map - ⚠️ Duplicate Assignment: Lines 107 and 108 both set Address__Street__s - ⚠️ No Input Validation: Doesn't validate updatedSite is non-null - ⚠️ No FLS Check: Doesn't validate field-level security - ⚠️ Generic Field Setting: Line 102 uses dynamic field assignment (no type safety) - ⚠️ Empty Address Handling: concatenateAddress doesn't handle null/blank strings well

Correct Implementation:

@AuraEnabled
public static void updatePracticeSite(Map<String, Object> updatedSite) {
    if (updatedSite == null || !updatedSite.containsKey('Id')) {
        throw new AuraHandledException('Site Id is required for update.');
    }

    PracticeSite__c siteToUpdate = new PracticeSite__c(
        Id = (Id)updatedSite.get('Id')
    );

    String addressLine1 = '';
    String addressLine2 = '';
    String addressLine3 = '';

    for (String fieldName : updatedSite.keySet()) {
        if (fieldName == 'Address_Line_1__c') {
            addressLine1 = (String) updatedSite.get(fieldName);
        } else if (fieldName == 'Address_Line_2__c') {
            addressLine2 = (String) updatedSite.get(fieldName);
        } else if (fieldName == 'Address_Line_3__c') {
            addressLine3 = (String) updatedSite.get(fieldName);
        } else if (fieldName != 'Address__c' && fieldName != 'Id') {
            siteToUpdate.put(fieldName, updatedSite.get(fieldName));
        }
    }

    String concatenatedAddress = concatenateAddress(addressLine1, addressLine2, addressLine3);
    siteToUpdate.Address__Street__s = concatenatedAddress;

    update siteToUpdate;
}

deletePracticeSite¶

@AuraEnabled
public static void deletePracticeSite(Map<String, Object> deleteSite)

Purpose: Deletes a practice site record.

Parameters: - deleteSite (Map) - Map containing 'Id' key with site Id

Returns: void

Business Logic: 1. Checks if map contains 'Id' key (line 114) 2. Extracts Id from map (line 115) 3. Deletes record via inline query (line 116) 4. Throws exception if no Id provided (line 118)

Issues/Concerns: - ⚠️ No Confirmation: Deletes without additional validation - ⚠️ WITHOUT SHARING: Can delete ANY practice site - ⚠️ No Record Validation: Doesn't verify site exists before delete - ⚠️ No Error Handling: Delete query exception not caught - ✅ Id Validation: Properly checks for Id presence

createPracticeSite¶

@AuraEnabled
public static void createPracticeSite(Map<String, Object> newSite, String accountId)

Purpose: Creates a new practice site record from LWC form data.

Parameters: - newSite (Map) - Field name → value map from LWC - accountId (String) - Account to associate practice site with

Returns: void

Business Logic: 1. Validates newSite is non-null (lines 124-126) 2. Creates new PracticeSite__c sobject (line 128) 3. Extracts address line fields (lines 130-145) 4. Skips Address__c and Id fields (line 140) 5. Sets all other fields dynamically (line 143) 6. Concatenates address lines (line 147) 7. Sets Address__Street__s (lines 148-149) 8. Sets Account__c lookup (line 150) 9. Inserts record (line 152)

Issues/Concerns: - ⚠️ Duplicate Address Assignment: Lines 148 and 149 both set Address__Street__s - ⚠️ No Account Validation: Doesn't verify accountId is valid - ⚠️ No FLS Check: Doesn't validate create permissions - ⚠️ WITHOUT SHARING: Can create practice site for ANY account - ⚠️ Generic Field Setting: Dynamic field assignment lacks type safety - ✅ Null Check: Validates newSite parameter

concatenateAddress¶

public static String concatenateAddress(String addressLine1, String addressLine2, String addressLine3)

Purpose: Concatenates three address line strings with comma separators.

Parameters: - addressLine1 (String) - First line of address - addressLine2 (String) - Second line of address - addressLine3 (String) - Third line of address

Returns: String - Concatenated address with ", " separators

Business Logic: 1. Uses String.join to concatenate array (line 158) 2. Joins with ", " separator

Issues/Concerns: - ⚠️ No Null Handling: Doesn't check for null/empty strings - ⚠️ Empty String Handling: Produces ", , " if strings are empty - ⚠️ Trailing Commas: May produce addresses like "123 Main St, , "

Recommended Improvement:

public static String concatenateAddress(String addressLine1, String addressLine2, String addressLine3) {
    List<String> addressParts = new List<String>();

    if (String.isNotBlank(addressLine1)) addressParts.add(addressLine1);
    if (String.isNotBlank(addressLine2)) addressParts.add(addressLine2);
    if (String.isNotBlank(addressLine3)) addressParts.add(addressLine3);

    return String.join(addressParts, ', ');
}

Dependencies¶

Salesforce Objects¶

PracticeSite__c (Custom Object)
Fields: 50+ fields including Address, Demographics, Clinical Focus, etc.
Access: Read, Create, Update, Delete
Account (Standard Object)
Relationship: PracticeSite__c.Account__c lookup
Access: Read (for PersonAccount detection)

Custom Settings/Metadata¶

None

Other Classes¶

None (standalone controller)

External Services¶

None

Design Patterns¶

LWC Controller Pattern: @AuraEnabled methods for Lightning Web Components
DTO Pattern: Map for flexible field passing
Helper Method: concatenateAddress utility
Dynamic Field Assignment: Uses sobject.put() for generic field setting

Governor Limits Considerations¶

Current Impact (Per Transaction)¶

SOQL Queries: 1-2 per method (getPracticeSites, updatePracticeSiteDisplay, etc.)
DML Statements: 1 per CUD operation
Heap Size: Minimal (small number of records)

Scalability Analysis¶

✅ Single Record Operations: All methods process 1 record at a time
✅ LIMIT 3: getPracticeSites limited to 3 records
⚠️ No Bulk Support: Methods designed for single-record LWC operations
⚠️ Dynamic Field Assignment: Heap usage depends on number of fields passed

Error Handling¶

Exception Types Thrown¶

AuraHandledException: Lines 79, 118, 125
IndexOutOfBoundsException: Line 66 (bug)
QueryException: Unhandled query failures
DmlException: Unhandled DML failures

Error Handling Gaps¶

No Try-Catch: Most methods lack error handling
Misleading Messages: Line 79 says "no permission" but means "not found"
Silent Failures: Errors not logged for debugging
Index Access Bug: Line 66 accesses array before checking isEmpty

Security Considerations¶

🚨 CRITICAL: WITHOUT SHARING¶

This class uses without sharing which creates severe security vulnerabilities:

Data Exposure: Any user can query ANY account's practice sites
Unauthorized Modification: Any user can update/delete ANY practice site
Cross-Account Access: No validation that user owns the account
NP Finder Manipulation: Any user can toggle visibility for any site

Recommended Fix: Change to with sharing and add explicit ownership checks:

public with sharing class PracticeSiteController {

    private static void validateOwnership(Id siteId) {
        List<PracticeSite__c> sites = [
            SELECT Account__c
            FROM PracticeSite__c
            WHERE Id = :siteId
            LIMIT 1
        ];

        if (sites.isEmpty()) {
            throw new AuraHandledException('Practice site not found.');
        }

        Id userId = UserInfo.getUserId();
        User currentUser = [SELECT AccountId FROM User WHERE Id = :userId];

        if (sites[0].Account__c != currentUser.AccountId) {
            throw new AuraHandledException('You do not have permission to access this practice site.');
        }
    }

    // Call validateOwnership in all CUD methods
}

FLS Considerations¶

⚠️ No FLS Checks: Doesn't validate field-level security
Fields Accessed: 50+ fields on PracticeSite__c
Risk: Users could access restricted fields

Test Class Requirements¶

Required Test Coverage¶

See PracticeSiteControllerTest.cls (154 lines)

Changes & History¶

Date	Author	Description
2025-04-09	Antoneac Victor	Initial implementation
2025-04-15	Antoneac Victor	Last modification
(Current)	-	Documentation added

Pre-Go-Live Concerns¶

🚨 CRITICAL¶

WITHOUT SHARING: Severe security vulnerability allowing cross-account access
Change to with sharing
Add ownership validation to all methods
Index Out of Bounds Bug (line 66): Accessing sites[0] before isEmpty check
Will crash if site not found
Fix immediately

HIGH¶

Missing Id in updatePracticeSite: siteToUpdate has no Id, update will fail
Extract Id from updatedSite map
Set Id on sobject before update
No Account Validation: createPracticeSite doesn't verify accountId exists
Add account existence check
No FLS Validation: All methods bypass field-level security
Add FLS checks or document assumption

MEDIUM¶

Hardcoded LIMIT 3: getPracticeSites arbitrarily limits to 3 sites
Make configurable or remove limit
Poor Address Handling: concatenateAddress doesn't handle nulls/blanks
Filter empty strings before joining
Debug Statement: Line 28 has System.debug in production code
Remove before deployment

LOW¶

Duplicate Assignments: Lines 107-108, 148-149 set same field twice
Remove redundant line
Misleading Error Messages: Line 79 says "no permission" but means "not found"

Maintenance Notes¶

📋 Monitoring Recommendations¶

Security Audits: Monitor for cross-account practice site access
NP Finder Usage: Track visibility toggle frequency
Error Rates: Monitor AuraHandledException occurrences

🔧 Future Enhancement Opportunities¶

Bulk Operations: Support creating multiple sites at once
Address Validation: Integrate with address validation API
Geolocation: Auto-populate lat/long from address
Ownership Validation: Add explicit security checks

⚠️ Breaking Change Risks¶

Changing to with sharing will restrict data access (correct behavior)
Fixing updatePracticeSite Id bug requires LWC to pass Id in map
Changing method signatures breaks existing LWC components

NP Finder LWC: Uses these methods to display/edit practice sites
PracticeSite__c Object: Custom object for practice location data
Geolocation Services: May integrate for address geocoding

Business Owner¶

Primary Contact: Member Services / NP Finder Team Technical Owner: Antoneac Victor / Salesforce Development Team Created: 2025-04-09 Last Reviewed: [Date]

Documentation Status: ✅ Complete Code Review Status: 🚨 CRITICAL - WITHOUT SHARING security risk, index out of bounds bug Test Coverage: Test class exists (PracticeSiteControllerTest.cls)

Class Name: PracticeSiteController¶

Business Purpose¶

Class Overview¶

⚠️ CRITICAL: WITHOUT SHARING¶

Public Methods¶

getPicklistValues¶

getPracticeSites¶

updatePracticeSiteDisplay¶

updatePracticeSite¶

deletePracticeSite¶

createPracticeSite¶

concatenateAddress¶

Dependencies¶

Salesforce Objects¶

Custom Settings/Metadata¶

Other Classes¶

External Services¶

Design Patterns¶

Governor Limits Considerations¶

Current Impact (Per Transaction)¶

Scalability Analysis¶

Error Handling¶

Exception Types Thrown¶

Error Handling Gaps¶

Security Considerations¶

🚨 CRITICAL: WITHOUT SHARING¶

FLS Considerations¶

Test Class Requirements¶

Required Test Coverage¶

Changes & History¶

Pre-Go-Live Concerns¶

🚨 CRITICAL¶

HIGH¶

MEDIUM¶

LOW¶

Maintenance Notes¶

📋 Monitoring Recommendations¶

🔧 Future Enhancement Opportunities¶

⚠️ Breaking Change Risks¶

🔗 Related Components¶

Business Owner¶