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

feature: add delete scan function #208

Merged
merged 1 commit into from
May 17, 2023

Conversation

nicolearagao
Copy link
Member

@nicolearagao nicolearagao commented May 15, 2023

What's included

fix(deleteScanDialog): locale strings
feature(deleteScan): adds scan deletion functionality

How to test

Coverage and basic unit test check

  1. update the NPM packages with $ yarn
  2. $ yarn test
  3. confirm tests come back clean

Local run check

  1. update the NPM packages with $ yarn
  2. $ yarn start:stage
  3. add a
  • credential (any source)
  • source (same type of credential)
  • scan, confirm it
    • on scan window, a thrash icon is displayed according to style requirements to every added scan
    • when scaling the borwser down, it should populate the kebab menu
    • when selecting a scan to delete a confirmation message should be displayed
    • when deleting a scan, a confirmation message of the deletion should be displayed

Example

  • Add credential
    cred
  • Add source
    source
  • Add scan
    scan
  • Visualize the trash icon
    trash
  • Confirmation if the item should be deleted
    confirm-deletion
    -Confirm item was deleted
    deletion-confirmed

Updates issue/story

feature(deleteScan): discovery-76

Copy link
Member

@cdcabrera cdcabrera left a comment

Choose a reason for hiding this comment

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

Looks pretty good so far. Just a couple of minor mods need to happen to align with the storyboard.

Attached some references for you and of course, as you noted, all those unit tests need to be updated.

src/redux/reducers/scansReducer.js Outdated Show resolved Hide resolved
src/components/scans/scansTableCells.js Outdated Show resolved Hide resolved
src/components/scans/scansContext.js Outdated Show resolved Hide resolved
@nicolearagao nicolearagao force-pushed the feature/delete-scans branch 2 times, most recently from 1bfe552 to 27fce57 Compare May 15, 2023 20:29
@nicolearagao nicolearagao marked this pull request as ready for review May 15, 2023 21:02
@codecov-commenter
Copy link

codecov-commenter commented May 16, 2023

Codecov Report

Merging #208 (400bb82) into master (a2f4151) will decrease coverage by 0.16%.
The diff coverage is 66.66%.

❗ Current head 400bb82 differs from pull request most recent head ba29b7e. Consider uploading reports for the commit ba29b7e to get more accurate results

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #208      +/-   ##
==========================================
- Coverage   84.28%   84.12%   -0.16%     
==========================================
  Files         116      116              
  Lines        3716     3761      +45     
  Branches     1323     1342      +19     
==========================================
+ Hits         3132     3164      +32     
- Misses        521      532      +11     
- Partials       63       65       +2     
Impacted Files Coverage Δ
src/services/scansService.js 100.00% <ø> (ø)
src/components/scans/scansTableCells.js 71.15% <40.00%> (+0.44%) ⬆️
src/components/scans/scans.js 77.50% <50.00%> (-1.45%) ⬇️
src/components/scans/scansContext.js 65.54% <63.33%> (-0.57%) ⬇️
src/redux/actions/scansActions.js 100.00% <100.00%> (ø)
src/redux/constants/scansConstants.js 100.00% <100.00%> (ø)
src/redux/reducers/scansReducer.js 100.00% <100.00%> (ø)

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a2f4151...ba29b7e. Read the comment docs.

Copy link
Member

@cdcabrera cdcabrera left a comment

Choose a reason for hiding this comment

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

lgtm! I can achieve a match and functionality by using both $ yarn start and $ yarn start:stage

I do recommend we have a double check from testing around deleting a scan that has just started. If that has already been confirmed props! (iterative works)

And finally, just need to work on the commit messaging a little. When we merge we can talk that over @nicolearagao

*locale, delete scan strings
*scansService, fix deleteScan func
*scansActions, add deleteScan action
*scansConstants, delete scan constants
*scansReducer, delete scan state
*scansContext, delete scan hook
*scans, delete scans props
*scansTableCells, delete button
@nicolearagao nicolearagao merged commit 59b2e37 into quipucords:master May 17, 2023
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.

4 participants