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

feat(risk-matrix): use plotly #2

Merged
merged 5 commits into from
Jun 11, 2024
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions requirements.txt
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,8 @@ zope.event==5.0
zope.interface==6.1
zope.schema==7.0.1
zope.sqlalchemy==3.1
numpy==1.26.3
plotly==5.18.0
psycopg2-binary==2.9.9
redis[hiredis]==5.0.3

Expand Down
3 changes: 2 additions & 1 deletion src/riskmatrix/models/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
from .asset import Asset
from .organization import Organization
from .risk import Risk
from .risk_assessment import RiskAssessment
from .risk_assessment import RiskAssessment, RiskMatrixAssessment
from .risk_catalog import RiskCatalog
from .risk_category import RiskCategory
from .user import User
Expand Down Expand Up @@ -60,5 +60,6 @@ def includeme(config: 'Configurator') -> None:
'RiskAssessment',
'RiskCatalog',
'RiskCategory',
'RiskMatrixAssessment',
'User'
)
6 changes: 5 additions & 1 deletion src/riskmatrix/models/risk_assessment.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
from riskmatrix.orm.meta import UUIDStrPK


from typing import Any
from typing import Any, ClassVar
from typing import TYPE_CHECKING
if TYPE_CHECKING:
from riskmatrix.types import ACL
Expand Down Expand Up @@ -146,3 +146,7 @@ def __acl__(self) -> list['ACL']:
return [
(Allow, f'org_{self.risk.organization_id}', ['view']),
]


class RiskMatrixAssessment(RiskAssessment):
nr: ClassVar[int]
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
nr: ClassVar[int]
nr: int

Based on how this is used, this shouldn't be a ClassVar.

1 change: 1 addition & 0 deletions src/riskmatrix/static/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -58,3 +58,4 @@ def css(
)
datatable_js = js('datatables_custom.js', depends=[datatable_bootstrap])
xhr_edit_js = js('xhr_edit.js', depends=[datatable_js])
plotly_js = js('plotly.min.js', depends=[datatable_js])
8 changes: 8 additions & 0 deletions src/riskmatrix/static/js/plotly.min.js

Large diffs are not rendered by default.

10 changes: 8 additions & 2 deletions src/riskmatrix/subscribers.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
from pyramid.events import NewRequest
from pyramid.events import NewResponse

import secrets
Copy link
Member

Choose a reason for hiding this comment

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

Try to keep imports sorted alphabetically and by type, if that's too tedious you can consider adding isort as precommit hook, so it does it for you. Although isort will not separate out typing imports into its own block at the bottom, like I usually do, I can live with those being mixed in with the other imports.


from typing import TYPE_CHECKING
if TYPE_CHECKING:
Expand All @@ -19,7 +19,7 @@ def default_csp_directives(request: 'IRequest') -> dict[str, str]:
"frame-ancestors": "'none'",
"img-src": "'self' data: blob:",
"object-src": "'self'",
"script-src": "'self' blob: resource:",
"script-src": f"'self' 'nonce-{request.csp_nonce}' blob: resource:",
"style-src": "'self' 'unsafe-inline'",
}

Expand Down Expand Up @@ -51,6 +51,12 @@ def sentry_context(event: NewRequest) -> None:
scope.user = {'id': request.user.id}


def request_none_generator(event: 'NewRequest') -> None:
request = event.request
request.set_property(lambda r: secrets.token_urlsafe(), 'csp_nonce', reify=True)
Copy link
Member

Choose a reason for hiding this comment

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

Make sure to also add a csp_nonce attribute to testing.DummyRequest, this can just be a static string for testing. Some tests currently fail because the attribute doesn't exist.



def includeme(config: 'Configurator') -> None:
config.add_subscriber(csp_header, NewResponse)
config.add_subscriber(request_none_generator, NewRequest)
config.add_subscriber(sentry_context, NewRequest)
238 changes: 133 additions & 105 deletions src/riskmatrix/views/risk_assessment.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,11 @@
from wtforms import StringField
from wtforms import TextAreaField
from wtforms.widgets import html_params
import plotly.graph_objects as go
import numpy as np

from riskmatrix.controls import Button
from riskmatrix.models import RiskAssessment
from riskmatrix.models import RiskAssessment, RiskMatrixAssessment
from riskmatrix.data_table import AJAXDataTable
from riskmatrix.data_table import DataColumn
from riskmatrix.data_table import maybe_escape
Expand All @@ -22,7 +24,7 @@
if TYPE_CHECKING:
from pyramid.interfaces import IRequest
from sqlalchemy.orm.query import Query
from typing import TypeVar
from typing import TypeVar, Iterator
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
from typing import TypeVar, Iterator
from collections.abc import Iterator
from typing import TypeVar

