Handle unnamed cluster event #497
Conversation
arbulu89
requested review from
dottorblaster,
fabriziosestito,
rtorrero,
stefanotorresi and
nelsonkopliku
assets/js/components/ClusterLink.jsx
Outdated
| const truncatedLength = 14; // Using 14 as it cuts the uuid format after the 2nd - | ||
| export const getClusterName = (cluster) => { | ||
| return cluster?.name || cluster?.id.slice(0, truncatedLength) + '...'; | ||
| }; | ||
|
|
||
| const ClusterLink = ({ cluster }) => { | ||
| const clusterName = getClusterName(cluster); | ||
|
|
There was a problem hiding this comment.
Instead of just cutting this way, an alternative approach is letting the browser deal with this through the text-ellipsis class, you wanna try?
There was a problem hiding this comment.
Updated here: e61e618
|
Moving to draft. I have found a nasty bug. Even though the frontend is updated, the database is not, so in page refresh, the name goes back to the previous one apparently. Working on it |
arbulu89
force-pushed
the
handler-unnamed-cluster-event
branch
from
234c2b3
to
e61e618
Compare
arbulu89
force-pushed
the
handler-unnamed-cluster-event
branch
from
e61e618
to
2850cbf
Compare
There was a problem hiding this comment.
Nice job!
Here's a though about the UI of the cluster detail.
If no cluster name has been detected, I'd consider:
- showing something like
N/Aundefinedor whatever else better comes to your mind - possibly add the full id in the details card (it would give a more comprehensive information about the resource, and maybe we could do the same in other pages)
No strong opinions here, just sharing some thoughts.
| @@ -17,7 +17,7 @@ defmodule Trento.ClusterReadModel do | |||
| @derive {Jason.Encoder, except: [:__meta__, :__struct__]} | |||
| @primary_key {:id, :binary_id, autogenerate: false} | |||
| schema "clusters" do | |||
| field :name, :string | |||
| field :name, :string, default: "" | |||
There was a problem hiding this comment.
My ocd calls for a nil here. 😄
Would that make sense? And would it be possible or does it have to be an empty string because of ecto?
There was a problem hiding this comment.
If we don't set the default value, when we get a nil value in the name field (which is the current scenario), it doesn't apply any change in the persistent storage, I guess because ecto works this way.
There was a problem hiding this comment.
@nelsonkopliku I will add some default value in the name field by now. Let's postpone the addition of the ID, as it would requires to change the nice 3x3 grid we have now, and I don't want to play with that by now hehe
There was a problem hiding this comment.
Here we go (a434de3):
Handle unnamed cluster (field
Namecoming empty in the cluster discovery event).In the frontend, it shows a truncated version of the cluster id. I have chosen 14 chars length mostly for cosmetic reasons, as it is the length that better fits in the current views.
Here some pics:
Clusters view:
Cluster details view:
Hosts view:
