Fix process-node-related bugs #967
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #967 +/- ##
==========================================
- Coverage 68.29% 68.28% -0.02%
==========================================
Files 110 110
Lines 6210 6211 +1
==========================================
Hits 4241 4241
- Misses 1969 1970 +1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
edan-bainglass
force-pushed
the
bug-fix
branch
from
72020be to
ebd9e79
Compare
|
@superstar54 rebased. Please review once tests finish 🙏 |
| @@ -189,6 +189,8 @@ def _update_node_view(self, nodes, refresh=False): | |||
| self.node_view_container.children = [self.node_view_loading_message] | |||
| self.node_view = self._create_workchain_viewer(node) | |||
| self.node_views[node.uuid] = self.node_view | |||
| else: | |||
There was a problem hiding this comment.
Could you add a comment on what this case means and why return?
There was a problem hiding this comment.
This guard was intended to cover cases where you select a non-QeAppWorkChain WorkChainNode. We decided not to change the rendered view in this case. However, I think it is best instead to render a "No viewer available for this node" message until one is developed (if we decide to do so). This is now implemented.
There was a problem hiding this comment.
Thanks! Could you add it as comment lines in the code?
There was a problem hiding this comment.
The current implementation is now self-documenting, no? If you're not registered in the cache AND ARE a WorkChainNode BUT ARE NOT the QeAppWorkChain, then you don't have a viewer. A comment would be redundant at this point.
There was a problem hiding this comment.
I didn't notice you pushed a new commit to replace the return with a message. Now, it's clear in the code.
Based on #946
This PR handles a couple of bugs related to the process node, specifically loading node views and managing the process state.