eChecklist Notes repetition

[JeffOdegard]

eChecklist notes should have code to squash repeated content. For example, putting "This is a note" in the eChecklist notes column, importing the eChecklist and then re-exporting - you get one additional instance of the same phrase for each host. (The title of this bug was from two iterations)

The same goes for pulling in SCC data. We should never see (SCC) (SCC) (SCC)... - just (SCC).

This could be solved with a simple regex whenever you are adding to the notes - just check to see if the new content is found in the old content. If so, just leave the note alone.

[godsgood33]

This is insanely difficult. I can add a regex that will remove duplicated words, but that would not work for notes like...

This is a note This is another note

I believe the resulting statement would be...

This is a note another

eChecklist notes overwrite any existing notes in the finding (line 278 & 317 OR 323). So cleaning up the notes (after fixing the error in #9) before importing the eChecklist, is the best that we can do short-term.

I would just say we can remove the "(SCC)" text so that it doesn't go in the notes. We need to figure out a different way of doing notes entirely. I think we should add a scan_notes and analyst_notes (maybe more) tables that clean this up.

[JeffOdegard]

The (SCC) note (1 of them) is actually helpful.

By rights, the notes should be per host, but the eChecklist doesn't lend itself to that.

You can't turn the string into a regex and match the whole string? "This[ ]is[ ]a[ ]note." ? I suppose it's tricky because notes can be multiline with \n's.

On Tue, Sep 11, 2018 at 2:50 PM Ryan P notifications@github.com wrote:

This is insanely difficult. I can add a regex that will remove duplicated words, but that would not work for notes like...

This is a note This is another note

I believe the resulting statement would be...

This is a note another

eChecklist notes overwrite any existing notes in the finding (line 278 & 317 OR 323). So cleaning up the notes (after fixing the error in #9 https://github.com/cyberperspectives/sagacity/issues/9) before importing the eChecklist, is the best that we can do short-term.

I would just say we can remove the "(SCC)" text so that it doesn't go in the notes. We need to figure out a different way of doing notes entirely. I think we should add a scan_notes and analyst_notes (maybe more) tables that clean this up.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/cyberperspectives/sagacity/issues/10#issuecomment-420420755, or mute the thread https://github.com/notifications/unsubscribe-auth/Aoyyh8W7fEPFFajkisWpeOBZVHqHePQKks5uaCIFgaJpZM4Wjqt3 .

--

• Jeff

---

Jeff Odegard jeff.odegard@gmail.com

[image: LinkedIn] https://www.linkedin.com/in/jeffodegard/ [image: Fakebook] https://www.facebook.com/jeff.odegard.98 [image: YouTube] https://www.youtube.com/user/OdegardOnline

[godsgood33]

If we split the analyst and scan notes we make the scan notes a comment to the target status (https://phpspreadsheet.readthedocs.io/en/develop/topics/recipes/#add-a-comment-to-a-cell), then the analyst notes would be in the notes column. To do this though it is going to require a rewrite of parse_excel_echecklist, and /ste/export.php. Not something I want to tackle now.

[JeffOdegard]

I agree that notes requires a redesign. I was just hoping for something simple in the short term.

On Tue, Sep 11, 2018 at 3:31 PM Ryan P notifications@github.com wrote:

If we split the analyst and scan notes we make the scan notes a comment to the target status ( https://phpspreadsheet.readthedocs.io/en/develop/topics/recipes/#add-a-comment-to-a-cell), then the analyst notes would be in the notes column. To do this though it is going to require a rewrite of parse_excel_echecklist, and /ste/export.php. Not something I want to tackle now.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/cyberperspectives/sagacity/issues/10#issuecomment-420432706, or mute the thread https://github.com/notifications/unsubscribe-auth/Aoyyh5IGgplypsXHXeleGvxstpapho_zks5uaCvDgaJpZM4Wjqt3 .

--

• Jeff

---

Jeff Odegard jeff.odegard@gmail.com

[image: LinkedIn] https://www.linkedin.com/in/jeffodegard/ [image: Fakebook] https://www.facebook.com/jeff.odegard.98 [image: YouTube] https://www.youtube.com/user/OdegardOnline

[godsgood33]

I added the following:

if(preg_match("/" . preg_quote($note, "/") . "/", $finding->get_Notes())) {

`$finding->set_Notes($note);`

} else {

`$finding->prepend_Notes($note);`

}

So it will check for the presence of a string that already exists in it's entirety in the existing notes. If it finds the entire message then it resets the entire message to that, otherwise it prepends it to the notes.

Examples: Existing note: (SCC) New note: (SCC) Final note: (SCC)

Existing note: This is an SCC note New note: (SCC) Final note: (SCC) This is an SCC note

Existing note: What to do with (SCC) finding New note: (SCC) Final note: (SCC)

I'm doubting this is the preferred behavior. I could add a check for string length (e.g. if existing note is longer than new note, then don't do anything, etc), or just don't change the note if the entirety of the new note is present in the existing note.

[godsgood33]

I implemented a fix for this, so you should not get the duplicating notes any more.

[godsgood33]

Added a add_cell_comment method to export.php today. This will simply take a worksheet, cell coordinates (A1), and note string and add it as a comment to at the cell coordinates. I had already added a scan_notes and analyst_notes tables to store these values. The analysts notes will be put in the cell, and the scanner notes will be put in the "comment" at that cell. It's going to take a bit more work to pull all of it together, but I've laid the groundwork for now.

[godsgood33]

Have a little bit of this in place, still need to work in the scan_notes and analyst_notes tables.

[JeffOdegard]

I saw some of this in our test results. Let's leave open and move to 1.4.0 We need to work on the underlying design and figure out how to do this the right way.