Skip to content

feat (challenge) deleteChallenge function added to challenge service#781

Open
JungleGiu wants to merge 4 commits intodevelopfrom
feature/241-delete-challenge-service
Open

feat (challenge) deleteChallenge function added to challenge service#781
JungleGiu wants to merge 4 commits intodevelopfrom
feature/241-delete-challenge-service

Conversation

@JungleGiu
Copy link
Collaborator

@JungleGiu JungleGiu commented Mar 11, 2026

📍Context

Related subissue

In this task As a mentor, I can to delete a challenge
we are aiming to create a new functionality for the authenticated mentor user, who could be capable of delete a challenge from the DB.
After considering the possible consequences in the way data is related the team decided to proceed and the function appointing to the existing endpoint DELETE /itachallenge/api/v1/challenge/challenges/{challengeId} was created in the challenge service

✅ What's done

  • deleteChallenge() function was created in challenge.service.ts with appropriate error handling
  • test coverage is been added for he new function

🔍 Proof

Screenshot 2026-03-11 alle 09 38 42

Notes

The function is still not being called since this would be out of scope for this subissue, but the integration in the challengeForm component is next. to come in IT-Academy-BCN/ita-challenges#252

Copy link
Collaborator

@polserrano8 polserrano8 left a comment

Choose a reason for hiding this comment

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

No he visto nada que me rechine en el código y la IA tampoco ha dado cosas a corregir que sean en este scope. Buen trabajo :)

const url = `${environment.BACKEND_ITA_CHALLENGE_BASE_URL}${environment.BACKEND_ALL_CHALLENGES_URL}/${challengeId}`
const headers = {
'Content-Type': 'application/json',
...this.authService.getAuthHeaders()
Copy link
Collaborator

Choose a reason for hiding this comment

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

The DELETE endpoint for challenges in back doesn't validate the user's role. An issue needs to be created to secure it. Have you discussed this with the back-end developers?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Hello Ivan, we have talked about that. Our approach is that the user won't be able to use this endpoint if it is not a mentor. That's becaouse the form where the button for action will be is already in the same page as the form editing. So technically the approach is the same as now with thye editing permisions.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I understand the reasoning, but UI-based security isn't enough. Any user could directly call the DELETE endpoint without going through the form, for example, from Postman or curl. Role validation needs to be on the back end, regardless of what the front end does. Since the back end hasn't identified this need, it would be good to bring it to their attention so they can create a technical debt.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes we discussed the issue and since the ui of editing it's only accessible from a admin role, we talked about maintaining the same functionality. However I understand a more solid approach is recommended, I suggest to put a limitation in the next task , where we can verify the user role before triggering this function, that should be as abstract as possible in case of future backend role limitation implementations.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Totally agree, i'll bring this issue to the attention of backend team

Copy link
Collaborator

@otrocadev otrocadev Mar 12, 2026

Choose a reason for hiding this comment

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

Should we also approach how this is handled with the edit process? I think the permissions and implications should be the same as the user role that shold be able to perform the action is the same.

We can see if this is already considered on the edit action of the same form so we can either repply the approach with the delete permission or to either make also a tech debt on this case scenario as well.

const url = `${environment.BACKEND_ITA_CHALLENGE_BASE_URL}${environment.BACKEND_ALL_CHALLENGES_URL}/${challengeId}`
const headers = {
'Content-Type': 'application/json',
...this.authService.getAuthHeaders()
Copy link
Collaborator

Choose a reason for hiding this comment

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

I understand the reasoning, but UI-based security isn't enough. Any user could directly call the DELETE endpoint without going through the form, for example, from Postman or curl. Role validation needs to be on the back end, regardless of what the front end does. Since the back end hasn't identified this need, it would be good to bring it to their attention so they can create a technical debt.

@sonarqubecloud
Copy link

@carlosPc1987
Copy link

I’ve added the tech debt in the link after reviewing the conversation.

Technical debt created
Backend role validation for the DELETE challenge endpoint is now tracked as technical debt:

IT-Academy-BCN/ita-challenges#263

Once the version and CHANGELOG updates are done, this PR is good to merge from my side.

Great work, everyone

@JungleGiu JungleGiu requested a review from ivilarop March 12, 2026 11:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[FE] Integrate service delete functionality with UI and modals

5 participants