SalesforceLabs / survey-force

85 stars 61 forks source link

Visual Studio Code Throwing Errors #97

Closed alvogt closed 3 years ago

alvogt commented 3 years ago

I pulled this into Visual Studio Code. It has a bunch of warnings. I was able to fix some of them but, I don't fully understand all of them. These are the ones I'm stuck on. I really would appreciate advice to resolve these. (I'm a mid level coder. Adding stuff to existing code is mostly my skill set)

global virtual with sharing class ViewSurveyController { ERROR: Avoid using global modifier (rule: Best Practices-AvoidGlobalModifier)apex pmdAvoidGlobalModifier The class 'ViewSurveyController' has a total cognitive complexity of 135 (highest 73). (rule: Design-CognitiveComplexity)apex pmdCognitiveComplexity This class has too many public methods and attributes (rule: Design-ExcessivePublicCount)apex pmdExcessivePublicCount

public ViewSurveyController(ApexPages.StandardController stdController) { void ViewSurveyController.(ApexPages.StandardController stdController) The constructor has an NCSS line count of 29 (rule: Design-NcssConstructorCount)apex pmdNcssConstructorCount

surveyId = String.escapeSingleQuotes(surveyId); caseId = String.escapeSingleQuotes(caseId); contactId = String.escapeSingleQuotes(contactId); campaignId = String.escapeSingleQuotes(campaignId); ( I added Campaign and demo Products to the surveys) demoProductId = String.escapeSingleQuotes(demoProductId); String Apex classes should escape Strings obtained from URL parameters (rule: Security-ApexXSSFromURLParam)apex pmdApexXSSFromURLParam

public ViewSurveyController(ViewShareSurveyComponentController controller) void ViewSurveyController.(ViewShareSurveyComponentController controller) The constructor has an NCSS line count of 23 (rule: Design-NcssConstructorCount)apex pmdNcssConstructorCount

setSurveyNameAndThankYou(surveyId); void ViewSurveyController.setSurveyNameAndThankYou(String sId) Sets the survey name variable param: sID The survey ID as specified in the DB

Apex classes should escape Strings obtained from URL parameters (rule: Security-ApexXSSFromURLParam)apex pmdApexXSSFromURLParam

public PageReference submitResults() System.Pagereference The method 'submitResults()' has a cognitive complexity of 73. (rule: Design-CognitiveComplexity)apex pmdCognitiveComplexity The method submitResults() has an NCSS line count of 78 (rule: Design-NcssMethodCount)apex pmdNcssMethodCount

private Boolean addSurveyTaker() Boolean ViewSurveyController.addSurveyTaker() The method 'addSurveyTaker()' has a cognitive complexity of 30. (rule: Design-CognitiveComplexity)apex pmdCognitiveComplexity The method addSurveyTaker() has an NCSS line count of 62 (rule: Design-NcssMethodCount)apex pmdNcssMethodCount

A few of the things I did fix were changing naming conventions: oldOrderNumber, newOrderNumber (No _) AddSurveyTaker changed to addSurveyTaker

And, checking CRUD: if(anonymousAnswer != 'Anonymous') { //If survey is taken by Contact OR Case already then no need to take it again if ( Schema.sObjectType.SurveyTakerc.fields.Id.isAccessible()&& Schema.sObjectType.SurveyTakerc.fields.Contactc.isAccessible() && Schema.sObjectType.SurveyTakerc.fields.Surveyc.isAccessible() && Schema.sObjectType.SurveyTakerc.fields.Casec.isAccessible() && Schema.sObjectType.SurveyTakerc.fields.Campaignc.isAccessible() && Schema.sObjectType.SurveyTakerc.fields.Demo_Productc.isAccessible() && Schema.sObjectType.SurveyTakerc.fields.Userc.isAccessible() ) { // do something //return [SELECT CaseNumber, Id, RecordTypeId FROM Case WHERE ID=:caseId]; List<SurveyTakerc> check = [SELECT Contactc, Surveyc, Casec, Campaignc,Demo_Productc, Userc From SurveyTakerc Where Surveyc=:surveyId and ((Contactc != null and Contactc=:contactId) or (Casec!= null and Casec = :caseId) or (Campaignc!= null and Campaignc = :campaignId) or (Demo_Product__c!= null and Demo_Productc = :demoProductId) or (Userc!= null and User__c = :userId))]; if(check != null && check.size()>0){ surveyPreTaken = true; Apexpages.addMessage(new ApexPages.Message(ApexPages.Severity.ERROR, System.Label.LABS_SF_You_have_already_taken_this_survey));

            return false;
        }
        }
     return null; 

    }
jrattanpal commented 3 years ago

These are warnings telling you to possibly make changes to code if needed. I try to avoid classes and methods unless there is no other way to achieve this; if you really need it then only use primitive function parameters (integer, boolean etc; avoid using sObjects).

To add, this is really old code and may require a complete overhaul to fix it entirely. But it's currently in maintenance mode and we don't have plan to re-write this app. Having said that, a while back, we did a huge cleanup of the code to fix many of those issues. We will see if we can cleanup further but nothing on our roadmap yet.

jrattanpal commented 3 years ago

I will close this issue for now. If this issue persists then you can reopen this with some logs