Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
6f00bb4
WIP: choices
reinhardt Apr 4, 2025
1bc4165
Fix saving and navigation of choices/options in client
reinhardt Apr 7, 2025
9be064d
Fix is_custom_risk
reinhardt Apr 7, 2025
3f76540
A choice is answered if there are any selected options
reinhardt Apr 7, 2025
656fe7e
Choice: add field allow_multiple_options
reinhardt Apr 7, 2025
5a8c05a
getSurveyTree: Include euphorie.choice
reinhardt Apr 7, 2025
2b08ec3
Implement conditions in admin backend
reinhardt Apr 9, 2025
6c0174b
Choice: Fix class inheritance
reinhardt Apr 10, 2025
d1f0a56
Export/import of choices and options
reinhardt Apr 10, 2025
9713aab
Black/flake8/zpretty
reinhardt Apr 28, 2025
79f970d
Fix test
reinhardt Apr 28, 2025
199f46b
Fix navigation
reinhardt Apr 30, 2025
59f56fe
Fix updating session with choices
reinhardt Apr 30, 2025
1350476
isort
reinhardt Apr 30, 2025
683c71b
Fix import and update of allow_multiple_options
reinhardt May 5, 2025
64e083e
Copy options when session is updated
reinhardt May 5, 2025
c39053b
Fix tests
reinhardt May 5, 2025
f1c3850
Add tool type `inventory` and allow Choice only in this type
reinhardt May 6, 2025
b44bb4f
Guard against missing `allow_choice` key
reinhardt May 7, 2025
fc78f17
Do some checks and housekeeping when choice identification view is ca…
reinhardt May 7, 2025
f7e472c
Implement some PR review suggestions
reinhardt May 8, 2025
9e49d38
Choice conditions: Limit to current tool (survey)
reinhardt May 8, 2025
781195c
Don't iterate over the list that you're modifying
reinhardt May 8, 2025
44abaad
Use `super().__call__()` instead of `self.index()`
reinhardt May 12, 2025
567f48f
Simplify
reinhardt May 12, 2025
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
107 changes: 107 additions & 0 deletions src/euphorie/client/browser/choice.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,107 @@
from euphorie.client import utils
from euphorie.client.navigation import getTreeData
from plone import api
from plone.memoize.instance import memoize
from Products.Five import BrowserView


class IdentificationView(BrowserView):
"""A view for displaying a choice in the identification phase."""

variation_class = "variation-risk-assessment"

@property
@memoize
def webhelpers(self):
return api.content.get_view("webhelpers", self.context.aq_parent, self.request)

def check_render_condition(self):
# Render the page only if the user can inspection rights,
# otherwise redirect to the start page of the session.
if not self.webhelpers.can_inspect_session:
return self.request.response.redirect(
"{session_url}/@@start".format(
session_url=self.webhelpers.traversed_session.absolute_url()
)
)
Comment on lines +22 to +26
Copy link
Member

Choose a reason for hiding this comment

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

To make the code more readable I would add a def redirect method that would make this possible:

Suggested change
return self.request.response.redirect(
"{session_url}/@@start".format(
session_url=self.webhelpers.traversed_session.absolute_url()
)
)
return self.redirect("@@start")

or something similar, IDK.... self.webhelpers.redirect_to_session_path or whatever.
That can be useful also for the other view.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, but let's refactor later and also include browser/risk.py to keep things halfways consistent.

if self.webhelpers.redirectOnSurveyUpdate():
return
Comment on lines +27 to +28
Copy link
Member

Choose a reason for hiding this comment

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

I do not understand this if clause.
It seems to me the code is equivalent to:

Suggested change
if self.webhelpers.redirectOnSurveyUpdate():
return
self.webhelpers.redirectOnSurveyUpdate():

To me

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copied from

if self.webhelpers.redirectOnSurveyUpdate():

My guess is that it's to make sure that any code that might be added after this line is not executed if the redirect kicks in.


@property
@memoize
def navigation(self):
return api.content.get_view("navigation", self.context, self.request)

def _get_next(self, reply):
_next = reply.get("next", None)
# In Safari browser we get a list
if isinstance(_next, list):
_next = _next.pop()
return _next

@property
def tree(self):
return getTreeData(self.request, self.context)

@property
@memoize
def session(self):
return self.webhelpers.traversed_session.session

@property
@memoize
def survey(self):
"""This is the survey dexterity object."""
return self.webhelpers._survey

@property
@memoize
def choice(self):
return self.webhelpers.traversed_session.restrictedTraverse(
self.context.zodb_path.split("/")
)

@property
@memoize
def selected(self):
return [option.zodb_path for option in self.context.options]

def set_answer_data(self, reply):
answer = reply.get("answer", [])
if not isinstance(answer, (list, tuple)):
answer = [answer]
# XXX Check if paths are valid?
# for path in answer[:]:
# try:
# self.webhelpers.traversed_session.restrictedTraverse(path)
# except KeyError:
# answer.remove(path)
return self.context.set_options_by_zodb_path(answer)

