a24danwe commented

Collaborator

Issue #339

Added function to retrieve all sensors in tempService
Added TemperatureAllView.js
Added useTemperatureAll.js
Added TemperatureAllView to the dropdown in Dashboard.js

Issue #339 Added function to retrieve all sensors in tempService Added TemperatureAllView.js Added useTemperatureAll.js Added TemperatureAllView to the dropdown in Dashboard.js

a24danwe added 4 commits

2026-04-17 07:23:12 +00:00

Add latest temperature for all sensors endpoint 2327e056b9

Added useTemperatureAll.js 98f2c6f203

Added TemperatureAllView.js 8cd5a11efd

Added TemperatureAllView to Dashboard.js 05b89879a0

a24danwe commented

2026-04-17 07:24:03 +00:00

Author

Collaborator

Waiting for c24elipe to review

c24elipe requested review from c24elipe

2026-04-17 08:46:23 +00:00

a24danwe referenced this pull request

2026-04-17 08:59:26 +00:00

BIG card for showing all temperature sensors #339

a24danwe was assigned by c24elipe

2026-04-17 09:14:33 +00:00

c24elipe commented

2026-04-17 10:43:14 +00:00

Collaborator

Small problems regarding formatting in service/tempService/Program.cs.

c24elipe commented

2026-04-17 10:43:32 +00:00

Collaborator

Formal review comes later, can't do one now

c24elipe commented

2026-04-17 14:41:19 +00:00

Collaborator

Review on #385

Tested on CahyOS & Linux Mint using Firefox

Running the application:
- ERRORS: An error that was found was in relation to a 505 error when trying to fetch the data from the back-end. However, this issue was found to be the result of running this application with the Brave browser or some ad-blocking plugins on Firefox. Once fixed, all such errors disappeared.
- Tested: This application was tested by first running it normally and seeing that all Docker containers start as they should. After they were found to indeed work correctly, the Firefox browser was opened to the frontend container, and from the dropdown, the relevant "TemperatureAllView" was added correctly. I also then tried to format the relevant files using Prettier and noted the file changes using git. (red being the current version)

Inspected the following files

OBS: No new dependencies from outside of this repo were found or added.

frontend/src/features/temperature/TemperatureAllView.js
- The indentation and style standards are almost completely followed by the Prettier-enforced formatting. The only formatting not allowed was around the import of "useTemperatureAll" at the top of the file.
- The Naming convention was followed.
- No comments are present. But I, as the review belive that the code is self-documenting enough to get away with it.
- The code quality is acceptable. Not too hard to understand, and it does what it needs to.
- A general issue I found with all Temperature components was that they include CSS in their JS files, but since all others do it, and it seems to be the standard, I deem it okay.
frontend/src/components/Dashboard/Dashboard.js:
- All that was done in this file was the import and addition of the new TemperatureAllView to the dropdown list for the frontend.
Since this is such a small change, with no formatting errors, that seems to be copied and pasted from other code, is that all I will write on the matter?
frontend/src/features/temperature/useTemperatureAll.js:
- In this file, the standard sharedFeatures.js code is used as intended without any indentation or usage errors.
service/tempService/Program.cs:
- A new endpoint was added here from lines 78->107.
- The indentation and style standard is again almost correct. The first error shown is that the whole function mapping is a little weirdly formatted. The main offender is "app.MapGet", that is for more space to the right than it should be to follow the standards set.
  
  Besides, were there errors, no other style standard blunders were found.
- To follow the Naming convention, the variable "sql" (found on line 81) is named incorrectly all over the file. The correct name should be "Sql" or "SQL" ( depending on preference ) due to the code standard stating "- Use PascalCase for constant names, both fields and local constants."
- The code quality is good, adequate, and is on par with the rest of the file.
- Comments are placed correctly and only where they are needed, on more complex logic.

What needs to be done

What needs to be done is a simple formatting on all relevant files, and if the "sql" naming convention is deemed to be correct, then a small refactor of the variable in service/tempService/Program.cs needs to be refactored.

Conclusion

Besides small flaws, this code looks good, and if fixed, can be merged.

