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

Support scaled answers #715

Closed
wants to merge 50 commits into from
Closed

Support scaled answers #715

wants to merge 50 commits into from

Conversation

mauritsvanrees
Copy link
Contributor

Add support for scaled answers. Usually: answers on a scale of 1-5.
This includes improvements in the navigation, mostly creating more possibilities for overriding code.
This seems safe enough to be merged and used, also when the specific project does not use the new scaled answer possibilities.
We could also split this up into multiple PRs, and do git rebases, to make this easier to digest.

mauritsvanrees and others added 30 commits February 2, 2024 11:30
They were marked as unvisited until now.
By default the first answer gets a value of 1, but if you want to have the "best" answer at the top, you can encode it like this: `very safe|5`.
In the risk content, use scaled_answers plural to store the possible answers.
In the risk client (sql), use scaled_answer plural to store the chosen answer.
There can easily be an empty row at the end.
The scaled answer itself is not enough to know if the risk is present or not.
This should be determined based on the module, and this is better done by overriding this method.
…ew 'oira_navigation_tree'.

Then we can subclass this browser view elsewhere to change the tree in one spot, instead of needing to override the tree in multiple views.
@mauritsvanrees
Copy link
Contributor Author

@ale-rt I made navigation_tree_legend a property as you requested. I have pushed the same change to the daimler.oira branch.

@ale-rt
Copy link
Member

ale-rt commented Mar 22, 2024

@ale-rt I made navigation_tree_legend a property as you requested. I have pushed the same change to the daimler.oira branch.

I meant something like this: #716
I moved that property to an attribute.
It can still be customized with a property.

@mauritsvanrees
Copy link
Contributor Author

Ah, sorry, now I get it. Makes sense. I have merged your PR.

ale-rt added a commit that referenced this pull request Apr 19, 2024
ale-rt added a commit that referenced this pull request Apr 19, 2024
ale-rt added a commit that referenced this pull request Apr 19, 2024
ale-rt added a commit that referenced this pull request Apr 19, 2024
mauritsvanrees added a commit that referenced this pull request Apr 19, 2024
…ew 'oira_navigation_tree'.

Then we can subclass this browser view elsewhere to change the tree in one spot, instead of needing to override the tree in multiple views.
Taken from #715
mauritsvanrees added a commit that referenced this pull request Apr 19, 2024
These are answers on a scale from usually 1-5, instead of only yes/no.
This is the main part from #715.
We are pulling that apart into some more digestible chunks, instead of 50 commits, including merges of main.
@mauritsvanrees mauritsvanrees marked this pull request as draft April 19, 2024 11:52
@mauritsvanrees
Copy link
Contributor Author

@ale-rt and me are splitting this up into multiple PRs so it is more understandable.

@mauritsvanrees
Copy link
Contributor Author

The alternative PRs have been merged. We are done here.

@ale-rt
Copy link
Member

ale-rt commented Apr 19, 2024

Almost, I merged main in the multiple answer branch and now I have only this thing left:

--- a/src/euphorie/client/browser/risk.py
+++ b/src/euphorie/client/browser/risk.py
@@ -1245,7 +1245,9 @@ class ActionPlanView(RiskBase):
 
     @property
     def risk_postponed(self):
-        return self.context.identification is None
+        return (
+            self.context.identification is None and self.context.scaled_answer is None
+        )
 
     @property
     def use_problem_description(self):

@ale-rt
Copy link
Member

ale-rt commented Apr 19, 2024

^ This was removed on purpose :)

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