Always use the collections.abc imports for these protocols, the typing aliases have been deprecated since 3.9. You could use pyupgrade to auto-correct imports. Although if you're also going to include isort it might be easier to just switch most of the linting over to ruff and only keep flake8 for some of the plugins that aren't included in ruff or not as up-to-date like flake8-type-checking.

ruff also includes auto fixes for a lot of the errors, if you're into that sort of thing.


from riskmatrix.models import Organization
from riskmatrix.types import MixedDataOrRedirect
Expand Down Expand Up @@ -154,14 +156,37 @@ def buttons(
return assessment_buttons(assessment, self.request)


_RADIO_TEMPLATE = Markup("""
class AssessmentOverviewTable(AssessmentBaseTable):
nr = DataColumn(_('Nr.'))
name = DataColumn(_('Name'))
description = DataColumn(_('Description'), class_name='visually-hidden')
category = DataColumn(_('Category'))
asset_name = DataColumn(_('Asset'))
likelihood = DataColumn(_('Likelihood'))
impact = DataColumn(_('Impact'))

def __init__(self, org: 'Organization', request: 'IRequest') -> None:
super().__init__(org, request, id='risks-table')
xhr_edit_js.need()

def query(self) -> 'Iterator[RiskMatrixAssessment]':
query = super().query()

for idx, item in enumerate(query, start=1):
item.nr = idx
yield item


_RADIO_TEMPLATE = Markup(
"""
<div class="form-check form-check-inline">
<input class="form-check-input" type="radio"
name="{name}" id="{name}-{value}" value="{value}"
data-url="{url}" data-csrf_token="{csrf_token}" {checked}/>
<label class="form-check-label" for="{name}-{value}">{value}</label>
</div>
""")
"""
)


class AssessImpactTable(AssessmentBaseTable):
Expand Down Expand Up @@ -296,112 +321,115 @@ def __html__(self) -> str:
)


def generate_risk_matrix_view(
context: 'Organization',
request: 'IRequest'
) -> 'RenderData':
def plot_risk_matrix(risks: 'Query[RiskMatrixAssessment]') -> Markup:
fig = go.Figure()

cells = [
[
Cell(value='Impact', rowspan=6, css_class='rotate', header=True),
Cell(value='5', title='Catastrophic', header=True),
Cell(css_class='medium'),
Cell(css_class='medium'),
Cell(css_class='high'),
Cell(css_class='high'),
Cell(css_class='high'),
],
[
Cell(value='4', title='Major', header=True),
Cell(css_class='low'),
Cell(css_class='medium'),
Cell(css_class='medium'),
Cell(css_class='high'),
Cell(css_class='high'),
],
[
Cell(value='3', title='Moderate', header=True),
Cell(css_class='low'),
Cell(css_class='medium'),
Cell(css_class='medium'),
Cell(css_class='medium'),
Cell(css_class='high'),
],
[
Cell(value='2', title='Minor', header=True),
Cell(css_class='low'),
Cell(css_class='low'),
Cell(css_class='medium'),
Cell(css_class='medium'),
Cell(css_class='medium'),
],
[
Cell(value='1', title='Insignificant', header=True),
Cell(css_class='low'),
Cell(css_class='low'),
Cell(css_class='low'),
Cell(css_class='low'),
Cell(css_class='low'),
],
[
Cell(header=True),
Cell(value='1', title='Rare', header=True),
Cell(value='2', title='Unlikely', header=True),
Cell(value='3', title='Possible', header=True),
Cell(value='4', title='Likely', header=True),
Cell(value='5', title='Almost Certain', header=True),
],
[

Cell(value='Likelihood', header=True, colspan=7),
]
]

session = request.dbsession
query = session.query(RiskAssessment)
query = query.filter(RiskAssessment.organization_id == context.id)
query = query.join(RiskAssessment.asset)
query = query.join(RiskAssessment.risk)

assessments = []
index = 0
for assessment in query:
impact = assessment.impact
likelihood = assessment.likelihood
if impact and likelihood:
index += 1
assessments.append({
'nr': index + 1,
'name': assessment.risk.name,
'asset': assessment.asset.name,
'impact': impact,
'likelihood': likelihood,
})

severity = 'success'
if impact * likelihood >= 5:
severity = 'warning'
if impact * likelihood >= 15:
severity = 'danger'

