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

1771 - API for Fetching Project Managers #1799

Open
wants to merge 2 commits into
base: development
Choose a base branch
from

Conversation

ntrehan
Copy link
Member

@ntrehan ntrehan commented Oct 21, 2024

Fixes #1771

What changes did you make and why did you make them ?

  • Added an API that fetches all the project Managers and the projects they manage

Screenshots of Proposed Changes Of The Website

The API request and response ![image](https://github.com/user-attachments/assets/94a1f998-7e98-4506-a537-6125f23eacc7)

Copy link

Want to review this pull request? Take a look at this documentation for a step by step guide!

From your project repository, check out a new branch and test the changes.

git checkout -b ntrehan-1771 development
git pull https://github.com/ntrehan/VRMS.git 1771

@JackHaeg JackHaeg added ready for dev lead Add to issue when dev lead needs to take some action and removed ready for dev lead Add to issue when dev lead needs to take some action labels Oct 22, 2024
@JackHaeg JackHaeg requested a review from trillium October 23, 2024 21:38
@pluto-bell
Copy link
Member

pluto-bell commented Oct 28, 2024

@ntrehan just a heads up that your API req & res screenshot is currently broken / doesn't populate.

@ntrehan ntrehan changed the title 1771 1771 - API for Fetching Project Managers Oct 28, 2024
@ntrehan
Copy link
Member Author

ntrehan commented Oct 28, 2024

@ntrehan just a heads up that your API req & res screenshot is currently broken / doesn't populate.

Hi @pluto-bell Thanks for pointing that out!
However no matter how many times I reupload it there
I am not able to get it fixed
image
So I'll just add it here

@JackHaeg JackHaeg added the time-sensitive should be solved as soon as possible label Oct 29, 2024
if (!userProjectMap[user]) {
userProjectMap[user] = [];
}
userProjectMap[user].push(project.name);
Copy link
Member

@jng34 jng34 Oct 31, 2024

Choose a reason for hiding this comment

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

Ref: #1771
@ntrehan It seems like you might have missed the capture of the project.id. Perhaps pushing an project object with id and name properties would be great. Other than that, the code block looks good!

Copy link
Member

Choose a reason for hiding this comment

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

This is a good catch

@vorleakyek
Copy link
Member

vorleakyek commented Nov 4, 2024

If curl.exe --location 'http://localhost:3000/api/projects/projectManagers' returns "Unauthorized",
try
curl.exe --location 'http://localhost:3000/api/projects/projectManagers' --header 'x-customrequired-header: custom-header-here'
Note: replace custom-header-here with your own header.

Note: it returns the test data because we're running it locally. It will the actual data for the production environment.

{ : [ "VRMS" ], lead one: [ "New Test Project A+" ], as: [ "Fancy New Project" ], qegq: [ "! ! ! ! ! ! ! ! ! 🚨! ! ! DOGEfdsafdasfdsa" ], test: [ "! ! ! ! !TESTING123fdsaf" ], user: [ "! ! ! ! ! ! ! Project !!!! - testing 🐢fdsaffds" ], a: [ "TESTING Editing", "Projj" ], none: [ "meep" ] }

Copy link
Member

@trillium trillium left a comment

Choose a reason for hiding this comment

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

Requesting clarification and changes on API

  • will user object be populated for each entry?
  • Can we switch to an array instead of an object so that the frontend doesnt need to duplicate data for users with more than one managed project?

Thanks for putting so much work into getting this over then hill!

@@ -67,4 +67,26 @@ ProjectController.destroy = async function (req, res) {
};


ProjectController.getProjectManagers = async function (req, res) {
try {
const userProjectMap = {};
Copy link
Member

Choose a reason for hiding this comment

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

I think we want an array of objects of shape:

const userProjectMap = []; //changed to array
// Object shape from API call
{
    user, // will this just be the userId string or will
          // we have a populated user object with email, etc?
    project: {
        name: string
        projectId: string
    }
}

Does that look right to you? Then we'd be returning an array that can be sorted more easily on the front end with some sort functions

userProjectMap[user].push(project.name);
}
}
return res.status(200).send(userProjectMap);
Copy link
Member

Choose a reason for hiding this comment

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

Am I right in understanding that the final out of the array (or object if thats the structure we go with) will only have user IDs? Do we need to make a second db call to populate the user object?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
time-sensitive should be solved as soon as possible
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Create API call to get all users who are listed as managers on a project
6 participants