Repository navigation
Conversation
There was a problem hiding this comment.
Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.
An organization owner can choose the repository organization as the billing source for reviews requested by members with a paid Copilot license. Learn about Copilot code review billing.
| @@ -0,0 +1,22 @@ | |||
| { | |||
There was a problem hiding this comment.
PMDSecurityDomainRule (ACTION)
PMD page is missing required securityDomains list. All PMD pages must define at least one security domain (excludes microConclusion and error pages).
Why: Ensures PMD pages have at least one security domain defined (excludes microConclusion and error pages unless strict mode is enabled)
| "authType": "sso", | ||
| "deferred": true, | ||
| "onSend": "<% self.data = {:}; self.data.query = pageVariables.orgQuery; self.data; %>", | ||
| "failOnStatusCodes": [ |
There was a problem hiding this comment.
EndpointFailOnStatusCodesRule (ACTION)
Inbound endpoint 'orgsWql' is missing required status codes: 400.
Why: Ensures endpoints have failOnStatusCodes with minimum required codes 400 and 403
Suggested change:
{"code": 400}
| * Reference IDs never contain commas, so quoted cells are rejected with a clear message instead of parsed. | ||
| */ | ||
|
|
||
| var cell = function(cols, i) { |
There was a problem hiding this comment.
ScriptVarUsageRule (ADVICE)
File section 'script' uses 'var' declaration for variable 'cell'. Consider using 'let' or 'const' instead.
Why: Ensures scripts use 'let' or 'const' instead of 'var' (best practice)
Suggested change
| var cell = function(cols, i) { | |
| let cell = function(cols, i) { |
| // string:split drops empty tokens, which would shift columns left when a cell is blank | ||
| // (e.g. a blank Company_ID would push Region_ID into the Company column). Wrapping every | ||
| // cell in '~' keeps each one non-empty; cell() strips the marker again. | ||
| var splitCells = function(line) { |
There was a problem hiding this comment.
ScriptVarUsageRule (ADVICE)
File section 'script' uses 'var' declaration for variable 'splitCells'. Consider using 'let' or 'const' instead.
Why: Ensures scripts use 'let' or 'const' instead of 'var' (best practice)
Suggested change
| var splitCells = function(line) { | |
| let splitCells = function(line) { |
| // (e.g. a blank Company_ID would push Region_ID into the Company column). Wrapping every | ||
| // cell in '~' keeps each one non-empty; cell() strips the marker again. | ||
| var splitCells = function(line) { | ||
| return string:split('~' + line.replace('~', '').replace(',', '~,~') + '~', ','); |
There was a problem hiding this comment.
ScriptStringConcatRule (ADVICE)
File section 'script' uses string concatenation with + operator in function 'splitCells': ''' + line.replace''''.replace','',' + '~''. Consider using PMD template strings with backticks and {{ }} syntax instead (e.g., Hello {{name}}!).
Why: Detects string concatenation with + operator - use PMD templates with backticks and {{ }} instead
Suggested change:
`~{{line.replace'~'''.replace',''~,~'}}~`
| 'hasQuotes': line.contains('"'), | ||
| 'employeeId': cell(cols, 0), 'positionId': cell(cols, 1), 'effectiveDate': cell(cols, 2), | ||
| 'costCenterRefId': cell(cols, 3), 'companyRefId': cell(cols, 4), 'regionRefId': cell(cols, 5), | ||
| 'customOrgs': list:filter([cell(cols, 6), cell(cols, 7), cell(cols, 8)], c => !string:isBlank(c)) |
There was a problem hiding this comment.
ScriptMagicNumberRule (ADVICE)
File section 'script' contains magic number '5' in function 'parseCsv': ''customOrgs': list:filter([cell(cols, 6), cell(cols, 7), cell(cols, 8)], c => !string:isBlank(c))'. Consider using a named constant instead.
Why: Ensures scripts don't contain magic numbers (use named constants)
| 'employeeId': cell(cols, 0), 'positionId': cell(cols, 1), 'effectiveDate': cell(cols, 2), | ||
| 'costCenterRefId': cell(cols, 3), 'companyRefId': cell(cols, 4), 'regionRefId': cell(cols, 5), | ||
| 'customOrgs': list:filter([cell(cols, 6), cell(cols, 7), cell(cols, 8)], c => !string:isBlank(c)) | ||
| }); |
There was a problem hiding this comment.
ScriptDescriptiveParameterRule (ADVICE)
Parameter 'c' in parseCsv (calls arrow_function()) in function 'parseCsv' should be more descriptive. Consider using a more descriptive name, such as 'param1' instead. Single-letter parameters make functions that take function parameters harder to read and debug.
Why: Ensures function parameters use descriptive names when functions take function parameters (except 'a', 'b' for sort)
| 'employeeId': cell(cols, 0), 'positionId': cell(cols, 1), 'effectiveDate': cell(cols, 2), | ||
| 'costCenterRefId': cell(cols, 3), 'companyRefId': cell(cols, 4), 'regionRefId': cell(cols, 5), | ||
| 'customOrgs': list:filter([cell(cols, 6), cell(cols, 7), cell(cols, 8)], c => !string:isBlank(c)) | ||
| }); |
There was a problem hiding this comment.
ScriptMagicNumberRule (ADVICE)
File section 'script' contains magic number '6' in function 'parseCsv': '});'. Consider using a named constant instead.
Why: Ensures scripts don't contain magic numbers (use named constants)
| 'employeeId': cell(cols, 0), 'positionId': cell(cols, 1), 'effectiveDate': cell(cols, 2), | ||
| 'costCenterRefId': cell(cols, 3), 'companyRefId': cell(cols, 4), 'regionRefId': cell(cols, 5), | ||
| 'customOrgs': list:filter([cell(cols, 6), cell(cols, 7), cell(cols, 8)], c => !string:isBlank(c)) | ||
| }); |
There was a problem hiding this comment.
ScriptMagicNumberRule (ADVICE)
File section 'script' contains magic number '7' in function 'parseCsv': '});'. Consider using a named constant instead.
Why: Ensures scripts don't contain magic numbers (use named constants)
| 'employeeId': cell(cols, 0), 'positionId': cell(cols, 1), 'effectiveDate': cell(cols, 2), | ||
| 'costCenterRefId': cell(cols, 3), 'companyRefId': cell(cols, 4), 'regionRefId': cell(cols, 5), | ||
| 'customOrgs': list:filter([cell(cols, 6), cell(cols, 7), cell(cols, 8)], c => !string:isBlank(c)) | ||
| }); |
There was a problem hiding this comment.
ScriptMagicNumberRule (ADVICE)
File section 'script' contains magic number '8' in function 'parseCsv': '});'. Consider using a named constant instead.
Why: Ensures scripts don't contain magic numbers (use named constants)
|
|
||
| // Step 2: build WHERE ... IN lists so WQL only fetches the IDs in this file | ||
| var quoteList = function(ids) { | ||
| return list:distinct(list:filter(ids, i => !string:isBlank(i))).map(i => { "'" + i + "'" }).join(', '); |
There was a problem hiding this comment.
ScriptVarUsageRule (ADVICE)
File section 'script' uses 'var' declaration for variable 'quoteList'. Consider using 'let' or 'const' instead.
Why: Ensures scripts use 'let' or 'const' instead of 'var' (best practice)
Suggested change:
let
| // Step 2: build WHERE ... IN lists so WQL only fetches the IDs in this file | ||
| var quoteList = function(ids) { | ||
| return list:distinct(list:filter(ids, i => !string:isBlank(i))).map(i => { "'" + i + "'" }).join(', '); | ||
| }; |
There was a problem hiding this comment.
ScriptDescriptiveParameterRule (ADVICE)
Parameter 'i' in quoteList (calls map()) in function 'quoteList' should be more descriptive. Consider using a more descriptive name, such as 'item' instead. Single-letter parameters make functions that take function parameters harder to read and debug.
Why: Ensures function parameters use descriptive names when functions take function parameters (except 'a', 'b' for sort)
| // Step 2: build WHERE ... IN lists so WQL only fetches the IDs in this file | ||
| var quoteList = function(ids) { | ||
| return list:distinct(list:filter(ids, i => !string:isBlank(i))).map(i => { "'" + i + "'" }).join(', '); | ||
| }; |
There was a problem hiding this comment.
ScriptDescriptiveParameterRule (ADVICE)
Parameter 'i' in quoteList (calls map()) in function 'quoteList' should be more descriptive. Consider using a more descriptive name, such as 'item' instead. Single-letter parameters make functions that take function parameters harder to read and debug.
Why: Ensures function parameters use descriptive names when functions take function parameters (except 'a', 'b' for sort)
| // Step 2: build WHERE ... IN lists so WQL only fetches the IDs in this file | ||
| var quoteList = function(ids) { | ||
| return list:distinct(list:filter(ids, i => !string:isBlank(i))).map(i => { "'" + i + "'" }).join(', '); | ||
| }; |
There was a problem hiding this comment.
ScriptStringConcatRule (ADVICE)
File section 'script' uses string concatenation with + operator in function 'quoteList': '"'" + i + "'"'. Consider using PMD template strings with backticks and {{ }} syntax instead (e.g., Hello {{name}}!).
Why: Detects string concatenation with + operator - use PMD templates with backticks and {{ }} instead
Suggested change:
`'{{i}}'`
| return list:distinct(list:filter(ids, i => !string:isBlank(i))).map(i => { "'" + i + "'" }).join(', '); | ||
| }; | ||
| var workerQuery = function(rows) { | ||
| return 'SELECT employeeID, positionID FROM workersForHCMReporting (dataSourceFilter = allActiveWorkers) WHERE employeeID IN (' + quoteList(rows.map(r => { r.employeeId })) + ')'; |
There was a problem hiding this comment.
ScriptVarUsageRule (ADVICE)
File section 'script' uses 'var' declaration for variable 'workerQuery'. Consider using 'let' or 'const' instead.
Why: Ensures scripts use 'let' or 'const' instead of 'var' (best practice)
Suggested change:
let
| }; | ||
| var ref = function(element, idType, value) { | ||
| return '<bsvc:' + element + '><bsvc:ID bsvc:type="' + idType + '">' + xmlEscape(value) + '</bsvc:ID></bsvc:' + element + '>'; | ||
| }; |
There was a problem hiding this comment.
ScriptStringConcatRule (ADVICE)
File section 'script' uses string concatenation with + operator in function 'ref': ''<bsvc:' + element + '><bsvc:ID bsvc:type="' + idType + '">' + xmlEscapevalue + '</bsvc:ID></bsvc:' + element + '>''. Consider using PMD template strings with backticks and {{ }} syntax instead (e.g., Hello {{name}}!).
Why: Detects string concatenation with + operator - use PMD templates with backticks and {{ }} instead
Suggested change:
`<bsvc:{{element}}><bsvc:ID bsvc:type="{{idType}}">{{xmlEscapevalue}}</bsvc:ID></bsvc:{{element}}>`
| // Matches Staffing v42.0 Change_Organization_Assignments_Request. Blank columns are omitted, | ||
| // which tells Workday to keep the existing assignment. | ||
| var buildSoap = function(r, autoComplete) { | ||
| var orgs = ''; |
There was a problem hiding this comment.
ScriptFunctionReturnConsistencyRule (ADVICE)
Function 'buildSoap' has some return statements but not all code paths return - consider adding else branches or a final return statement
Why: Functions should have consistent return patterns - either all code paths return a value or none do
| // Matches Staffing v42.0 Change_Organization_Assignments_Request. Blank columns are omitted, | ||
| // which tells Workday to keep the existing assignment. | ||
| var buildSoap = function(r, autoComplete) { | ||
| var orgs = ''; |
There was a problem hiding this comment.
ScriptVarUsageRule (ADVICE)
File section 'script' uses 'var' declaration for variable 'buildSoap'. Consider using 'let' or 'const' instead.
Why: Ensures scripts use 'let' or 'const' instead of 'var' (best practice)
Suggested change:
let
| // which tells Workday to keep the existing assignment. | ||
| var buildSoap = function(r, autoComplete) { | ||
| var orgs = ''; | ||
| if (!string:isBlank(r.companyRefId)) { orgs += ref('Company_Assignments_Reference', 'Company_Reference_ID', r.companyRefId); } |
There was a problem hiding this comment.
ScriptVarUsageRule (ADVICE)
File section 'script' uses 'var' declaration for variable 'orgs' in function 'buildSoap'. Consider using 'let' or 'const' instead.
Why: Ensures scripts use 'let' or 'const' instead of 'var' (best practice)
Suggested change:
let
| if (!string:isBlank(r.costCenterRefId)) { orgs += ref('Cost_Center_Assignments_Reference', 'Cost_Center_Reference_ID', r.costCenterRefId); } | ||
| if (!string:isBlank(r.regionRefId)) { orgs += ref('Region_Assignments_Reference', 'Region_Reference_ID', r.regionRefId); } | ||
| for (var c : r.customOrgs) { | ||
| orgs += '<bsvc:Custom_Organization_Assignment_Data>' + ref('Custom_Organization_Assignment_Reference', 'Custom_Organization_Reference_ID', c) + '</bsvc:Custom_Organization_Assignment_Data>'; |
There was a problem hiding this comment.
ScriptVarUsageRule (ADVICE)
File section 'script' uses 'var' declaration for variable 'var' in for-in loop. Consider using 'let' or 'const' instead.
Why: Ensures scripts use 'let' or 'const' instead of 'var' (best practice)
Suggested change:
let
Example auditAudited 2 to fix and 90 suggestion(s). Fix before merge
Suggestions (90, never block)
Run it yourself: ACTION items fail the Audit examples check. ADVICE never blocks. Maintainers can add the 1 finding(s) are file-level and appear only in this summary. |
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this PR adds or changes
Adds examples/smart-csv-org-loader: an Extend app and async orchestration that loads organization assignment changes from a flat one-row-per-worker CSV, instead of multi-row Assign Organization EIB spreadsheets. It pre-validates workers, positions, dates and duplicates with WQL, calls Staffing Change_Organization_Assignments (v42.0) in the background, and writes each row's COMPLETED or SOAP_FAULT result back to a business object shown in Batch history.
Checklist for new or changed examples
Anything reviewers should know?