trento-project / web Goto Github PK

System Replication is missing from HANA databases overview and Sap system overview.

changeset =
  HostReadModel
  |> Repo.get(id)
  |> HostReadModel.changeset(%{
    heartbeat: :critical
  })

Ecto.Multi.update(multi, :host, changeset)

can be written

changeset =
  %HostReadModel{id: host_id}
  |> HostReadModel.changeset(%{
    heartbeat: :critical
  })

Ecto.Multi.update(multi, :host, changeset)

we test just embedded validation, would be cool to test required fields etc..

API and react routes have low dash and dash

choose your destiny

Allow the user to override the cluster cloud provider

Maybe a middleware is not required if we use domo precond

Now we have several controllers

• cluster_controller
• host_controller
• sap_system_controller
• database_controller

defining these two functions

• create_tag
• delete_tag

@spec create_tag(Plug.Conn.t(), map) :: Plug.Conn.t()
def create_tag(conn, %{
      "id" => id,
      "value" => value
    }) do
  case Tags.create_tag(value, id, "host") do
    {:ok, _} ->
      conn
      |> put_status(:accepted)
      |> json(%{})

    {:error, _} ->
      conn
      |> put_status(:bad_request)
      |> json(%{error: "tag creation failed"})
  end
end

@spec delete_tag(Plug.Conn.t(), map) :: Plug.Conn.t()
def delete_tag(conn, %{
      "id" => resource_id,
      "value" => value
    }) do
  case Tags.delete_tag(value, resource_id) do
    :ok ->
      conn
      |> put_status(:accepted)
      |> json(%{})

    :not_found ->
      conn
      |> put_status(:not_found)
      |> json(%{})
  end
end

The point is that delete_tag is exactly the same for all the controllers while create_tag only differs on the third argument of Tags.create_tag/3, being the resource_type (host, cluster, database, sap_system).

I believe we can improve a bit by:

• having clearer function names like tag_host, tag_sap_system, etc... and delegate the detail of the resource type
• putting delete_tag in a module to reduce duplication 🤔
• having a TaggingController where to route the various POST /<resourceType>/<resourceId>/tags and DELETE /<resourceType>/<resourceId>/tags/<value>

In sap system detail we don't know anything about the related database and the same is true the other way around.

Options are:

• we show db related info in sapsystem detail, and the other way around when available
• we provide a link to the Database detail in sapsystem detail and a link to the sapsystem detail in the Database detail when possible.

We need to project:

• CIB Last Written
• IPs and Virtual IPs
• Host IDs

assets/js/components/ClusterDetails/ClusterDetails.jsx

<div className="mt-8">
  <div>
    <h2 className="text-2xl font-bold">Pacemaker Site details</h2>
  </div>
</div>
<div className="mt-2">
  <h3 className="text-l font-bold">NBG</h3>
  <Table
    config={siteDetailsConfig}
    data={renderedNodes.filter(({ site }) => site === 'NBG')} // fix hardcoded
  />
  <h3 className="text-l font-bold">WDF</h3>
  <Table
    config={siteDetailsConfig}
    data={renderedNodes.filter(({ site }) => site === 'WDF')} // fix hardcoded
  />
</div>

In order to get the user attention is important events, having a prominent notification message option would come handy.

For example, when the runner is not available and I want to display some sort of big message, where the user has the option do something like close, try again, etc.

The current toast notifications look a bit small for this kind of business

Could be handy

Nested read models factory are tedious, ExMachina could be helpful

use factory / fixtures for the read models

to speed things up

We need to add this to our config and check if the compile time checks are executed correctly

config :my_app, MyApp.Endpoint,
  reloadable_compilers: [:phoenix, :domo_compiler] ++ Mix.compilers()

Switch credo to strict mode and fix what reported

commanded/recipes#21

The Host overview right now collects the sap systems each host belongs to using:

const getSapSystemsByHost = (sapSystems, hostId) => {
  const appInstances = sapSystems
    .flatMap((sapSystem) => sapSystem.application_instances)
    .filter((sapSystem) => sapSystem.host_id === hostId)
    .map((sapSystem) => ({ type: 'sap-systems', instance: sapSystem }));
  const dbInstances = sapSystems
    .flatMap((sapSystem) => sapSystem.database_instances)
    .filter((sapSystem) => sapSystem.host_id === hostId)
    .map((sapSystem) => ({ type: 'databases', instance: sapSystem }));

  return appInstances.concat(dbInstances);
};

we could instead use the getInstancesOnHost from @state/selectors. I did a quick try but it seems that calling useSelector on each host iteration on the list breaks the app after a refresh.

We should improve this.

In order to reach parity, the tags creation should offer an autocomplete feature with the already existing tags of this resource (host, clusters, etc).

Besides this, tags "edition mode" doesn't go away unless enter key is pressed. I don't know if this is desired or not

Module names should reflect their location in the architecture.

For instance the application service Trento.SapSystems.HealthSummaryService should be Trento.Application.UseCases.SapSystems.HealthSummaryService.

It doesn't have to be prescriptive, yet clear and useful.
Let's have a discussion about this.

For instance the dto located in lib/trento/application/usecases/sap_systems/dto/health_summary_dto.ex

could be
Trento.Application.UseCases.SapSystems.Dto.HealthSummary or
Trento.Application.UseCases.SapSystems.Dto.HealthSummaryDto 👎

or simply
Trento.Application.UseCases.SapSystems.HealthSummaryDto

which is also acceptable.

check mox/hammox
phoenix controller testcase + ecto sandbox + factory

Make expandable tables trigger when clicking on an arrow icon rather than the whole row.
Might simplify frontend implementation.

..for the sake of clarity

Consider bubbling up the whole changeset or use this: https://hexdocs.pm/ecto/Ecto.Changeset.html#traverse_errors/2

As of now, if an uncatched error happens into the app, we see a pretty non-expressive white page. We need an outer error boundary to display at least an error message to end-users.

https://reactjs.org/docs/error-boundaries.html

warn - Thepurge/contentoptions have changed in Tailwind CSS v3.0. warn - Update your configuration file to eliminate this warning.

Let's borrow inet:ntoa/1 from erlang

https://hexdocs.pm/phoenix/Phoenix.Controller.html#action_fallback/1

Currently our tagging system allows empty tags.
If I press enter on a tag input without typing any text, trento allows that tag to be created.

I would argue about the usefulness of an empty tag already here.

Then this opens the issue of not being able to delete those tags.
When I refresh the page I get the items with the empty tag pill, and when I try to delete it, the backend goes 404 because of a non-existent deletion endpoint.

Simply put

the deletion endpoint should be something like

DELETE /api/clusters/6bd7ec60-8cb1-5c6b-a892-29e1fd2f8380/tags/<the-tag-value>

When <the-tag-value> is empty, the computed enpoint being called is

DELETE /api/clusters/6bd7ec60-8cb1-5c6b-a892-29e1fd2f8380/tags/

which does not exist.

At the moment the host overview and others are missing the link to the related cluster.

Let's add it.

In order to make development easier, when configuration flags are involved, the application should allow the usage of local configurations.

An use case:

• I want to use the real runner adapter in dev environment

http://blog.plataformatec.com.br/2015/10/mocks-and-explicit-contracts/