# Review on #385 ## Tested on CahyOS & Linux Mint using Firefox - Running the application: + **ERRORS**: An error that was found was in relation to a 505 error when trying to fetch the data from the back-end. However, this issue was found to be the result of running this application with the Brave browser or some ad-blocking plugins on Firefox. Once fixed, all such errors disappeared. + **Tested**: This application was tested by first running it normally and seeing that all Docker containers start as they should. After they were found to indeed work correctly, the Firefox browser was opened to the frontend container, and from the dropdown, the relevant "TemperatureAllView" was added correctly. I also then tried to format the relevant files using Prettier and noted the file changes using git. (red being the current version) ![image](/attachments/ae1d0ec3-edc8-4e4d-a50e-8e8433602c0f) ## Inspected the following files OBS: No new dependencies from outside of this repo were found or added. - frontend/src/features/temperature/**TemperatureAllView**.js - The **indentation and style standards** are almost completely followed by the Prettier-enforced formatting. The only formatting not allowed was around the import of "useTemperatureAll" at the top of the file. - The **Naming convention** was followed. - No **comments** are present. But I, as the review belive that the code is self-documenting enough to get away with it. - The **code quality** is acceptable. Not too hard to understand, and it does what it needs to. - A **general issue** I found with all Temperature components was that they include CSS in their JS files, but since all others do it, and it seems to be the standard, I deem it okay. - frontend/src/components/Dashboard/**Dashboard**.js: - All that was done in this file was the import and addition of the new TemperatureAllView to the dropdown list for the frontend. Since this is such a small change, with no formatting errors, that seems to be copied and pasted from other code, is that all I will write on the matter? - frontend/src/features/temperature/**useTemperatureAll**.js: - In this file, the standard sharedFeatures.js code is used as intended without any indentation or usage errors. - service/tempService/Program.cs: - A new endpoint was added here from lines 78->107. - The **indentation and style standard** is again almost correct. The first error shown is that the whole function mapping is a little weirdly formatted. The main offender is "app.MapGet", that is for more space to the right than it should be to follow the standards set. ![image](/attachments/90f34980-62ba-4b77-8bf8-73bb972395c1) Besides, were there errors, no other style standard blunders were found. - To follow the **Naming convention**, the variable "sql" (found on line 81) is named incorrectly all over the file. The correct name should be "Sql" or "SQL" ( depending on preference ) due to the code standard stating "- Use PascalCase for constant names, both fields and local constants." - The **code quality** is good, adequate, and is on par with the rest of the file. - **Comments** are placed correctly and only where they are needed, on more complex logic. ## What needs to be done What needs to be done is a simple formatting on all relevant files, and if the "sql" naming convention is deemed to be correct, then a small refactor of the variable in service/tempService/Program.cs needs to be refactored. ## Conclusion Besides small flaws, this code looks good, and if fixed, can be merged.

image.png

18 KiB

image.png

9.1 KiB

a24danwe added 1 commit

2026-04-17 14:56:03 +00:00

fixed formatting on TemperatureView.js ed2a57bcb2

a24danwe added 1 commit

2026-04-17 15:03:37 +00:00

Fixed formatting on tempService/Program.cs 5dc72e4985

a24danwe added 1 commit

2026-04-17 15:06:44 +00:00

Fixed naming convention. sql -> Sql 4b3442c8ec

a24danwe commented

2026-04-17 15:19:18 +00:00

Author

Collaborator

Thank you for the review!

I think I have fixed the formatting issues and also the naming convention of sql -> Sql.

Also the reason for some code being copied in "Since this is such a small change, with no formatting errors, that seems to be copied and pasted from other code, is that all I will write on the matter?" was because I had to redo the branch and so I copied all my code and pasted it in, that was maybe also the reason why the indentation was wrong in some areas.

Thank you for the review! I think I have fixed the formatting issues and also the naming convention of sql -> Sql. Also the reason for some code being copied in "Since this is such a small change, with no formatting errors, that seems to be copied and pasted from other code, is that all I will write on the matter?" was because I had to redo the branch and so I copied all my code and pasted it in, that was maybe also the reason why the indentation was wrong in some areas.