Global code review #9

jpilet · 2024-06-04T19:16:31Z

I am using this pull request to write down my review started on 4.6.2024.

* feat: bring interface from another repo * feat: bring code for the ESP32 * feat: add graph in the readme * feat: recreate database * feat: bring user login from another repo

* feat: display averages for the last two months, refactor the way data is retrieved and placed in the pie chart * feat: token added to link for API data, change to display of month element * feat: link login element with login backend * feat: link login element with login backend * feat: the token in sampleContext takes the value from the user who logged in * feat: store the token in localstorage * fix: fix small errors to launch npm run build * feat: added login functionality, created a function to get tokens from localstorage * style: change input form design * feat: redirect the user on login if the token is wrong * feat: create a function that checks for 401 errors * refactor: the method of collecting data and putting it into the monthly chart * feat: add log out button

* feat: group docker compose together * feat: move docker compose to root * fix: rename secret example file the name it had before made esphome think it was an esp32 config file * refactor: rename login_login to login login_login is a pleonasme, it's useless to say login twice * feat: Launch php login backend with the docker * refactor: move sql file to root it's the only required file for the database, therefore it doesn't need to be in another directory * refactor: rewrite Readme.md * feat: launch react build with the web server * feat: isolate the sub domain from the domain before if you were making a request to anything.domain.com, the domain would be domain.com. Now the domain will be anything.domain.com it means that now everything starting with login.memoires-info.com will be run as php code instead of being ignored the reason it worked before was because the uri contained "login." * feat: access everythign from the same web server * feat: update readme * feat: change web server port to be the default one User now don't need to specify the port when accessing the web server * refactor: move sql script to it's own directory now postgres data won't apear in root * refactor: rename assets folder to be hidden * refactor: move postgrest config in another file * feat: add a way to get postgrest error in php login * fix: update api test file * refactor: shorten gitignore file * feat: automatically create the db * feat: implement login (#5) * feat: display averages for the last two months, refactor the way data is retrieved and placed in the pie chart * feat: token added to link for API data, change to display of month element * feat: link login element with login backend * feat: link login element with login backend * feat: the token in sampleContext takes the value from the user who logged in * feat: store the token in localstorage * fix: fix small errors to launch npm run build * feat: added login functionality, created a function to get tokens from localstorage * style: change input form design * feat: redirect the user on login if the token is wrong * feat: create a function that checks for 401 errors * refactor: the method of collecting data and putting it into the monthly chart * feat: add log out button * fix: add link to esphome github page --------- Co-authored-by: jordyBSK <125542402+jordyBSK@users.noreply.github.com>

fix: fix 404 when trying to access router path fix: rename ngnix conf file fix: add font back in esp32 config fix: post request of type json now get decoded feat: get esp jwt token from the php fix: "sequence user_id_seq not exist" when creating database fix: give nginx it's real name back I'll probably write it wrong next time I have to write it ^^" fix: add lockfile to ensure plugin's hash match feat: make production docker compose fix: take pg password from .env fix: put interface in the right directory and remove adminer fix: rename add-esp.php to esp.php cause ngnix fail with the - refactor: change docker images to take images from last feature version at this date fix: copy .env in the php container fix: handle web socker for esphome esphome needs web sockets to work, without them it just freezes right before the compilation dev: script to reset the state of the mounted volumes\n\n this script is ignored by the gitignore feat: specify default user in readme refactor: change the url to use Subdirectory instead of subdomain fix: repare sed at build of prod fix: postgrest subdirectory now redirect to postg-rest container fix: restore php and postgrest url call fix: 404 when sending data from esp to database refactor: group all .env file in a single one fix: update readme fix: update readme fix: make readme easier to understand fix: remove hard coded env variable from Dockerfile.Postgres

* feat: add new plan card * feat: differentiate the different ip's in the database * feat: add a new room in database * feat: dislay name of room in api * feat: sorted all data by ip * feat: put the data in cardPlanElement * feat:change button for selected month, change function add room, change plan style * feat: add floor plan elements with sensors * feat: add a circle to the svg with the esp coordinates in the database

Dockerfile.Postgres

Dockerfile.Php

database/db.sql

jpilet · 2024-06-04T19:31:38Z

Interface/src/elements/LoginElement.tsx

+    const submit = async (e: FormEvent<HTMLFormElement>) => {
+        e.preventDefault();
+
+        await fetch(`${SampleContext.urlLogin}/login.php?username=${username}&password=${password}`, {


either use:

try { response = await fetch(...); } catch(...) {...}

or .then .catch without await

jpilet · 2024-06-04T19:34:02Z

Interface/src/elements/LoginElement.tsx

+                if (reponse.token) {
+                    localStorage.setItem('token', reponse.token);
+                    localStorage.setItem('username', username);
+                    window.location.replace("http://localhost:5173/dashboard");


don't hardcode protocol, host and port. localhost:5173 is probably wrong.
A relative path would be better.

…function (#21) * !feat: get ip in the sql average function ouput * feat: get count of entries in average function output now that data are separated from the other, some may accidentaly be more important than other ones when trying to get the global average, with the count as an information it is possible to weight the outputs to ensure none of them are more important

feat: initialize sqitch feat: create schema and roles for the postgrest api feat: create roles to distinguish users esp & login feat: Add table to store data feat: add a table to store user and passwords fix: test migrations refactor: remove old useless stuff feat: launch migration on docker start fix: ensure container wait enougth before starting

refactor: change docker image versions style: use latex instead of html to display color\n\nhtml isn't properly render on github while latex is\nthis line is here to show the user what the line with the ip address looks like on the esphone, the color matches the ones on esphome fix: ask to change the ip in secret.yaml refactor: use post request to send data to backend with curl dev: make gitignore more precise

refactor: rename login/ -> php/\n\ncalling the folder php is more representative of what there is inside refactor: move files from public folder to private refactor: move Dockerfile in their own folder

feat: install php stan to lint code fix: class NoReturn not exist fix: no value in iterrable array dev: set paths in phpstan config file Before you had to set it by hand every time you wanted to lint the code

* revert: remove interface * feat: change interface with nextjs

* feat: implement login * fix: dashboard redirection

* feat: get lastdata from database * fix: function if in data.ts

jpilet · 2024-06-18T20:07:26Z

README.md

+```bash
+cp .env.example .env
+cp esp32/config/secrets.yaml.example esp32/config/secrets.yaml
+cp Interface/src/contexts/SampleContext.example.tsx Interface/src/contexts/SampleContext.tsx


the file SampleContext.example.tsx does not exist.
Can you update this?

jpilet · 2024-06-18T20:17:05Z

docker-compose.yml

+  migration:
+    build:
+      context: .
+      dockerfile: Dockerfile.Sqitch


this file has been moved to Dockerfile/ folder

Do we really need 2 docker-compose files?
maybe we can reduce duplication with https://docs.docker.com/compose/multiple-compose-files/

oh right I forgot to change the path there but the docker-compose.yml is supposed to be the one for development is a bit outdated, the production one is docker-compose.prod.yml

I've looked at the link you provide and for debugging it is better for us to have the docker-compose.yml while for production we want to have the smallest and fastest program so we use the docker-compose.prod.yml wich automatically install dependencies and build the project in a way that only the build files will be keep inside the docker image

I am fine with differenciating prod/dev environments.

The problem is that we have redundant code in both docker-compose yml files. And then, the risk of forgetting to update one is big. So we could "include" things that are common to both files.

jpilet · 2024-06-18T20:18:21Z

docker-compose.yml

+    ports:
+      - "80:80"
+    volumes:
+      - ./Interface/dist:/var/www/memoires-info/html


The Interface folder does not exist.

jpilet · 2024-06-18T20:21:03Z

nextjs-interface/README.md

+First, run the development server:
+
+```bash
+npm run dev


when I run "npm run dev", I get

> climat_guardian_nextjs@0.1.0 dev > next dev sh: 1: next: not found

npm install is required first

jpilet · 2024-06-18T20:29:31Z

Dockerfile/Dockerfile.Nginx

@@ -0,0 +1,19 @@
+# node 20, build the application
+FROM node:20.12.2-alpine3.19
+COPY ../Interface .


I do not have the Interface folder. Where does it come from?

is it nextjs-interface ?

jpilet · 2024-06-18T20:36:30Z

Dockerfile/Dockerfile.Nginx

+# nginx 1.26 on port 80, keep the build output, nginx config
+FROM nginx:1.26-alpine-otel
+EXPOSE 80
+COPY --from=0 ../dist /var/www/memoires-info/html


../dist does not exist...
is it nextjs-interface/.next ?

docker-compose.yml

* feat: create table esp * feat: revert table esp * feat: verify table esp

* feat: avgData interface * feat: change type of data in data.ts * feat: change ip adress in EspLinksElement.tsx * feat: implement data in room chart * feat: change date format in chart * feat: add ip parameter in api ur * feat: implement sort data in page with esp ip

This reverts commit f12bde6.

* feat: add ip parameter in useLastData * refactor: refactor code with prettier * refactor: change esp ip in Plan.tsx * feat: add espMap.tsx components * feat: display last data on map point in plan * refactor: create AddPointElement.tsx to create new point in map * refactor: refactor code with prettier

* feat: added dynamic data in dashboard and esp pages * fix: fixed function in map

* feat: retrieves and displays date in datepicker * refactor: refactor and remove unused import * feart: set the default date to the current month * feat: add date parameter in useFetchData function * feat: implement date range * refactor: refactor code

* refactor: add userIcon * refactor: create function to get all users in database * refactor: create user interface in context.tsx * refactor: install bcrypt package to hash password * revert: install bcrypt package to hash password * revert: install bcrypt package to hash password * feat: implement add an user in database * refactor: moves icon files into svgs files * refactor: change icon path in NavLinksElement.tsx * refactor: migration to give web user rights to delete and add a user * refactor: install bcrypt package to hash password * feat: implement add and delete an user in database * style: reformat code

* refactor: changed font size * refactor: removed unused import * refactor: created new component to get token * refactor: created new component to get token * fix: fixed function getting user token to get esp token * refactor: removed console.log * fix: only get token when asked * style: reformated files

* fix: fixed jwt token fetch getting no Ip * style: reformated files

feat: setup a router fix: correct nginx config to work with router feat: serve the web pages using the router refactor: require autoload and lib from index.php fix: update interface according to new format --------- Co-authored-by: Alex-zReeZ <145986307+Alex-zReeZ@users.noreply.github.com>

…p dynamically (#97) * chore: removed console.log * refactor: removed window.location.reload and added dynamically new esp in sidebar * refactor: remade logic to only use one map * refactor: fix warning exception caught locally * style: reformated file

* refactor: added verification of ip in js * style: reformated file * refactor: simplified regex (shorter) * style: reformated file * refactor: compacted regex

) * refactor: added timestamp on graph hover * refactor: added import react from reacr * feat: added bar for every new day * style: reformated file and removed unused import

Co-authored-by: Alex-zReeZ <145986307+Alex-zReeZ@users.noreply.github.com>

* refactor: removed console log * refactor: added hour in graph xAxis

* fix: set timezon UTC+02:00 and removed seconde on timestamp * style: reformated file * fix: fix date possibly having no value * chore: showing returned error in auth thest * chore: added debugging * test: do only the curl to see if it works * refactor: merge docker and test in one run * refactor: added a waiting time for the server to be ready * refactor: changed method of waiting service to POST * refactor: moved sleep just before the curl * refactor: readded token content again

* feat: added zooming interface in plan closes: #115 * style: reformated code * style: reformated code

* feat: show the day when clicking on it closes: #116 * style: format

* feat: correct espId link when clicking on the day, for each esp * style: reformat

* feat: show variance between days with two esp * feat: show variance between days with two esp * style: reformat code * refactor: reformat code * refactor: reformat code * style: responsive esp page, dashboard * refactor: reformat code * style: remove comment

refactor(database): changed sqitch dockerfile to only docker compose No more need of the dedicated docker file. Migrations mounted in the container directly from docker compose. Closes: #127

Closes: #134

…#135) Bumps [micromatch](https://github.com/micromatch/micromatch) from 4.0.7 to 4.0.8. - [Release notes](https://github.com/micromatch/micromatch/releases) - [Changelog](https://github.com/micromatch/micromatch/blob/master/CHANGELOG.md) - [Commits](micromatch/micromatch@4.0.7...4.0.8) --- updated-dependencies: - dependency-name: micromatch dependency-type: indirect ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

Bumps [@types/react](https://github.com/DefinitelyTyped/DefinitelyTyped/tree/HEAD/types/react) from 18.3.3 to 18.3.4. - [Release notes](https://github.com/DefinitelyTyped/DefinitelyTyped/releases) - [Commits](https://github.com/DefinitelyTyped/DefinitelyTyped/commits/HEAD/types/react) --- updated-dependencies: - dependency-name: "@types/react" dependency-type: direct:development update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

Bumps [@radix-ui/react-popover](https://github.com/radix-ui/primitives) from 1.0.7 to 1.1.1. - [Changelog](https://github.com/radix-ui/primitives/blob/main/release-process.md) - [Commits](https://github.com/radix-ui/primitives/commits) --- updated-dependencies: - dependency-name: "@radix-ui/react-popover" dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

Bumps [@radix-ui/react-label](https://github.com/radix-ui/primitives) from 2.0.2 to 2.1.0. - [Changelog](https://github.com/radix-ui/primitives/blob/main/release-process.md) - [Commits](https://github.com/radix-ui/primitives/commits) --- updated-dependencies: - dependency-name: "@radix-ui/react-label" dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

Bumps [@radix-ui/react-slot](https://github.com/radix-ui/primitives) from 1.0.2 to 1.1.0. - [Changelog](https://github.com/radix-ui/primitives/blob/main/release-process.md) - [Commits](https://github.com/radix-ui/primitives/commits) --- updated-dependencies: - dependency-name: "@radix-ui/react-slot" dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

Rignchen and others added 5 commits April 8, 2024 09:59

refactor: change repository (#1)

be1792d

* feat: bring interface from another repo * feat: bring code for the ESP32 * feat: add graph in the readme * feat: recreate database * feat: bring user login from another repo

jpilet commented Jun 4, 2024

View reviewed changes

Rignchen and others added 9 commits June 7, 2024 08:38

refactor: reposition php code in the folder (#25)

3148fca

refactor: rename login/ -> php/\n\ncalling the folder php is more representative of what there is inside refactor: move files from public folder to private refactor: move Dockerfile in their own folder

fix: install php stan (#26)

b983d79

feat: install php stan to lint code fix: class NoReturn not exist fix: no value in iterrable array dev: set paths in phpstan config file Before you had to set it by hand every time you wanted to lint the code

Interface next js (#28)

6283251

* revert: remove interface * feat: change interface with nextjs

Login (#29)

ad260bc

* feat: implement login * fix: dashboard redirection

Get data.ts files (#30)

16af540

* feat: get lastdata from database * fix: function if in data.ts

refactor: renamed file type and function for better separation (#31)

d94906a

jpilet commented Jun 18, 2024

View reviewed changes

jordyBSK and others added 14 commits June 19, 2024 09:53

feat: implement logout (#32)

e095c1b

add Esp table (#33)

32d7886

* feat: create table esp * feat: revert table esp * feat: verify table esp

fix: redirect to login when we logout (#34)

5fd26f4

Revert "sort data by room ip (#35)" (#36)

48db262

This reverts commit f12bde6.

Revert "Revert "sort data by room ip (#35)" (#36)" (#37)

00d6c9e

feat: added dynamic data in dashboard and esp pages (#39)

38e1aec

* feat: added dynamic data in dashboard and esp pages * fix: fixed function in map

Date range (#40)

f3ed78b

* feat: retrieves and displays date in datepicker * refactor: refactor and remove unused import * feart: set the default date to the current month * feat: add date parameter in useFetchData function * feat: implement date range * refactor: refactor code

fix: fix import duplication (#41)

8ec2912

fix: change dockerfile to work with next js

2be57e8

refactor: added new Ip adresse

4a1417d

refactor: removed output and added distdir to out

3db6d23

refactor: reformated file

0d16c0f

jordyBSK and others added 30 commits July 5, 2024 14:34

feat: added gap on add user input (#93)

4ec1499

fix: fixed jwt token fetch getting no Ip (#94)

79b8d1b

* fix: fixed jwt token fetch getting no Ip * style: reformated files

feat: remove reload on move esp (#96)

d8a64f5

feat: remove the choice of precision according to the days chosen (#95)

d9a1758

refactor: added verification of IP input with js (#99)

313a2a9

* refactor: added verification of ip in js * style: reformated file * refactor: simplified regex (shorter) * style: reformated file * refactor: compacted regex

docs: update readme (#101)

e591139

feat: Seperate days by a bar and added time stamp of data on graph (#103

ed38067

) * refactor: added timestamp on graph hover * refactor: added import react from reacr * feat: added bar for every new day * style: reformated file and removed unused import

fix: allow esp to know who they are (#106)

4a32441

Co-authored-by: Alex-zReeZ <145986307+Alex-zReeZ@users.noreply.github.com>

fix: removed hardcoded localhost link (#107)

f95ef11

refactor: added timestamp in graph xAxis (#108)

4697933

* refactor: removed console log * refactor: added hour in graph xAxis

refactor: removed readme in nextjs interface (#110)

bab4e08

fix: fix warning "list should have unique key" (#118)

26ed858

feat: added zooming interface in plan (#121)

01c45dc

* feat: added zooming interface in plan closes: #115 * style: reformated code * style: reformated code

feat: show the day when clicking on it (#122)

04cba05

* feat: show the day when clicking on it closes: #116 * style: format

Fix espid link when clicking on day (#123)

c5c372f

* feat: correct espId link when clicking on the day, for each esp * style: reformat

docs: add database and migrations documentation (#125)

c451411

refactor(database): changed sqitch dockerfile to only docker compose No more need of the dedicated docker file. Migrations mounted in the container directly from docker compose. Closes: #127

ci: setup dependabot for automatic versions updates (#128)

558a92a

build: refactor dockerfiles name to avoid file type mismatches (#134)

8c667f1

Closes: #134

build(deps-dev): bump typescript in /nextjs-interface (#140)

0e4ecff

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Global code review #9

Global code review #9

jpilet commented Jun 4, 2024

jpilet Jun 4, 2024

jpilet Jun 4, 2024

jpilet Jun 18, 2024

jpilet Jun 18, 2024

Rignchen Jun 21, 2024

Rignchen Jun 21, 2024

jpilet Jun 24, 2024

jpilet Jun 18, 2024

jpilet Jun 18, 2024

jpilet Jun 18, 2024

jpilet Jun 18, 2024

Global code review #9

Are you sure you want to change the base?

Global code review #9

Conversation

jpilet commented Jun 4, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment