Feature/95/archive project #99
Conversation
martinrobled0
requested review from
jackbravo,
juanan0309,
LuisTejedaS and
JCoronaWize
There was a problem hiding this comment.
I have a question a a favor:
Favor: Can you please go to the app/routes/manager/filter-tags/test/labels.test.tsx and remove the request variable that is causing the ESLint issue? I forgot to remove it in my PR.
Question: Everything works fine for me, but, if I am an Admin, and archive a project I'm not a member of, I can't unarchive it as it doesn't appear in my "My proposals" list, is there a way to unarchive the projects in this case?
| IconButton, | ||
| Tooltip, | ||
| } from "@mui/material"; | ||
| import { blue } from "@mui/material/colors"; |
| type="submit" | ||
| className="primary warning" | ||
| > | ||
| Yes,archive it |
There was a problem hiding this comment.
Can we add a space after the comma? So it be Yes, archive it
| }); | ||
|
|
||
| if (!isAdmin) | ||
| validateIsTeamMember(profileId, projectMembers, currentProject.ownerId); |
There was a problem hiding this comment.
I'm thinking we should move these validation functions out of the *.server.ts files, and instead do them inside the action functions. That way the database layer will be cleaner, and authorization logic will live close to the view/controller layer.
What do you think?
There was a problem hiding this comment.
For an example of this, look at the app/routes/projects/$projectId/updateRelatedProjects.ts file.
There was a problem hiding this comment.
ok, makes sense for me. Do you want me to start it on this ticket? 👀
There was a problem hiding this comment.
Yes, but only for the validations you added. To move them to the app/routes/projects/archive.tsx action.
| )} | ||
|
|
||
| {(isTeamMember || isAdmin) && |
There was a problem hiding this comment.
Since you are doing the same validation as in the edit button, I think you don't need to create your own boolean block, but just add the IconButton to the top one.
Also, I think it would be better if the edit button is the one on the rigth.
| @@ -39,6 +39,7 @@ | |||
| "@remix-validated-form/with-zod": "^2.0.5", | |||
| "@uiw/react-md-editor": "^3.20.1", | |||
| "bcryptjs": "^2.4.3", | |||
| "ci": "^2.2.0", | |||
| const isAdmin = user.role == adminRoleName; | ||
|
|
||
| try { | ||
| await unarchiveProject(projectId, profile.id, isAdmin); |
There was a problem hiding this comment.
Since both archive and unarchive routes are pretty similar, I wonder if we should instead have a single /toggle-archive route. 🙈 . But this change is not required, is just a nitpick, if you like it as it is, you can ignore it.
| <Button | ||
| disabled={isButtonDisabled} | ||
| type="submit" | ||
| className="primary warning" |
jackbravo
force-pushed
the
main
branch
2 times, most recently
from
24fa5ac to
8cdd302
Compare
|
By @LuisTejedaS 's comment here (and I agree), seems like we should just change how we display this feature. Instead of an archive button, that seems somewhat complicated, it could be displayed just like the admin-only delete button, but for owners of the project who are not administrators. The functionality would be similar, we activate a boolean column, and hide it from most views. We would also have an admin-only view of soft-deleted projects so we can delete them completely. |
What does this PR do?
I added the option of archive and unarchived projects. Admins and project members could archive and unarchived projects.
The archive projects will be hidden in the active list of projects. But you will find it on your proposal list.
How should this be manually tested?
What are the relevant tickets?
#95
Screenshots
Checklist