-
Notifications
You must be signed in to change notification settings - Fork 18
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
feat: add endpoint to delete draft project #1752
Conversation
WalkthroughThis update significantly enhances project management by introducing a feature for deleting draft projects and their associated data. It includes comprehensive tests to ensure reliability and maintain data integrity, along with localized error messages for better user experience. The changes span both backend logic and corresponding GraphQL mutations, creating a cohesive and user-friendly interface throughout the application. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant ProjectResolver
participant Repository
User->>ProjectResolver: Request to delete draft project
ProjectResolver->>Repository: Call removeProjectAndRelatedEntities(projectId)
Repository-->>ProjectResolver: Confirm deletion success
ProjectResolver-->>User: Return success response
Recent review detailsConfiguration used: CodeRabbit UI Files selected for processing (2)
Files skipped from review as they are similar to previous changes (1)
Additional comments not posted (1)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
Outside diff range, codebase verification and nitpick comments (2)
src/repositories/projectRepository.test.ts (1)
523-606
: LGTM! Consider adding edge case tests.The test case correctly verifies the deletion of a project and its related entities. However, consider adding tests for edge cases such as attempting to delete a non-existent project or handling database errors.
function removeProjectAndRelatedEntitiesTestCase() { it('should remove project and related entities', async () => { const user = await saveUserDirectlyToDb(generateRandomEtheriumAddress()); const projectData = createProjectData(); projectData.adminUserId = user.id; //It creates a project, projectUpdate, and ProjectAddress const project = await saveProjectDirectlyToDb(projectData); await Promise.all([ saveDonationDirectlyToDb( { ...createDonationData(), }, user.id, project.id, ), saveAnchorContractDirectlyToDb({ creatorId: user.id, projectId: project.id, }), Reaction.create({ projectId: project.id, userId: user.id, reaction: '', }).save(), ProjectSocialMedia.create({ projectId: project.id, type: ProjectSocialMediaType.FACEBOOK, link: 'https://facebook.com', }).save(), ProjectStatusHistory.create({ projectId: project.id, createdAt: new Date(), }).save(), ProjectVerificationForm.create({ projectId: project.id }).save(), FeaturedUpdate.create({ projectId: project.id }).save(), SocialProfile.create({ projectId: project.id }).save(), ]); const relatedEntitiesBefore = await Promise.all([ Donation.findOne({ where: { projectId: project.id } }), Reaction.findOne({ where: { projectId: project.id } }), ProjectAddress.findOne({ where: { projectId: project.id } }), ProjectSocialMedia.findOne({ where: { projectId: project.id } }), AnchorContractAddress.findOne({ where: { projectId: project.id } }), ProjectStatusHistory.findOne({ where: { projectId: project.id } }), ProjectVerificationForm.findOne({ where: { projectId: project.id }, }), FeaturedUpdate.findOne({ where: { projectId: project.id } }), SocialProfile.findOne({ where: { projectId: project.id } }), ProjectUpdate.findOne({ where: { projectId: project.id } }), ]); relatedEntitiesBefore.forEach(entity => { assert.isNotNull(entity); }); await removeProjectAndRelatedEntities(project.id); const relatedEntities = await Promise.all([ Donation.findOne({ where: { projectId: project.id } }), Reaction.findOne({ where: { projectId: project.id } }), ProjectAddress.findOne({ where: { projectId: project.id } }), ProjectSocialMedia.findOne({ where: { projectId: project.id } }), AnchorContractAddress.findOne({ where: { projectId: project.id } }), ProjectStatusHistory.findOne({ where: { projectId: project.id } }), ProjectVerificationForm.findOne({ where: { projectId: project.id }, }), FeaturedUpdate.findOne({ where: { projectId: project.id } }), SocialProfile.findOne({ where: { projectId: project.id } }), ProjectUpdate.findOne({ where: { projectId: project.id } }), ]); const fetchedProject = await Project.findOne({ where: { id: project.id } }); assert.isNull(fetchedProject); relatedEntities.forEach(entity => { assert.isNull(entity); }); }); it('should handle non-existent project gracefully', async () => { const nonExistentProjectId = 999999; await removeProjectAndRelatedEntities(nonExistentProjectId); // Add assertions to ensure no errors are thrown and the function handles this case correctly }); it('should handle database errors gracefully', async () => { // Simulate a database error and ensure the function handles it correctly // Add assertions to verify error handling }); }src/resolvers/projectResolver.test.ts (1)
5675-5675
: Add a descriptive comment for the test suite.Consider adding a comment to describe the purpose of the
deleteDraftProjectTestCases
function.+ // Test cases for deleting draft projects function deleteDraftProjectTestCases() {
export const removeProjectAndRelatedEntities = async ( | ||
projectId: number, | ||
): Promise<void> => { | ||
// Delete related entities | ||
await Donation.createQueryBuilder() | ||
.delete() | ||
.where('projectId = :projectId', { projectId }) | ||
.execute(); | ||
|
||
await Reaction.createQueryBuilder() | ||
.delete() | ||
.where('projectId = :projectId', { projectId }) | ||
.execute(); | ||
|
||
await ProjectAddress.createQueryBuilder() | ||
.delete() | ||
.where('projectId = :projectId', { projectId }) | ||
.execute(); | ||
|
||
await ProjectSocialMedia.createQueryBuilder() | ||
.delete() | ||
.where('projectId = :projectId', { projectId }) | ||
.execute(); | ||
|
||
await AnchorContractAddress.createQueryBuilder() | ||
.delete() | ||
.where('projectId = :projectId', { projectId }) | ||
.execute(); | ||
|
||
await ProjectStatusHistory.createQueryBuilder() | ||
.delete() | ||
.where('projectId = :projectId', { projectId }) | ||
.execute(); | ||
|
||
await ProjectVerificationForm.createQueryBuilder() | ||
.delete() | ||
.where('projectId = :projectId', { projectId }) | ||
.execute(); | ||
|
||
await FeaturedUpdate.createQueryBuilder() | ||
.delete() | ||
.where('projectId = :projectId', { projectId }) | ||
.execute(); | ||
|
||
await SocialProfile.createQueryBuilder() | ||
.delete() | ||
.where('projectId = :projectId', { projectId }) | ||
.execute(); | ||
|
||
await ProjectUpdate.createQueryBuilder() | ||
.delete() | ||
.where('projectId = :projectId', { projectId }) | ||
.execute(); | ||
|
||
await Project.createQueryBuilder() | ||
.delete() | ||
.where('id = :id', { id: projectId }) | ||
.execute(); | ||
}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! But consider adding error handling.
The function correctly deletes the project and its related entities. However, it would be beneficial to add error handling to ensure that any issues during the deletion process are properly managed.
export const removeProjectAndRelatedEntities = async (
projectId: number,
): Promise<void> => {
try {
// Delete related entities
await Donation.createQueryBuilder()
.delete()
.where('projectId = :projectId', { projectId })
.execute();
await Reaction.createQueryBuilder()
.delete()
.where('projectId = :projectId', { projectId })
.execute();
await ProjectAddress.createQueryBuilder()
.delete()
.where('projectId = :projectId', { projectId })
.execute();
await ProjectSocialMedia.createQueryBuilder()
.delete()
.where('projectId = :projectId', { projectId })
.execute();
await AnchorContractAddress.createQueryBuilder()
.delete()
.where('projectId = :projectId', { projectId })
.execute();
await ProjectStatusHistory.createQueryBuilder()
.delete()
.where('projectId = :projectId', { projectId })
.execute();
await ProjectVerificationForm.createQueryBuilder()
.delete()
.where('projectId = :projectId', { projectId })
.execute();
await FeaturedUpdate.createQueryBuilder()
.delete()
.where('projectId = :projectId', { projectId })
.execute();
await SocialProfile.createQueryBuilder()
.delete()
.where('projectId = :projectId', { projectId })
.execute();
await ProjectUpdate.createQueryBuilder()
.delete()
.where('projectId = :projectId', { projectId })
.execute();
await Project.createQueryBuilder()
.delete()
.where('id = :id', { id: projectId })
.execute();
} catch (error) {
// Handle the error appropriately
console.error('Error deleting project and related entities:', error);
throw new Error('Failed to delete project and related entities');
}
};
Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
export const removeProjectAndRelatedEntities = async ( | |
projectId: number, | |
): Promise<void> => { | |
// Delete related entities | |
await Donation.createQueryBuilder() | |
.delete() | |
.where('projectId = :projectId', { projectId }) | |
.execute(); | |
await Reaction.createQueryBuilder() | |
.delete() | |
.where('projectId = :projectId', { projectId }) | |
.execute(); | |
await ProjectAddress.createQueryBuilder() | |
.delete() | |
.where('projectId = :projectId', { projectId }) | |
.execute(); | |
await ProjectSocialMedia.createQueryBuilder() | |
.delete() | |
.where('projectId = :projectId', { projectId }) | |
.execute(); | |
await AnchorContractAddress.createQueryBuilder() | |
.delete() | |
.where('projectId = :projectId', { projectId }) | |
.execute(); | |
await ProjectStatusHistory.createQueryBuilder() | |
.delete() | |
.where('projectId = :projectId', { projectId }) | |
.execute(); | |
await ProjectVerificationForm.createQueryBuilder() | |
.delete() | |
.where('projectId = :projectId', { projectId }) | |
.execute(); | |
await FeaturedUpdate.createQueryBuilder() | |
.delete() | |
.where('projectId = :projectId', { projectId }) | |
.execute(); | |
await SocialProfile.createQueryBuilder() | |
.delete() | |
.where('projectId = :projectId', { projectId }) | |
.execute(); | |
await ProjectUpdate.createQueryBuilder() | |
.delete() | |
.where('projectId = :projectId', { projectId }) | |
.execute(); | |
await Project.createQueryBuilder() | |
.delete() | |
.where('id = :id', { id: projectId }) | |
.execute(); | |
}; | |
export const removeProjectAndRelatedEntities = async ( | |
projectId: number, | |
): Promise<void> => { | |
try { | |
// Delete related entities | |
await Donation.createQueryBuilder() | |
.delete() | |
.where('projectId = :projectId', { projectId }) | |
.execute(); | |
await Reaction.createQueryBuilder() | |
.delete() | |
.where('projectId = :projectId', { projectId }) | |
.execute(); | |
await ProjectAddress.createQueryBuilder() | |
.delete() | |
.where('projectId = :projectId', { projectId }) | |
.execute(); | |
await ProjectSocialMedia.createQueryBuilder() | |
.delete() | |
.where('projectId = :projectId', { projectId }) | |
.execute(); | |
await AnchorContractAddress.createQueryBuilder() | |
.delete() | |
.where('projectId = :projectId', { projectId }) | |
.execute(); | |
await ProjectStatusHistory.createQueryBuilder() | |
.delete() | |
.where('projectId = :projectId', { projectId }) | |
.execute(); | |
await ProjectVerificationForm.createQueryBuilder() | |
.delete() | |
.where('projectId = :projectId', { projectId }) | |
.execute(); | |
await FeaturedUpdate.createQueryBuilder() | |
.delete() | |
.where('projectId = :projectId', { projectId }) | |
.execute(); | |
await SocialProfile.createQueryBuilder() | |
.delete() | |
.where('projectId = :projectId', { projectId }) | |
.execute(); | |
await ProjectUpdate.createQueryBuilder() | |
.delete() | |
.where('projectId = :projectId', { projectId }) | |
.execute(); | |
await Project.createQueryBuilder() | |
.delete() | |
.where('id = :id', { id: projectId }) | |
.execute(); | |
} catch (error) { | |
// Handle the error appropriately | |
console.error('Error deleting project and related entities:', error); | |
throw new Error('Failed to delete project and related entities'); | |
} | |
}; |
@Reshzera Can you fix the CI/CD test workflow, it's failing |
@mohammadranjbarz fixed |
@Meriem-B Can you review this? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @Reshzera great job, please resolve conflicts then merge it yourself, after you merged it, let FE guys know that you implemented it by commenting the endpoint and instruction to have call it in the issue
The frontend change related to delete draft donation hasn't been merged on staging , am I right? |
@maryjaf Yes just the backend is on production, the FE side has not been implemented yet |
Summary by CodeRabbit
New Features
Bug Fixes
Documentation