feat: 2025-11-21 improvements #6

Merged

Pertempto merged 5 commits from 2025-11-21-improvements into main

2025-11-22 09:28:48 -05:00

Pertempto commented

2025-11-22 05:25:10 -05:00

(Migrated from github.com)

No description provided.

github-actions[bot] commented

2025-11-22 05:26:51 -05:00

(Migrated from github.com)

backend/internal/server/invoices.go — rows.Scan uses concrete types (string, float64) which will panic or error if DB returns NULLs or different types. Please use sql.NullString / sql.NullFloat64 (or validate/COALESCE in SQL) and map them to the response struct, e.g. handle NULL Sales Rep or Tran Subtotal values. Also confirm the actual DB column types for "Sales Rep" and "Tran Subtotal" and adjust scanning types accordingly.

`backend/internal/server/invoices.go` — rows.Scan uses concrete types (`string`, `float64`) which will panic or error if DB returns NULLs or different types. Please use `sql.NullString` / `sql.NullFloat64` (or validate/COALESCE in SQL) and map them to the response struct, e.g. handle NULL `Sales Rep` or `Tran Subtotal` values. Also confirm the actual DB column types for `"Sales Rep"` and `"Tran Subtotal"` and adjust scanning types accordingly.

github-actions[bot] commented

2025-11-22 05:27:32 -05:00

(Migrated from github.com)

backend/internal/database/database.go — The connection strings now include encrypt=%s. For SQL Server older versions, allowed values are disable, true, false. Ensure DB_ENCRYPT env var is validated/limited to these values. Suggest adding validation in config.Load() to return an error if an invalid value is supplied (to avoid runtime connection errors).

`backend/internal/database/database.go` — The connection strings now include `encrypt=%s`. For SQL Server older versions, allowed values are `disable`, `true`, `false`. Ensure `DB_ENCRYPT` env var is validated/limited to these values. Suggest adding validation in `config.Load()` to return an error if an invalid value is supplied (to avoid runtime connection errors).

github-actions[bot] commented

2025-11-22 05:27:44 -05:00

(Migrated from github.com)

Changes Requested

Validate DB_ENCRYPT in config.Load() to ensure it is one of "disable", "true", or "false" (fail fast with a clear error).
Make DB row scanning robust to NULLs/types in backend/internal/server/invoices.go and backend/internal/server/employees.go (use sql.NullString, sql.NullFloat64 or COALESCE in SQL).
Confirm Employees filter change from DateReleased IS NULL to inactive = 0 is intentional and consistent with Aptora schema.

Summary of Changes

Added DB_ENCRYPT environment variable to /.env.example and integrated it into the configuration and DB connection string.
Updated .gitignore to ignore .env.* files but keep .env.example tracked.
Propagated DBEncrypt setting from config through main into the database manager and included encrypt=%s in SQL Server connection strings.
Split handleEmployees and handleInvoices out of server.go into separate handler files and refactored invoice handling to use the aptCDV_VW_APT_InvSalCredEstList view with new column names and pagination/limit checks.
Updated frontend InvoicesPage.tsx to use number/subtotal/employee query parameter names matching backend changes.

Overall Feedback

Negative: There are a few risky assumptions that may cause runtime errors or incorrect data: unvalidated DB_ENCRYPT values, and direct scanning into concrete Go types from DB columns that may be NULL or have different types. The Employees SQL filter was changed in a way that could alter results if the inactive column doesn't exist or behaves differently.
Positive: The refactor cleanly extracts handlers into separate files, adds safeguards (invoice count limit), and wires TLS/ENCRYPT handling through configuration consistently. Frontend updates align with backend field name changes. Nice work simplifying and organizing handlers! 🎉

If you want, I can provide patch suggestions for the sql.Null* scan changes and DB_ENCRYPT validation.

# Changes Requested - Validate `DB_ENCRYPT` in `config.Load()` to ensure it is one of `"disable"`, `"true"`, or `"false"` (fail fast with a clear error). - Make DB row scanning robust to NULLs/types in `backend/internal/server/invoices.go` and `backend/internal/server/employees.go` (use `sql.NullString`, `sql.NullFloat64` or COALESCE in SQL). - Confirm `Employees` filter change from `DateReleased IS NULL` to `inactive = 0` is intentional and consistent with Aptora schema. ## Summary of Changes - Added `DB_ENCRYPT` environment variable to `/.env.example` and integrated it into the configuration and DB connection string. - Updated `.gitignore` to ignore `.env.*` files but keep `.env.example` tracked. - Propagated `DBEncrypt` setting from `config` through `main` into the `database` manager and included `encrypt=%s` in SQL Server connection strings. - Split `handleEmployees` and `handleInvoices` out of `server.go` into separate handler files and refactored invoice handling to use the `aptCDV_VW_APT_InvSalCredEstList` view with new column names and pagination/limit checks. - Updated frontend `InvoicesPage.tsx` to use `number`/`subtotal`/`employee` query parameter names matching backend changes. ## Overall Feedback - Negative: There are a few risky assumptions that may cause runtime errors or incorrect data: unvalidated `DB_ENCRYPT` values, and direct scanning into concrete Go types from DB columns that may be NULL or have different types. The `Employees` SQL filter was changed in a way that could alter results if the `inactive` column doesn't exist or behaves differently. - Positive: The refactor cleanly extracts handlers into separate files, adds safeguards (invoice count limit), and wires TLS/ENCRYPT handling through configuration consistently. Frontend updates align with backend field name changes. Nice work simplifying and organizing handlers! 🎉 If you want, I can provide patch suggestions for the `sql.Null*` scan changes and `DB_ENCRYPT` validation.