-
Notifications
You must be signed in to change notification settings - Fork 393
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: validate the max number of characters of an apex class or trigger #4580
Conversation
…y into SelectFileName
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
QE:
Verified unable to submit file name with greater then 40 chars for class ✅
Verified unable to submit file name with greater then 40 chars for trigger ✅
Verified we are able to submit file name with less then or equal to 40 chars for class ✅
Verified we are able to submit file name with less then or equal to 40 chars for trigger ✅
import { LibraryBaseTemplateCommand } from '../libraryBaseTemplateCommand'; | ||
import { APEX_CLASS_TYPE } from '../metadataTypeConstants'; | ||
|
||
export class LibraryForceApexClassCreateExecutor extends LibraryBaseTemplateCommand< |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Have the executor and main command function both exported from the same file leads to sadness in unit test land so I moved the executor for each of these out to their own file.
fileNameGatherer, | ||
outputDirGatherer, | ||
metadataTypeGatherer | ||
}; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it's good to only every create the one instance of these for the command handler (and didn't want to change functionality) but I wanted to avoid creating instances on loading of the module so I created the function to wrap the creation of the gatherers.
@@ -19,6 +19,9 @@ import { SfdxPackageDirectories } from '../../sfdxProject'; | |||
import { workspaceUtils } from '../../util'; | |||
import { RetrieveDescriber } from '../forceSourceRetrieveMetadata'; | |||
|
|||
export const CONTINUE = 'CONTINUE'; | |||
export const CANCEL = 'CANCEL'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
just a couple of constants that get verified in the unit tests so I exported them here.
SelectFileName | ||
} from '../../../../src/commands/util/parameterGatherers'; | ||
import { nls } from '../../../../src/messages'; | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These are pretty weird to test b/c they depend on some vscode internal functionality. Not my fav to have to mock implementation of showInputBox in each test, but the best flow I could come up with for this validation.
@@ -658,5 +658,7 @@ export const messages = { | |||
lwc_doc_url: 'https://developer.salesforce.com/tools/vscode/en/lwc/writing', | |||
functions_doc_url: | |||
'https://developer.salesforce.com/tools/vscode/en/functions/overview', | |||
default_doc_url: 'https://developer.salesforce.com/tools/vscode' | |||
default_doc_url: 'https://developer.salesforce.com/tools/vscode', | |||
parameter_gathere_file_name_max_length_validation_error_message: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit -> parameter_gatherer_file_name_max_length_validation_error_message
default_doc_url: 'https://developer.salesforce.com/tools/vscode' | ||
default_doc_url: 'https://developer.salesforce.com/tools/vscode', | ||
parameter_gathere_file_name_max_length_validation_error_message: | ||
'Filename cannot exceed {0} characters' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also nit: 'File name
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good but for some nits!
I felt offended that my name was not added as a contributor 😅 🥲 |
Hey @AllanOricil sorry about that. Thanks for calling it out. We've been shifting over from circleci to github actions for publishing and everything has been an adjustment. We'll get that corrected. |
What does this PR do?
validate the max number of characters of an apex class or trigger -> copied to branch for CI from #4533
What issues does this PR fix or reference?
#3624
Functionality Before
SFDX: Create Apex Class
Screen.Recording.2022-11-02.at.22.19.57.mov
SFDX: Create Apex Trigger
Screen.Recording.2022-11-02.at.22.20.55.mov
Functionality After
SFDX: Create Apex Class
Screen.Recording.2022-11-06.at.19.14.36.mov
SFDX: Create Apex Trigger
Screen.Recording.2022-11-06.at.19.15.41.mov