Skip to content
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

Update a Boefje #3521

Merged
merged 120 commits into from
Sep 26, 2024
Merged

Update a Boefje #3521

merged 120 commits into from
Sep 26, 2024

Conversation

madelondohmen
Copy link
Contributor

@madelondohmen madelondohmen commented Sep 12, 2024

NOTE: This PR is a follow-up of PR #3456


Changes

This PR makes it possible to update a Boefje variant.

Features added

Boefje detail page:

  • "Edit variant" button on Boefje detail page (in variants table)
  • Variants table: add "Boefje ID"
  • Variants table: add age column
  • Variants table: fix created date (make it readable)

New Boefje form page:

  • Change submit/discard button names to "Create new Boefje" and "Discard new Boefje"

Edit Boefje form page:

  • "Edit Boefje" form page that is pre-filled
  • Update helpertext for "Container image" input field
  • Return to updated Boefje page

Boefje variant form page:

  • Fix breadcrumbs
  • Remove read-only from "Container image" input field

Code:

  • In View: improve code and reduce duplicate code

Issue link

Closes #3468

Demo

afbeelding
afbeelding

QA notes

First create a variant of Nmap TCP:

  • Go to "Nmap TCP" boefje
  • Check the "Variants" table. There should be one item (the current Boefje)
  • Click "Add variant"
  • Type a different name, make some small changes to the other input fields
  • Click "Create variant"
  • You should return to the "Nmap TCP" boefje

Then, edit the variant:

  • Check the "Variants" table. Unfold your just created variant.
  • Click "Edit variant"
  • Check if all fields in de form are pre-filled
  • Type a different name, make some small changes to the other input fields
  • Click "Save changes"
  • You should return to the variant boefje detail page
  • Check if the changes were saved

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/templates folder into your comment.


Checklist for QA:

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

@madelondohmen madelondohmen mentioned this pull request Sep 23, 2024
9 tasks
@madelondohmen madelondohmen mentioned this pull request Sep 25, 2024
9 tasks
@TwistMeister
Copy link
Contributor

TwistMeister commented Sep 26, 2024

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:

  • Adding a variant
  • Editing a variant

What doesn't work:

Bug or feature?:

  • There's no validation on the input values (like name can be one character, an integer or any character really). I don't think this is a major risk, but for improved UX it would be nice to have a bit more checks in place. Not a show stopper for now, but maybe a new ticket to address this would be appropriate.

@underdarknl underdarknl merged commit f7b899e into main Sep 26, 2024
15 checks passed
@underdarknl underdarknl deleted the feature/update-boefje branch September 26, 2024 11:47
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.

Create Boefjes - Edit Boefje
5 participants