Skip to content

Fix delete recipe modal #4350

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

Open
wants to merge 26 commits into
base: main
Choose a base branch
from
Open

Fix delete recipe modal #4350

wants to merge 26 commits into from

Conversation

madelondohmen
Copy link
Contributor

@madelondohmen madelondohmen commented Apr 17, 2025

Changes

If a user tried to delete a recipe, it would always delete the first row of the scheduled reports column.
Also, the delete modal in the second row didn't open.

It was caused by two things:

  • The delete-modal didn't have a specific id. This way, the same modal was opened for every row.
  • The form inside of the modal didn't have specific id. This way all the hidden input of all different delete-forms was submitted when deleting one item.

@stephanie0x00 found another bug while QA'ing:
After deleting a recipe, if a user tried to rerun this report, an error occured.
Fixed this by checking if the schedule exists. If not, an error message is shown to the user.

Demo

afbeelding

afbeelding

QA notes

Create multiple report rand the try to delete the recipes on the scheduled reports page. Make sure the correct recipes are deleted. Also check if the rerun works as expected and if the warning is clear enough.


Code Checklist

  • All the commits in this PR are properly PGP-signed and verified.
  • This PR only contains functionality relevant to the issue.
  • I have written unit tests for the changes or fixes I made.
  • I have checked the documentation and made changes where necessary.
  • I have performed a self-review of my code and refactored it to the best of my abilities.
  • Tickets have been created for newly discovered issues.
  • For any non-trivial functionality, I have added integration and/or end-to-end tests.
  • I have informed others of any required .env changes files if required and changed the .env-dist accordingly.
  • I have included comments in the code to elaborate on what is not self-evident from the code itself, including references to issues and discussions online, or implicit behavior of an interface.

Checklist for code reviewers:

Copy-paste the checklist from the docs/source/contributor/templates folder into your comment.


Checklist for QA:

Copy-paste the checklist from the docs/source/contributor/templates folder into your comment.

@madelondohmen madelondohmen added the bug Something isn't working label Apr 17, 2025
@madelondohmen madelondohmen requested a review from a team as a code owner April 17, 2025 15:08
@github-project-automation github-project-automation bot moved this to Incoming features / Need assessment in KAT Apr 17, 2025
@madelondohmen madelondohmen moved this from Incoming features / Need assessment to Review in KAT Apr 17, 2025
ammar92
ammar92 previously approved these changes Apr 18, 2025
@ammar92 ammar92 moved this from Review to QA review / functional testing in KAT Apr 18, 2025
@stephanie0x00
Copy link
Contributor

Checklist for QA:

  • I have checked out this branch, and successfully ran a fresh make reset.
  • I confirmed that there are no unintended functional regressions in this branch:
    • I have managed to pass the onboarding flow
    • Objects and Findings are created properly
    • Tasks are created and completed properly
  • I confirmed that the PR's advertised feature or hotfix works as intended.
  • I checked the logs for errors and/or warnings and made issues where necessary

What works:

Scheduling of reports still works.

What doesn't work:

  • Deleting a recipe still results in the incorrect recipe to be thrown away.
  • After deleting a recipe the interface stops responding in that tab. You have to switch to a different tab to interact with kat again.
  • Rerunning a report for which the recipe was deleted throws an error.

image

Bug or feature?:

image

@stephanie0x00 stephanie0x00 added the 😸 Review/QA feedback Review/QA feedback provided label Apr 22, 2025
@madelondohmen madelondohmen removed the 😸 Review/QA feedback Review/QA feedback provided label Apr 23, 2025
@stephanie0x00 stephanie0x00 added the 😸 Review/QA feedback Review/QA feedback provided label Apr 24, 2025
@madelondohmen madelondohmen removed the 😸 Review/QA feedback Review/QA feedback provided label Apr 24, 2025
@stephanie0x00
Copy link
Contributor

Seems to work now! Interface works after deleting an item. Rerunning a deleted report now gives a nice error to the user and the right recipe is deleted. 🎉

@stephanie0x00 stephanie0x00 moved this from QA review / functional testing to Ready for merge in KAT Apr 28, 2025
@madelondohmen madelondohmen moved this from Ready for merge to In Progress in KAT Apr 28, 2025
@madelondohmen madelondohmen moved this from In Progress to Review in KAT Apr 29, 2025
@madelondohmen madelondohmen removed their assignment May 1, 2025
@stephanie0x00 stephanie0x00 added 😸 Review/QA feedback Review/QA feedback provided and removed 😸 Review/QA feedback Review/QA feedback provided labels May 1, 2025
Copy link

sonarqubecloud bot commented May 1, 2025

Quality Gate Failed Quality Gate failed

Failed conditions
69.1% Coverage on New Code (required ≥ 80%)

See analysis details on SonarQube Cloud

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
Status: Review
Development

Successfully merging this pull request may close these issues.

4 participants