Improvements to project deletion operation #614
Conversation
jaragunde
commented
Feb 17, 2023
jaragunde
commented
- Better feedback on error.
- Port to PDO.
- Delete user assignments when deleting a project.
- Remove unused code.
jaragunde
force-pushed
the
project-dao
branch
from
0cda372
to
1bf659d
Compare
| $sql = "DELETE FROM project WHERE id=".$projectVO->getId(); | ||
| $result->setIsSuccessful(true); | ||
| $result->setMessage('Project deleted successfully.'); | ||
| $result->setResponseCode(201); |
There was a problem hiding this comment.
Shouldn't it be 200 in this case as 201 is for created?
There was a problem hiding this comment.
Agree with Ana that this should probably return 200. We can use 201 in results where we are adding records, since a 'created' is more descriptive.
There was a problem hiding this comment.
Copy & paste error, thanks for pointing out 🙂
| } | ||
| $string .= "</errors></return>"; | ||
| } | ||
| } |
There was a problem hiding this comment.
I was wondering if we shouldn't have just the if (count($deleteProjects) > 1) block since we run a loop in the list of projects it should work with 1 or more.
There was a problem hiding this comment.
Can we even delete multiple projects at once from the UI? I remember we had a discussion about this for the creation of projects and landed on leaving in code that handles multiples for the time being. Still, Ana has a good point that this block could be simplified either way. I think that means that the code I wrote for creating projects could similarly be simplified.
There was a problem hiding this comment.
It's the same case here, the UI cannot even delete multiple projects but the service allows it. Following the same reasoning, I left that ability in place, just in case.
It's true it can be simplified, anyway, so I'll do it. I'll submit a follow-up for the create operation.
We add a DELETE CASCADE restriction to remove entries in the project_usr table at the same time we delete the corresponding project. Other restrictions to delete projects are still in place, for example, you cannot delete a project that has assigned tasks due to other foreign key constraints. This being a change in the database, we bump its version too, and write an upgrade script.
jaragunde
force-pushed
the
project-dao
branch
from
1bf659d
to
8499ad3
Compare
|
Thanks for your reviews! |
