Upgrade to Phoenix 1.7

[marvelm]

Overview

First off of all, thank you for making and maintaining this project! It's extremely useful to me. I'm really happy that it's written in Elixir because I can actually help out 😄

I noticed that the project is still using Phoenix 1.6. I followed the upgrade guide and it was very simple.

I also made a bunch of changes to get rid of all the compile-time warnings.

Testing

I'm not quite sure how to QA these changes. Maybe we could come up with a checklist together?

[netlify]

✅ Deploy Preview for teslamate ready!

Name	Link
🔨 Latest commit	c0672f3
🔍 Latest deploy log	https://app.netlify.com/sites/teslamate/deploys/65c3c9b506da7c0008eb15e5
😎 Deploy Preview	https://deploy-preview-3615--teslamate.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

---

To edit notification comments on pull requests, go to your Netlify site configuration.

[JakobLichterfeld]

Thanks for your work! At least all 321 test cases still pass :-)

[JakobLichterfeld]

lgtm

[JakobLichterfeld]

Test cases pass, but linting failed, please format the files correctly, see https://github.com/teslamate-org/teslamate/actions/runs/7523079638/job/20657593930?pr=3615#step:12:9

[JakobLichterfeld]

Please lint properly

[marvelm]

Thanks for reviewing @JakobLichterfeld

CI is passing now.

[JakobLichterfeld]

CI is passing now.

Unfortunately not, https://github.com/teslamate-org/teslamate/actions/runs/7602510278/job/20718030463?pr=3615#step:10:10

[JakobLichterfeld]

@brianmay can you help with the outdated pot file? https://github.com/teslamate-org/teslamate/actions/runs/7767265925/job/21183920267?pr=3615#step:10:11

Running mix gettext.extract --merge manually works fine.

[brianmay]

I am not going to be able to update this PR as is, I probably don't have access to the source repo.

Apologies if I misunderstood something here ;-)

[JakobLichterfeld]

I am not going to be able to update this PR as is, I probably don't have access to the source repo.

I was able to merge our master into his repo.

Apologies if I misunderstood something here ;-)

Do you have a hint how to solve the outdated pot warning?

[brianmay]

The mix gettext.extract --merge command is probably a good start... Looks like some new messages have been added.

But I am a bit puzzled why we aren't getting errors from the master branch. Unless something is wrong with the pot checks there.

[JakobLichterfeld]

I linted the files correctly.
@marvelm please fix the test: https://github.com/teslamate-org/teslamate/actions/runs/7789209181/job/21240291062?pr=3615#step:10:73
Not sure, why it is expecting 4.32kWh instead of 4.32 kWh

and the Linting warning about the uncalled function: https://github.com/teslamate-org/teslamate/actions/runs/7789209181/job/21240291393?pr=3615#step:16:34

[brianmay]

I think that uncalled function warning is a false warning, fetch_settings is used as a plug. Not sure how to make it happy here. Maybe I missed something.

[brianmay]

The other error, I think there is a missing space in the template:

diff --git a/lib/teslamate_web/live/car_live/summary.html.heex b/lib/teslamate_web/live/car_live/summary.html.heex
index 70d0b72f..ae9632b1 100644
--- a/lib/teslamate_web/live/car_live/summary.html.heex
+++ b/lib/teslamate_web/live/car_live/summary.html.heex
@@ -272,7 +272,7 @@
             <%= unless is_nil(@summary.charge_energy_added) do %>
               <tr>
                 <td class="has-text-weight-medium"><%= gettext("Charged") %></td>
-                <td><%= @summary.charge_energy_added %>kWh</td>
+                <td><%= @summary.charge_energy_added %> kWh</td>
               </tr>
             <% end %>
             <%= unless is_nil(@summary.charger_power) do %>

This appears to have happened in the "mix format" commit. I can't reproduce this, but something we may need to watch out for.

[brianmay]

Possible solution to the unused function problem, just make the function public:

diff --git a/lib/teslamate_web/router.ex b/lib/teslamate_web/router.ex
index 246b47fa..f00d9ea0 100644
--- a/lib/teslamate_web/router.ex
+++ b/lib/teslamate_web/router.ex
@@ -54,7 +54,7 @@ defmodule TeslaMateWeb.Router do
     put "/car/:id/logging/suspend", CarController, :suspend_logging
   end

-  defp fetch_settings(conn, _opts) do
+  def fetch_settings(conn, _opts) do
     settings = Settings.get_global_settings!()

     conn

[JakobLichterfeld]

Giving up on this. A whitespace result in test failing, using the unicode character result in failed lint. Assume we need to adopt the test to use the whitespace

[brianmay]

It looks like that worked. Maybe the tests require the unicode whitespace character.

[JakobLichterfeld]

Using whitespace in html and in test instead of nbsp unicode finally works.