def __call__(self):
# Render the page only if the user has inspection rights,
# otherwise redirect to the start page of the session.
if not self.webhelpers.can_inspect_session:
return self.request.response.redirect(
self.context.aq_parent.absolute_url() + "/@@start"
)
self.check_render_condition()

utils.setLanguage(self.request, self.survey, self.survey.language)

if self.request.method == "POST":
reply = self.request.form
if not self.webhelpers.can_edit_session:
return self.navigation.proceed_to_next(reply)
_next = self._get_next(reply)
# Don't persist anything if the user skipped the question
if _next == "skip":
return self.navigation.proceed_to_next(reply)

changed = self.set_answer_data(reply)

if changed:
self.session.touch()

return self.navigation.proceed_to_next(reply)
return super().__call__()
18 changes: 18 additions & 0 deletions src/euphorie/client/browser/configure.zcml
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,14 @@
layer="euphorie.client.interfaces.IClientSkinLayer"
/>

<browser:page
name="navigation"
for="*"
class="euphorie.client.browser.navigation.NavigationView"
permission="zope.Public"
layer="euphorie.client.interfaces.IClientSkinLayer"
/>

<browser:page
name="risk_macros"
for="*"
Expand Down Expand Up @@ -702,6 +710,16 @@
layer="euphorie.client.interfaces.IClientSkinLayer"
/>

<!-- Choice -->
<browser:page
name="identification"
for="euphorie.client.model.Choice"
class=".choice.IdentificationView"
template="templates/choice_identification.pt"
permission="euphorie.client.ViewSurvey"
layer="euphorie.client.interfaces.IClientSkinLayer"
/>

<browser:page
name="tool-more-info"
for="euphorie.client.adapters.session_traversal.ITraversedSurveySession"
Expand Down
105 changes: 105 additions & 0 deletions src/euphorie/client/browser/navigation.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,105 @@
from euphorie.client import model
from euphorie.client.interfaces import CustomRisksModifiedEvent
from euphorie.client.navigation import FindNextQuestion
from euphorie.client.navigation import FindPreviousQuestion
from plone import api
from plone.memoize.instance import memoize
from Products.Five import BrowserView
from sqlalchemy import and_
from z3c.saconfig import Session
from zope.event import notify


class NavigationView(BrowserView):
question_filter = None

@property
@memoize
def webhelpers(self):
return api.content.get_view("webhelpers", self.context, self.request)

@property
@memoize
def session(self):
return self.webhelpers.traversed_session.session

@property
@memoize
def previous_question(self):
return FindPreviousQuestion(
self.context, dbsession=self.session, filter=self.question_filter
)

@property
@memoize
def next_question(self):
return FindNextQuestion(
self.context, dbsession=self.session, filter=self.question_filter
)

def proceed_to_next(self, reply):
_next = reply.get("next", None)
# In Safari browser we get a list
if isinstance(_next, list):
_next = _next.pop()
Comment on lines +41 to +44
Copy link
Member

Choose a reason for hiding this comment

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

I like more what you did on the other view, where you have a dedicated _get_next method.
Any reason to not do the same here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The goal right now is only to move this from browser/risk.py so that we can use it for choices, too. Refactoring can be done later.

if _next == "previous":
target = self.previous_question
if target is None:
# We ran out of questions, step back to intro page
url = "{session_url}/@@identification".format(
session_url=self.webhelpers.traversed_session.absolute_url()
)
return self.request.response.redirect(url)
elif _next in ("next", "skip"):
target = self.next_question
if target is None:
# We ran out of questions, proceed to the action plan
if self.webhelpers.use_action_plan_phase:
next_view_name = "@@actionplan"
elif self.webhelpers.use_consultancy_phase:
next_view_name = "@@consultancy"
else:
next_view_name = "@@report"
base_url = self.webhelpers.traversed_session.absolute_url()
url = f"{base_url}/{next_view_name}"
return self.request.response.redirect(url)

elif _next == "add_custom_risk" and self.webhelpers.can_edit_session:
sql_module = (
Session.query(model.Module)
.filter(
and_(
model.SurveyTreeItem.session == self.session,
model.Module.zodb_path == "custom-risks",
)
)
.first()
)
if not sql_module:
url = self.context.absolute_url() + "/@@identification"
Copy link
Member

Choose a reason for hiding this comment

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

I am big fan of fstring:

Suggested change
url = self.context.absolute_url() + "/@@identification"
url = f"{self.context.absolute_url()}/@@identification"

They should be even faster, but what you did has nothing wrong.

return self.request.response.redirect(url)

