lenra-io/lenra_cli

[Task] check template naming convention

Open

#284 opened on Jun 13, 2023

 (0 comments) (0 reactions) (1 assignee)Rust (2 forks)auto 404
enhancementhelp wanted

Repository metrics

Stars
 (16 stars)
PR merge metrics
 (PR metrics pending)

Description

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 ?

Contributor guide