Skip to content
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.

Sign up for GitHub

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-4094: Add admin to leave request managees list #2811

Merged
merged 2 commits into from
Aug 2, 2018
Merged

PCHR-4094: Add admin to leave request managees list #2811

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
ab37348
PCHR-4094: Add admin to managees list
igorpavlov Aug 2, 2018
0659b91
PCHR-4094: PR Feedback
igorpavlov Aug 2, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion uk.co.compucorp.civicrm.hrleaveandabsences/js/dist/absence-tab.min.js
View file
Open in desktop

Large diffs are not rendered by default.

4 changes: 2 additions & 2 deletions uk.co.compucorp.civicrm.hrleaveandabsences/js/dist/admin-dashboard.min.js
View file
Open in desktop

Large diffs are not rendered by default.

4 changes: 2 additions & 2 deletions uk.co.compucorp.civicrm.hrleaveandabsences/js/dist/manager-leave.min.js
View file
Open in desktop

Large diffs are not rendered by default.

4 changes: 2 additions & 2 deletions uk.co.compucorp.civicrm.hrleaveandabsences/js/dist/my-leave.min.js
View file
Open in desktop

Large diffs are not rendered by default.

8 changes: 4 additions & 4 deletions ...civicrm.hrleaveandabsences/js/src/leave-absences/shared/controllers/request.controller.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -814,6 +814,9 @@ define([

/**
* Loads the managees of currently logged in user
* If a contact is pre-selected, then a single managee is loaded.
* If user is an admin, then all contacts, including the admin, are loaded.
* If user is a manager, then only contacts they manage are loaded.
*
* @return {Promise}
*/
Expand All @@ -828,10 +831,7 @@ define([
// In case of general administration
return Contact.all()
.then(function (contacts) {
vm.managedContacts = _.remove(contacts.list, function (contact) {
// Removes the admin from the list of contacts
return contact.id !== loggedInContact.id;
});
vm.managedContacts = contacts.list;
Copy link
Contributor

@deb1990 deb1990 Aug 2, 2018 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please update the doc for loadManagees, adding a note saying that the admin itself is also part of the managedContact. Normally by managees one does not think of himself.

Also we need to know why exactly the admin was removed from managees before. I already found that it was done as part of #1999, as a requirement. Could you please add this in the PR documentation.

});
} else {
// In any other case (including managing)
Expand Down
6 changes: 3 additions & 3 deletions ...ompucorp.civicrm.hrleaveandabsences/js/test/shared/controllers/request.controller.spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1132,7 +1132,7 @@

describe('admin opens leave request popup in create mode', function () {
var leaveRequest;
var adminId = '206';
var adminId = CRM.vars.leaveAndAbsences.contactId.toString();

beforeEach(function () {
leaveRequest = LeaveRequestInstance.init();
Expand All @@ -1151,8 +1151,8 @@
expect(controller.isRole('admin')).toBeTruthy();
});

it('does not contain admin in the list of managees', function () {
expect(_.find(controller.managedContacts, { 'id': adminId })).toBeUndefined();
it('contains admin in the list of managees', function () {
expect(!!_.find(controller.managedContacts, { 'id': adminId })).toBe(true);
});
});
});
Expand Down