row = 5 - impact
col = likelihood
if row == 0:
col += 1
cells[row][col].value += Markup(
' <span class="badge rounded-pill bg-{severity} {css_class}"'
' title="{title}">{nr}</span>'
).format(
severity=severity,
nr=index,
title=f'{assessment.asset.name}: {assessment.risk.name}',
css_class='text-dark' if severity == 'warning' else ''
# Define the colors for different risk levels
colors = {
'green': [10, 15, 16, 20, 21],
'yellow': [0, 5, 6, 11, 17, 22, 23],
'orange': [1, 2, 7, 12, 13, 18, 19, 24],
'red': [3, 4, 8, 9, 14],
}

# Create a 5x5 grid and set the color for each cell
for color, indices in colors.items():
for index in indices:
i, j = divmod(index, 5)

fig.add_shape(
type='rect',
x0=j,
y0=4 - i,
x1=j + 1,
y1=5 - i,
line={'color': 'white', 'width': 0},
fillcolor=color,
layer='below',
)

# Plot points
for risk in list(risks):
if risk.likelihood and risk.impact:
x, y = float(risk.likelihood - 1), float(4 - (risk.impact - 1))

# Adjust the position within the cell to avoid overlap
noise_x = np.random.uniform(0.1, 0.9)
noise_y = np.random.uniform(0.1, 0.9)
x, y = x + noise_x, y + noise_y

fig.add_trace(
go.Scatter(
x=[x],
y=[y],
text=[str(risk.nr)],
name='',
mode='markers+text',
marker={'color': 'black', 'size': 18},
textposition='middle center',
hoverinfo='text',
hovertemplate=f'{risk.nr} {risk.name} \
(Impact: {risk.impact} Likelihood: {risk.likelihood})',
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
hovertemplate=f'{risk.nr} {risk.name} \
(Impact: {risk.impact} Likelihood: {risk.likelihood})',
hovertemplate=f'{risk.nr} {risk.name} '
'(Impact: {risk.impact} Likelihood: {risk.likelihood})',

Please avoid using broken lines, especially inside string literals. Python has implicit string concatenation. There's flake8-broken-lines in case this is something copilot tries to make you do, rather than a conscious decision.

textfont={'color': 'white'},
)
)

fig.update_xaxes(fixedrange=True)
fig.update_yaxes(fixedrange=True)

# Update layout
fig.update_layout(
xaxis={
'showgrid': False,
'zeroline': False,
'showticklabels': False,
'range': [-0.5, 5]
},
yaxis={
'showgrid': False,
'zeroline': False,
'showticklabels': False,
'range': [-0.5, 5]
},
showlegend=False,
width=700,
height=700,
margin={'l': 5, 'r': 5, 't': 5, 'b': 5}, # Adjusted margins
paper_bgcolor='white',
plot_bgcolor='white',
)

# Add axis labels
fig.add_annotation(
x=2.5, y=-0.2, text='Impact', showarrow=False, font={'size': 20}
)
fig.add_annotation(
x=-0.2,
y=2.5,
text='Likelihood',
showarrow=False,
textangle=-90,
font={'size': 20},
)

return Markup(fig.to_html(
full_html=False,
include_plotlyjs=True,
config={
'modeBarButtonsToRemove': ['zoom', 'pan', 'select', 'lasso2d']
},
))


def generate_risk_matrix_view(
context: 'Organization', request: 'IRequest'
) -> 'RenderData':
table = AssessmentOverviewTable(context, request)

return {
'title': _('Risk Matrix'),
'cells': cells,
'assessments': assessments,
'plot': plot_risk_matrix(table.query()).replace('<script', f'<script nonce="{request.csp_nonce}"'),
'table': table,
}


Expand Down
20 changes: 9 additions & 11 deletions src/riskmatrix/views/templates/matrix.pt
Original file line number Diff line number Diff line change
Expand Up @@ -5,21 +5,19 @@
xmlns:i18n="http://xml.zope.org/namespaces/i18n"
i18n:domain="riskmatrix">


<tal:block metal:fill-slot="content">

<h1>${title}</h1>

<table width="100%" class="matrix table table-bordered">
<tr tal:repeat="row cells">
<tal:block tal:repeat="cell row">
${cell}
</tal:block>
</tr>
</table>

<ol class="risk-list">
<li tal:repeat="item assessments">${item['asset']}: ${item['name']}</li>
</ol>

${plot}


<div style="display: block; padding: 24px"></div>

${table}


</tal:block>

Expand Down
Loading