view = api.content.get_view("identification", sql_module, self.request)
view.add_custom_risk()
notify(CustomRisksModifiedEvent(self.context.aq_parent))
risk_id = self.context.aq_parent.children().count()
# Construct the path to the newly added risk: We know that there
# is only one custom module, so we can take its id directly. And
# to that we can append the risk id.
url = "{session_url}/{module}/{risk}/@@identification".format(
session_url=self.webhelpers.traversed_session.absolute_url(),
module=sql_module.getId(),
risk=risk_id,
)
return self.request.response.redirect(url)
elif _next == "actionplan":
url = self.webhelpers.traversed_session.absolute_url() + "/@@actionplan"
return self.request.response.redirect(url)
# stay on current risk
else:
target = self.context
url = ("{session_url}/{path}/@@identification").format(
session_url=self.webhelpers.traversed_session.absolute_url(),
path="/".join(target.short_path),
)
return self.request.response.redirect(url)
87 changes: 5 additions & 82 deletions src/euphorie/client/browser/risk.py
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,6 @@
from Products.CMFPlone.utils import getAllowedSizes
from Products.Five import BrowserView
from Products.Five.browser.pagetemplatefile import ViewPageTemplateFile
from sqlalchemy import and_
from z3c.saconfig import Session
from zope.component import getUtility
from zope.deprecation import deprecate
Expand Down Expand Up @@ -487,10 +486,8 @@ def get_collapsible_section_state(self, collapsible_section_name):

@property
@memoize
def next_question(self):
return FindNextQuestion(
self.context, dbsession=self.session, filter=self.question_filter
)
def navigation(self):
return api.content.get_view("navigation", self.context, self.request)

@property
@memoize
Expand Down Expand Up @@ -547,11 +544,11 @@ def __call__(self):
if self.request.method == "POST":
reply = self.request.form
if not self.webhelpers.can_edit_session:
return self.proceed_to_next(reply)
return self.navigation.proceed_to_next(reply)
_next = self._get_next(reply)
# Don't persist anything if the user skipped the question
if _next == "skip":
return self.proceed_to_next(reply)
return self.navigation.proceed_to_next(reply)
old_values = {}
for prop, default in self.monitored_properties.items():
val = getattr(self.context, prop, default)
Expand Down Expand Up @@ -593,7 +590,7 @@ def __call__(self):
if changed:
self.session.touch()

return self.proceed_to_next(reply)
return self.navigation.proceed_to_next(reply)

else:
self._prepare_risk()
Expand Down Expand Up @@ -1009,80 +1006,6 @@ def _prepare_risk(self):
or "template-default"
)

@property
@memoize
def previous_question(self):
return FindPreviousQuestion(
self.context, dbsession=self.session, filter=self.question_filter
)

def proceed_to_next(self, reply):
_next = reply.get("next", None)
# In Safari browser we get a list
if isinstance(_next, list):
_next = _next.pop()
if _next == "previous":
target = self.previous_question
if target is None:
# We ran out of questions, step back to intro page
url = "{session_url}/@@identification".format(
session_url=self.webhelpers.traversed_session.absolute_url()
)
return self.request.response.redirect(url)
elif _next in ("next", "skip"):
target = self.next_question
if target is None:
# We ran out of questions, proceed to the action plan
if self.webhelpers.use_action_plan_phase:
next_view_name = "@@actionplan"
elif self.webhelpers.use_consultancy_phase:
next_view_name = "@@consultancy"
else:
next_view_name = "@@report"
base_url = self.webhelpers.traversed_session.absolute_url()
url = f"{base_url}/{next_view_name}"
return self.request.response.redirect(url)

elif _next == "add_custom_risk" and self.webhelpers.can_edit_session:
sql_module = (
Session.query(model.Module)
.filter(
and_(
model.SurveyTreeItem.session == self.session,
model.Module.zodb_path == "custom-risks",
)
)
.first()
)
if not sql_module:
url = self.context.absolute_url() + "/@@identification"
return self.request.response.redirect(url)

view = api.content.get_view("identification", sql_module, self.request)
view.add_custom_risk()
notify(CustomRisksModifiedEvent(self.context.aq_parent))
risk_id = self.context.aq_parent.children().count()
# Construct the path to the newly added risk: We know that there
# is only one custom module, so we can take its id directly. And
# to that we can append the risk id.
url = "{session_url}/{module}/{risk}/@@identification".format(
session_url=self.webhelpers.traversed_session.absolute_url(),
module=sql_module.getId(),
risk=risk_id,
)
return self.request.response.redirect(url)
elif _next == "actionplan":
url = self.webhelpers.traversed_session.absolute_url() + "/@@actionplan"
return self.request.response.redirect(url)
# stay on current risk
else:
target = self.context
url = ("{session_url}/{path}/@@identification").format(
session_url=self.webhelpers.traversed_session.absolute_url(),
path="/".join(target.short_path),
)
return self.request.response.redirect(url)

@memoize
def get_existing_measures_with_activation(self):
saved_standard_measures = {
Expand Down
Loading