New fetch logic #1627
New fetch logic #1627
Conversation
|
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. |
|
Per Trillium - Changes requested:
|
|
@Spiteless I opted for filtering on server because I didn't find a way to use multiple calls to Mongo. |
client/src/pages/ProjectList.js
Outdated
| @@ -70,7 +67,7 @@ export default function ProjectList() { | |||
| } | |||
|
|
|||
| // Render loading circle until project data is served from API | |||
| if (!projects) | |||
| if (!project && !projects) | |||
There was a problem hiding this comment.
This is the same test twice, was it meant to be something else?
There was a problem hiding this comment.
Nevermind, I see that its project singular, and projects, multiple. My bad
There was a problem hiding this comment.
It would help to change the names of these variables so they're more clear though
| @@ -99,7 +96,21 @@ export default function ProjectList() { | |||
| )} | |||
|
|
|||
| <TitledBox title="Active Projects" childrenBoxSx={{ p: 2 }}> | |||
| {projects.map((project) => ( | |||
There was a problem hiding this comment.
This looks like it can be refactorered so that we're not duplicating the component structure.
How do you feel about defining a projectName, projectTo, projectId and maybe projectsList properties in the function body based on the differences in then user?.accessLevel map so we don't need to redefine the component structure?
If you feel like this is more clear as written that's fine too
yarn.lock
Outdated
There was a problem hiding this comment.
Can you restore this file to an unchanged state
| @@ -7,6 +7,8 @@ const { AuthUtil } = require("../middleware"); | |||
| // The base is /api/projects | |||
| router.get('/', ProjectController.project_list); | |||
|
|
|||
| router.put('/', ProjectController.projects) | |||
There was a problem hiding this comment.
This feels like we don't want this to a PUT request because the client is only requesting information, not updating anything. If we were to turn this into a GET request what would need to change in order for it to work properly?
Fixes #1619
What changes did you make and why did you make them ?
This PR might open up a few further questions.
First, Can we guarantee that a User will only PM one project? I hardcoded a fetch based off only 1 ID in a users Managed Projects array. That would introduce a slippery slope of additional requests if there are more than one. if, 2, if 3, etc.
Screenshots of Proposed Changes Of The Website (if any, please do not screen shot code changes)
Visuals before changes are applied
Visuals after changes are applied