fix: Resolve <UNDEFINED> error in 'generate' command WebApp processing #970
Conversation
AshokThangavel
requested review from
isc-dchui,
isc-egabhart,
isc-eneil,
isc-jili,
isc-jlechtne,
isc-kiyer,
isc-pbarton and
isc-tleavitt
as code owners
isc-kiyer
left a comment
isc-kiyer
left a comment
There was a problem hiding this comment.
@AshokThangavel The fix looks good to me! Please add a corresponding changelog entry and then I will approve
|
Great, thank you for the review. I’ve added the corresponding changelog entry. |
| set tPath = props("Path") | ||
| }else{ | ||
| set tAppName = $listget(tAppList,i) | ||
| set tAutheEnabled = 64 //Unauthenticated |
There was a problem hiding this comment.
If we must have a default, I'd prefer 32 (password) over 64 (unauthenticated) - also consider using macros from %sySecurity.inc
There was a problem hiding this comment.
(e.g., $$$AuthePassword)
There was a problem hiding this comment.
Thank you! Fixed: I've applied the fix, updating the default authentication preference (32 over 64) and converting the values to use system macros.
| set ..TemplateResources(tAppName,"PasswordAuthEnabled") = 1 | ||
| } elseif ( props("AutheEnabled") = 64 ) { | ||
| } elseif ( tAutheEnabled = 64 ) { | ||
| set ..TemplateResources(tAppName,"UnauthenticatedEnabled") = 0 |
There was a problem hiding this comment.
Worth fixing this typo while we're at it - if tAutheEnabled = 64 we should set UnauthenticatedEnabled, not PasswordAuthEnabled!
There was a problem hiding this comment.
Thank you!. I've corrected the logic.
|
|
||
| new $namespace | ||
| set $namespace = "%SYS" | ||
| set (tMatchRoles,tDispatchClass,tAutheEnabled,tPath)="" |
There was a problem hiding this comment.
nit: for net new code, we do not use t-prefixes. So these should be defined as matchRoles, dispatchClass, etc
There was a problem hiding this comment.
Removed the t prefix from variable names.
Thanks!
| set tAutheEnabled = props("AutheEnabled") | ||
| set tDispatchClass = props("DispatchClass") | ||
| set tPath = props("Path") | ||
| }else{ |
There was a problem hiding this comment.
nit: conventionally there should be a space before and after else
There was a problem hiding this comment.
I’ve fixed the spacing around else to follow the convention.
|
|
||
| Method TestFixUndefinedCLIGenCommand() | ||
| { | ||
| set tWebAppList = "/test/test" |
There was a problem hiding this comment.
Removed the t prefix from variable names.
4 participants
Fix: Resolve
<UNDEFINED>Error ingenerateWebApp ProcessingFixes #969
This Pull Request addresses a bug in the
generatecommand that caused the command to fail when a user attempted to specify a web application path during module creation.📝 Bug Summary
The
generatecommand, which relies on the%IPM.Storage.ModuleTemplateclass to process configuration, failed with an<UNDEFINED>error when the user provided input for the prompt. The error prevented the module template from being successfully created whenever web application properties were involved.Original Error Trace:
🛠️ Key Fix
The issue was traced to the
AddWebAppsmethod within%IPM.Storage.ModuleTemplate. The fix involves ensuring that all required properties within the method are correctly defined and initialized before any attempt is made to access or process them.The code is now robust against missing or improperly structured input variables during the web app configuration phase.
🧪 Unit Test Verification
The bug fix was validated using a targeted unit test that directly called the failing method with the problematic input.
This confirms that the
AddWebAppsmethod now processes the web application list correctly and returns an OK status, resolving the<UNDEFINED>error in isolation.