-
Notifications
You must be signed in to change notification settings - Fork 48
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
PCHR-3742: Disabling or Deleting Options Breaks Job Roles Tab #2646
PCHR-3742: Disabling or Deleting Options Breaks Job Roles Tab #2646
Conversation
updatedRole.department = (updatedRole.department === undefined) ? updatedRole.department = '' : updatedRole.department; | ||
updatedRole.region = (updatedRole.region === undefined) ? updatedRole.region = '' : updatedRole.region; | ||
// Ensure location, level, department, region exist and not disabled | ||
updateLocation = _.includes(_.keys(vm.LocationsData), updatedRole.location); |
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.
Please move the code to a separate function with a meaningful name, there is too much duplication.
updateDepartment = _.includes(_.keys(vm.DepartmentsData), updatedRole.department); | ||
updateRegion = _.includes(_.keys(vm.RegionsData), updatedRole.region); | ||
|
||
updatedRole.location = (updatedRole.location === undefined || !updateLocation) ? updatedRole.location = '' : updatedRole.location; |
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.
Please move the code to a separate function with a meaningful name, there is too much duplication.
@ajesamson are there JS test suites for JobRoles extension? |
@igorpavlov There are JS test suites. |
/** | ||
* Update submitted role if undefined or option type is disabled | ||
* | ||
* @param updatedRole |
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.
* @param updatedRole
* @param optionData
* @return {Object}
2347c37
to
d6a280b
Compare
d6a280b
to
ec9f5f4
Compare
Overview
This PR fixes issues with editing disabled or deleted job role option group. Dropdown displays wrongly if such record was previously submitted.
Before
After
Technical Details
Values that are invalid gets submitted as the existing implementation only checks if the value is defined or not. To guide against this, submitted data is checked against valid list of options data.
✅Manual Tests - passed
✅Jasmine Tests - passed
✅JS distributive files - included