-
Notifications
You must be signed in to change notification settings - Fork 6
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
PCHR-4073: Fix SSP Filter on Staff Directory #533
PCHR-4073: Fix SSP Filter on Staff Directory #533
Conversation
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.
Some changes
$query->add_where($group, $roleEndField, date('Y-m-d'), '>='); | ||
$query->add_where($group, $roleEndField, NULL, 'IS'); | ||
// if department or title is set we must limit to only current roles | ||
if (!empty($queryParts[$departmentField]) || |
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.
Doesn't this fit in the 80 character limit on one line?
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.
Will check and update as appropriate 👓
if (!empty($queryParts[$departmentField]) || | ||
!empty($queryParts[$titleField]) | ||
) { | ||
$orGroup = $query->set_where_group('OR'); // add a new WHERE group |
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 this is $orGroup
because it's endDate is in future OR is null
, but you have $andGroup
that doesn't contain AND conditions. Do you think it would be clearer to rename these roleEndGroup
and roleStartGroup
or roleEndCondition
or something like that?
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.
The add_where
function requires a group which will determine what section the query will be added (AND/OR). It makes sense to rename as suggested 👌
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 🔥
094c189
to
ca7fdcd
Compare
f189aad
into
PCHR-3963-fix-ssp-filters-to-take-into-account-role-start-end-dates
Overview
Filtering staff records in SSP have some issue with job roles that are not active. This PR fixes the issue by applying additional filter condition to job role start date. Date filtering is now applied to filter requests containing department and job title.
Before
Staff with job roles starting in the future or past end dates show up when running filter.
After
Staff with job roles starting in the future or past end dates no longer show up in filter result.
Technical Details
In Staff Details View the query was altered to cater for
job title
androle start date
field in addition todepartment
androle end date
fields.Additional query grouping was added.