Select rented node in automated selection in case of its existence #3749
Conversation
maayarosama
requested review from
AhmedHanafy725,
zaelgohary,
MohamedElmdary,
Mahmoud-Emad,
amiraabouhadid,
AlaaElattar,
0oM4R and
samaradel
as code owners
| const profileManager = useProfileManager(); | ||
| let node; |
There was a problem hiding this comment.
no need to include the profile manager here, we can pass it from the caller component TfAutoNodeSelector as it already has the gridstore.grid.twinID
| node = nodes.find(n => { | ||
| return n.rentedByTwinId === profileManager?.profile?.twinId; | ||
| }); | ||
|
|
||
| if (node && isNodeValid(getFarm, node, selectedMachines, filters)) { | ||
| if (nodesLock && !locked) { | ||
| release(nodesLock); | ||
| } | ||
| return node; | ||
| } |
There was a problem hiding this comment.
in this case we will always override the old selected node if there is a rented node is that the required case ?
and i suggest for reduce code redundancy :
const rentedNode = nodes.find(n => n.rentedByTwinId == twinId);
if (oldSelectedNodeId || rentedNode) {
const node = rentedNode || nodes.find(n => n.nodeId === oldSelectedNodeId);
if (node && isNodeValid(getFarm, node, selectedMachines, filters)) {
if (nodesLock && !locked) {
release(nodesLock);
}
return node;
}
}
|
please fix build error |
|
it shows the rented node (id 271) at the top of the list but then it autoscrolls to another node |
There was a problem hiding this comment.
Update test scenarios:
- test if more than one rented node is available (maybe two or three)
- test if the user has more than one rented, which one will be suggested and on which conditions.
- un-reserv one or more rented node and the automation selection should get back to the normal behavior.
- test if the rented node is in grace period; (should not be selected in this case)
Was node 271 rented by you? |
Yes |
| if (oldSelectedNodeId) { | ||
| const node = nodes.find(n => n.nodeId === oldSelectedNodeId); | ||
| if (oldSelectedNodeId || rentedNode) { | ||
| const node = rentedNode || nodes.find(n => n.nodeId === oldSelectedNodeId); |
There was a problem hiding this comment.
we need to keep the old selected node if it exists.
| const node = rentedNode || nodes.find(n => n.nodeId === oldSelectedNodeId); | |
| const node = nodes.find(n => n.nodeId === oldSelectedNodeId) || rentedNode ; |
There was a problem hiding this comment.
I remember in Th BAM we agreed on always choosing the rented node, @AhmedHanafy725 can you confirm that we'll choose the old selected node even if there's a rented node?
There was a problem hiding this comment.
yes we should select the old selected node
…old selected node will be picked
Description
Select rented node in automated selection in case of its existence
Related Issues
Tested Scenarios
Checklist