-
Notifications
You must be signed in to change notification settings - Fork 18
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: add multi-query support #2644
base: master
Are you sure you want to change the base?
Conversation
tomwwinter
commented
Oct 30, 2024
•
edited
Loading
edited
Deployed to https://pr-2644.aam-digital.net/ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code looks very good 👍
Is this deployed for testing already?
* {groupTitle: "This is a group", items: [...]} | ||
* | ||
*/ | ||
@DatabaseField() reportDefinition: ReportDefinitionDto[]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe better "reportDefinitions" as this is an array?
* (sql v2 only) transformations that are applied to input variables (e.g. startDate, endDate) | ||
* example: {startDate: ["SQL_FROM_DATE"], endDate: ["SQL_TO_DATE"]} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
* (sql v2 only) transformations that are applied to input variables (e.g. startDate, endDate) | |
* example: {startDate: ["SQL_FROM_DATE"], endDate: ["SQL_TO_DATE"]} | |
* (sql v2 only) transformations that are applied to input variables (e.g. startDate, endDate) | |
* the keys can be used as placeholders (`$key`; e.g. `WHERE c.date BETWEEN $startDate AND $endDate`) in SQL queries | |
* example: {startDate: ["SQL_FROM_DATE"], endDate: ["SQL_TO_DATE"]} |
Is this the correct format or ${key}?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's just $key. e.g.
SELECT * FROM WHERE c.date BETWEEN $startDate AND $endDate
src/app/features/reporting/reporting/select-report/select-report.component.ts
Outdated
Show resolved
Hide resolved
src/app/features/reporting/reporting/sql-v2-table/sql-v2-table.component.html
Outdated
Show resolved
Hide resolved
src/app/features/reporting/reporting/sql-v2-table/sql-v2-table.component.ts
Outdated
Show resolved
Hide resolved
try { | ||
return this.sqlReportService.flattenData(value); | ||
} catch (error) { | ||
console.log(error); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
use Logging
system?
* [ | ||
* [ | ||
* { | ||
* "Patenschaften neu verknüpft": 6 | ||
* } | ||
* ], | ||
* [ | ||
* { | ||
* "Patenschaften Aktiv": 0 | ||
* } | ||
* ], | ||
* [ | ||
* { | ||
* "Mentees neu aufgenommen": 2 | ||
* } | ||
* ], | ||
* { | ||
* "Mentoren erreicht": [ | ||
* [ | ||
* { | ||
* "junge Erwachsene 20 bis 29 Jahren": 4 | ||
* } | ||
* ], | ||
* [ | ||
* { | ||
* "Erwachsene 30 bis 49 Jahren": 4 | ||
* } | ||
* ] | ||
* ] | ||
* }, | ||
* [ | ||
* { | ||
* "anzahl": 3, | ||
* "status": "BEENDET", | ||
* "foerderungDurch": null | ||
* }, | ||
* { | ||
* "anzahl": 1, | ||
* "status": "MATCHING_LÄUFT", | ||
* "foerderungDurch": null | ||
* }, | ||
* { | ||
* "anzahl": 1, | ||
* "status": "MATCHING_LÄUFT", | ||
* "foerderungDurch": "Menschen stärken Menschen" | ||
* }, | ||
* { | ||
* "anzahl": 1, | ||
* "status": "PATENSCHAFT", | ||
* "foerderungDurch": null | ||
* } | ||
* ] | ||
* ] | ||
* |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe we can define a slightly shorter and English example for the code docs here?
* @param data raw ReportCalculationData or sub-array | ||
* @param level each level will add some left-padding to visualize hierarchy | ||
*/ | ||
flattenData(data: any[], level = 0): SqlReportRow[] { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
strictly speaking we should add some unit tests for such methods ...
|
||
let csv = ""; | ||
|
||
csv += "Name" + ",".repeat(deepestLevel + 1) + "Value\n"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
any particular reason why we don't use PapaParse library for this?
Are we sure values are always escaped properly to not break CSV formating / delimiters?