lenra-io/lenra_cli

[Task] check template naming convention

Aperta

#284 aperta il 13 giu 2023

 (0 commenti) (0 reazioni) (1 assegnatario)Rust (2 fork)auto 404
enhancementhelp wanted

Metriche repository

Star
 (16 stelle)
Metriche merge PR
 (Metriche PR in attesa)

Descrizione

What is the issue ?

For now (not a bug), the check template command ask for a precise convention about the view naming. I think this is an issue and this limit the way that future template could work. For the elixir template, i've created an automatic binding/ref system. With this system, the user don't have to create any sort of manual binding or don't event have to know what the view "name" is at all. This binding system create the name based on the module of the created view. For example :

defmodule App.Views.Bar do
  use Lenra.View

  defview _ do
    %{
      "type" => "text",
      "value" => "Hello from Bar View"
    }
  end
end

App.View.Bar.name()
# > "Elixir.App.Views.Bar"

The issue is that the check template enforce certain names, otherwise an error is raised.

view:main           : Error
    view:main:unexpectedError
        Error loading view:main checker data: Request(Status(500, Response[status: 500, status_text: Internal Server Error, url: http://localhost:8080/]))
view:menu           : Error
    view:menu:unexpectedError
        Error loading view:menu checker data: Request(Status(500, Response[status: 500, status_text: Internal Server Error, url: http://localhost:8080/]))
view:home           : Error
    view:home:unexpectedError
        Error loading view:home checker data: Request(Status(500, Response[status: 500, status_text: Internal Server Error, url: http://localhost:8080/]))
view:counter        : Error
    view:counter:unexpectedError
        Error loading view:counter checker data: Request(Status(500, Response[status: 500, status_text: Internal Server Error, url: http://localhost:8080/]))

First, the manifest is wrong.

manifest            : Error
    manifest:sameValue:manifest.rootView
        Not matching value for manifest.rootView: got String("Elixir.App.Views.Main") but expected String("main")
    manifest:additionalProperty:manifest.jsonRoutes
        Additional property manifest.jsonRoutes
    manifest:additionalProperty:manifest.lenraRoutes
        Additional property manifest.lenraRoutes

Because it return default routes for json/lenra (the warnings) and return another name for the rootView.

// expected
{
  "manifest": {
    "rootView": "main"
  }
}
// Returned
{
  "manifest": {
    "jsonRoutes": [],
    "lenraRoutes": [],
    "rootView": "Elixir.App.Views.Main"
  }
}

Then, all the view are incorrect :

view:main           : Error
    view:main:unexpectedError
        Error loading view:main checker data: Request(Status(500, Response[status: 500, status_text: Internal Server Error, url: http://localhost:8080/]))
view:menu           : Error
    view:menu:unexpectedError
        Error loading view:menu checker data: Request(Status(500, Response[status: 500, status_text: Internal Server Error, url: http://localhost:8080/]))
view:home           : Error
    view:home:unexpectedError
        Error loading view:home checker data: Request(Status(500, Response[status: 500, status_text: Internal Server Error, url: http://localhost:8080/]))
view:counter        : Error
    view:counter:unexpectedError
        Error loading view:counter checker data: Request(Status(500, Response[status: 500, status_text: Internal Server Error, url: http://localhost:8080/]))

Because the check ask for the views main, home, menu, counter but the template names are Elixir.App.Views.Main, Elixir.App.Views.Home and so on..

What we should do (in my opinion)

  • We should remove the opinionated names and maybe let the template decide what the checker should call.
  • We should allow (encourage ?) the optional lenraRoutes and jsonRoutes, even empty.
  • We should remove the (deprecated ?) rootView and ask for a default / route on lenraRoutes

Technical recommandation (How to do that)

Open to suggestion ! We could allow a "private" configuration somewhere ? We could change the checks to simply get and run the / route whatever the name is and check the full result ?

Guida contributor