Add sap-system/databases linking from host overview #369
Conversation
rtorrero
requested review from
fabriziosestito,
dottorblaster,
nelsonkopliku and
arbulu89
and removed request for
dottorblaster
| return instance; | ||
| }; | ||
|
|
||
| const getSapSystemsByHost = (sapSystems, hostId) => { |
There was a problem hiding this comment.
I believe this can be a selector on the state, selecting all the instances on a host.
There was a problem hiding this comment.
Ah! nice, I see you used it in HostDetails, going to give it a try
There was a problem hiding this comment.
I'll create a ticket to figure out later how to use this. It seems to work on first load, but breaks after a refresh. I suspect that useSelector is not intended to be used each time we iterate on the hosts list to display each entry of the table. The warning itself after it breaks is:
Warning: React has detected a change in the order of Hooks called by HostsList. This will lead to bugs and errors if not fixed. For more information, read the Rules of Hooks: https://reactjs.org/link/rules-of-hooks
|
as a reminder for possible improvements. I am not completely confident with the choice of how we show the sap systems on a host. Then are we solely interested in instances or the aggregation As an alternative UI I'd consider having sort of a popover if the information needs to be more than the simple sid. |
assets/js/components/HostsList.jsx
Outdated
| filter: (filter, key) => (element) => | ||
| element[key].some((sid) => filter.includes(sid)), | ||
| render: (sids, { sap_systems }) => { | ||
| let sidsArray = sids.map((sid, index) => [ |
There was a problem hiding this comment.
I would maybe just return this map instead of storing and returning later
rtorrero
force-pushed
the
add-cluster-details-links
branch
from
f6e5b34 to
4a40135
Compare
This PR adds linking to sap-systems and databases overview from the SID column. I've added it as draft as I'm not really sure there is a better way to do this. It gets a bit ugly when we need to apply some logic to find out if the SID should link to a DB or a sap-system.
Thanks to @arbulu89 for the help!