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.

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

Conversation

igorpavlov-zz
Copy link
Contributor

@igorpavlov-zz igorpavlov-zz commented Aug 2, 2018

Overview

As per #1999 the admin was excluded from the list of managed contacts, admin was not able to create approved requests for themselves.

Now, admins are always treated as self leave approvers. This PR allows admins to select themselves from the managees list to create a leave request for themselves.

Before

Admin admin@example.com does not appear in the list:

image

After

Admin admin@example.com appears in the list:

image

Technical Details

Simply remove the existing code that checks if the user is admin and removes them from the list:

function loadManagees () {
  if (vm.selectedContactId) {
    // ...
  } else if (vm.isRole('admin')) {
    return Contact.all()
      .then(function (contacts) {
        // vm.managedContacts = _.remove(contacts.list, function (contact) {
        //   return contact.id !== loggedInContact.id;
        vm.managedContacts = contacts.list; // no check needed anymore
  } else {
    // ...

✅Manual Tests - passed
✅Jasmine Tests - amended and passed
✅JS distributive files - included
⏹CSS distributive files - not needed
⏹Backstop tests - not needed
⏹PHP Unit Tests - not needed

// 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

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.

@igorpavlov-zz igorpavlov-zz merged commit 1844505 into PCHR-3997-allow-own-leave-approver Aug 2, 2018
@igorpavlov-zz igorpavlov-zz deleted the PCHR-4094-add-admin-in-the-managees-list branch August 2, 2018 10:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants