Pr check aggregate

[dedalus2000]

User description

La query con selection(_aggregateRows=True) sembra fallire in caso di aggregazione di campi testo vuoti: invece che None torna la stringa vuota ''

In questo branch creo la colonna "colonna_testo" in testata fattura e poi eseguo una aggregazione dalla tabella dei clienti

Query di verifica e popolamento database sono tra gli script di upgrades

---

PR Type

enhancement, tests

---

Description

• Added test scripts to verify aggregation behavior on empty text fields.

  • Script 001_insertmario.py creates a test client and related invoices.
  • Script 002_read_aggregate.py runs queries to check aggregation results.

• Introduced new column colonna_testo to fattura table.

• Demonstrated issue where aggregation returns '' instead of None for empty text.

---

Changes walkthrough 📝

	Relevant files
Enhancement	fattura.py Add `colonna_testo` column to `fattura` table                        packages/fatt/model/fattura.py Added new column colonna_testo (text, size 20) to fattura table. Minor formatting adjustment. +2/-1
Tests	001_insertmario.py Add script to insert test client and invoices                        packages/fatt/lib/upgrades/001_insertmario.py New script to insert a test client (Mario Rossi Srl). Inserts 10 invoices for the new client with colonna_testo as None. +20/-0    002_read_aggregate.py Add script to test aggregation of empty text fields            packages/fatt/lib/upgrades/002_read_aggregate.py New script to run and print results of various queries on colonna_testo. Tests and demonstrates aggregation behavior for empty text fields. Provides diagnostic output for debugging aggregation logic. +41/-0

---

Need help?

Type /help how to ... in the comments thread for any questions about Qodo Merge usage.

Check out the documentation for more information.

[qodo-code-review]

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪

🧪 PR contains tests

🔒 No security concerns identified

⚡ Recommended focus areas for review Column Definition The new column 'colonna_testo' is added without name_long property and with an unusual size format (':20'). Consider adding a descriptive name_long and verifying the size format. tbl.column('colonna_testo', size=":20") Commented Code There's a commented line that appears to be a check for existing records before insertion. Consider either removing it or implementing it to prevent duplicate records on multiple runs. #if tbl_cliente.query(where='$ragione_sociale=:nn', nn=name).count()==0:

[qodo-code-review]

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Impact
Possible issue	Prevent duplicate client creation Uncomment the check for existing client to prevent duplicate insertions. Without this check, the script will create duplicate clients with the same name each time it runs, potentially causing data integrity issues. packages/fatt/lib/upgrades/001_insertmario.py [11-15] -#if tbl_cliente.query(where='$ragione_sociale=:nn', nn=name).count()==0: +if tbl_cliente.query(where='$ragione_sociale=:nn', nn=name).count()==0: + # inserisco il cliente + cliente_record = tbl_cliente.newrecord(ragione_sociale=name) + tbl_cliente.insert(cliente_record) -# inserisco il cliente -cliente_record = tbl_cliente.newrecord(ragione_sociale=name) -tbl_cliente.insert(cliente_record) - Apply / Chat Suggestion importance[1-10]: 6 __ Why: The suggestion correctly identifies a potential data integrity issue where duplicate clients could be created on repeated script runs. However, since this appears to be a test/upgrade script, the behavior might be intentional.	Low
General	Close database connection properly Add proper cleanup by closing the database connection after committing. Without proper cleanup, database connections might remain open, potentially causing resource leaks. packages/fatt/lib/upgrades/002_read_aggregate.py [36-41] if __name__ == '__main__': gnrapp = GnrApp('sandbox') db = gnrapp.db main(db) db.commit() + db.closeConnection() Apply / Chat Suggestion importance[1-10]: 4 __ Why: While proper resource cleanup is good practice, this is a minor improvement for a script context where the application framework likely handles connection cleanup automatically when the script terminates.	Low
More