feat(TargetUnit): Add DepTech hours calculation and Redmine custom field IDs #71
Conversation
- Added `REPORT_FILTER_FIELD_ID` and `RELATED_PROJECT_FIELD_ID` constants in `weeklyFinancialReport.ts` for better report filtering and linking issues to billable projects. - Refactored `TARGET_UNITS_QUERY` in `queries.ts` to utilize the new constants, improving the SQL query structure and ensuring accurate data retrieval based on custom fields. These changes enhance the flexibility and accuracy of the weekly financial report generation process.
📝 Walkthrough""" WalkthroughTwo new Redmine custom field ID constants were added to the weekly financial report configuration. The Changes
Sequence Diagram(s)sequenceDiagram
participant Config as weeklyFinancialReport.ts
participant QueryBuilder as queries.ts
participant DB as Database
Config->>QueryBuilder: Provide REPORT_FILTER_FIELD_ID and RELATED_PROJECT_FIELD_ID
QueryBuilder->>QueryBuilder: Build COMMON_SELECT, COMMON_FROM, COMMON_WHERE
QueryBuilder->>QueryBuilder: Build direct and related project subqueries
QueryBuilder->>DB: Execute UNION ALL of subqueries with dynamic group filtering
DB-->>QueryBuilder: Return aggregated results (project_hours, deptech_hours, filter_name, etc.)
Possibly related PRs
Suggested reviewers
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (1)
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
🔍 Vulnerabilities of
|
| digest | sha256:757364bc4a51d05b64b32fdd7c0825e4c91e3efe75a9c278ca3c2d508f7efe92 |
| vulnerabilities |     |
| platform | linux/amd64 |
| size | 243 MB |
| packages | 1628 |
📦 Base Image node:20-alpine
| also known as |
|
| digest | sha256:37a5a350292926f98d48de9af160b0a3f7fcb141566117ee452742739500a5bd |
| vulnerabilities |    |
Description
Description
| ||||||||||||||||
Description
| ||||||||||||||||
Description
| ||||||||||||||||
Description
| ||||||||||||||||
Description
| ||||||||||||||||
Description
| ||||||||||||||||
Description
| ||||||||||||||||
Description
|
- Updated the SQL query in `queries.ts` to replace the `custom_values` alias from `cv2` to `icv`, improving readability and consistency in the code. - Ensured that the join conditions remain intact while enhancing the clarity of the query structure. These changes contribute to better maintainability and understanding of the SQL logic within the TargetUnit service.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
workers/main/src/configs/weeklyFinancialReport.ts (1)
9-13: Consider more descriptive constant names for better clarity.The constants work well, but their names could be more specific to indicate they are Redmine custom field IDs.
Consider renaming for clarity:
-export const REPORT_FILTER_FIELD_ID = 253; +export const REDMINE_REPORT_FILTER_CUSTOM_FIELD_ID = 253; -export const RELATED_PROJECT_FIELD_ID = 20; +export const REDMINE_RELATED_PROJECT_CUSTOM_FIELD_ID = 20;workers/main/src/services/TargetUnit/queries.ts (2)
39-65: Well-structured helper functions for query composition.The separation of direct and related project time entries into distinct functions improves code clarity and maintainability.
Consider adding JSDoc comments to document the purpose and return value of each function:
/** * Builds SQL query for time entries directly linked to projects * @returns SQL query fragment selecting project hours */ function buildDirectGroupTimeEntries() {
67-98: Query structure looks good, but consider performance implications.The refactored query with UNION ALL and aggregation is well-structured. However, there are a few considerations:
- For large datasets, the UNION ALL approach might impact performance compared to a single query with conditional logic.
- There's an inconsistency with date formatting -
spent_onis formatted in the SELECT but grouped by the raw value.Consider applying DATE_FORMAT consistently:
GROUP BY t.group_id, t.group_name, t.project_id, t.project_name, t.user_id, t.username, - t.spent_on + DATE_FORMAT(t.spent_on, '%Y-%m-%d')Or move the formatting to the subqueries for consistency.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
workers/main/src/configs/weeklyFinancialReport.ts(1 hunks)workers/main/src/services/TargetUnit/queries.ts(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: anatolyshipitz
PR: speedandfunction/automatization#34
File: workers/main/src/workflows/financial/FinancialReportFormatter.ts:3-7
Timestamp: 2025-05-30T17:57:21.010Z
Learning: User anatolyshipitz prefers to keep code implementations simple during early development stages rather than adding comprehensive error handling and validation. They consider extensive type annotations and error handling "redundant" when focusing on core functionality and testing.
workers/main/src/services/TargetUnit/queries.ts (1)
Learnt from: CR
PR: speedandfunction/automatization#0
File: .cursor/rules/temporal-project-structure.mdc:0-0
Timestamp: 2025-06-24T12:29:48.641Z
Learning: In Temporal-based TypeScript projects, each worker should be structured as an independent package under 'workers/<worker-name>/', with clear separation of workflows, activities, entry point, and documentation to promote modularity and maintainability.
🧬 Code Graph Analysis (1)
workers/main/src/services/TargetUnit/queries.ts (1)
workers/main/src/configs/weeklyFinancialReport.ts (2)
REPORT_FILTER_FIELD_ID(10-10)RELATED_PROJECT_FIELD_ID(13-13)
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: Docker Security Scanning (temporal, Dockerfile.temporal, temporal-test:latest)
- GitHub Check: Docker Security Scanning (n8n, Dockerfile.n8n, n8n-test:latest)
- GitHub Check: Service Availability Check
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (1)
workers/main/src/services/TargetUnit/queries.ts (1)
11-37: Excellent refactoring with modular SQL fragments.Great job extracting common SQL fragments to reduce duplication and improve maintainability. The use of constants for custom field IDs enhances configurability.
- Updated the mapping function in `queries.ts` to escape single quotes in group names, ensuring proper SQL syntax and preventing potential query errors. - This change enhances the robustness of the SQL query generation process within the TargetUnit service, contributing to better data integrity and reliability.
- Adjusted the mapping function in `queries.ts` to ensure consistent formatting of group names by removing unnecessary whitespace. - This change maintains the integrity of the SQL query generation process and enhances code readability.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
workers/main/src/services/TargetUnit/queries.ts (1)
7-9: Security improvement acknowledgedGood to see the SQL injection concern from the previous review has been addressed with proper single quote escaping.
🧹 Nitpick comments (4)
workers/main/src/services/TargetUnit/queries.ts (4)
11-21: Consider adding documentation for the SQL fragmentsThe common SQL fragments are well-structured but could benefit from comments explaining their purpose and the business logic they represent.
Add documentation above each fragment:
+// Common SELECT fields for both direct and related project time entries const COMMON_SELECT = ` SELECT g.id AS group_id,
23-28: Confusing table alias namingThe alias
gfor theuserstable is misleading sincegtypically suggests "groups". While the WHERE clause filters fortype = 'Group', the alias doesn't clearly convey this intent.Consider renaming for clarity:
- FROM users AS g + FROM users AS group_usersAnd update all references accordingly throughout the query fragments.
35-36: Complex date range calculation needs documentationThe date range calculation using
WEEKDAY(CURDATE())is complex and could benefit from a comment explaining the business logic (appears to calculate previous week's date range).Add a comment above the date calculation:
+ -- Calculate previous week's date range (Monday to Sunday) AND te.spent_on BETWEEN DATE_SUB(CURDATE(), INTERVAL WEEKDAY(CURDATE()) + 7 DAY)
51-65: Potential fragility in custom field value matchingThe join condition
icv.value = CAST(p.id AS CHAR)assumes that the custom field value is stored as a string representation of the project ID. This could be fragile if the data format changes or if there are leading/trailing spaces.Consider adding trimming and validation:
- JOIN custom_values icv ON icv.value = CAST(p.id AS CHAR) + JOIN custom_values icv ON TRIM(icv.value) = CAST(p.id AS CHAR)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
workers/main/src/services/TargetUnit/queries.ts(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: anatolyshipitz
PR: speedandfunction/automatization#34
File: workers/main/src/workflows/financial/FinancialReportFormatter.ts:3-7
Timestamp: 2025-05-30T17:57:21.010Z
Learning: User anatolyshipitz prefers to keep code implementations simple during early development stages rather than adding comprehensive error handling and validation. They consider extensive type annotations and error handling "redundant" when focusing on core functionality and testing.
workers/main/src/services/TargetUnit/queries.ts (1)
Learnt from: CR
PR: speedandfunction/automatization#0
File: .cursor/rules/temporal-project-structure.mdc:0-0
Timestamp: 2025-06-24T12:29:48.641Z
Learning: In Temporal-based TypeScript projects, each worker should be structured as an independent package under 'workers/<worker-name>/', with clear separation of workflows, activities, entry point, and documentation to promote modularity and maintainability.
🧬 Code Graph Analysis (1)
workers/main/src/services/TargetUnit/queries.ts (1)
workers/main/src/configs/weeklyFinancialReport.ts (2)
REPORT_FILTER_FIELD_ID(10-10)RELATED_PROJECT_FIELD_ID(13-13)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: Analyze (actions)
🔇 Additional comments (3)
workers/main/src/services/TargetUnit/queries.ts (3)
1-5: LGTM: Clean import structureThe imports are well-organized and appropriately reference the configuration constants.
39-49: LGTM: Clear function structureThe function clearly builds the direct project time entries query with appropriate field mappings.
67-98: Well-structured aggregation queryThe main query effectively unions the two subqueries and provides proper aggregation with clear field separation between project_hours and deptech_hours.
- Updated the SQL query in `queries.ts` to add the `filter_name` field, improving the data retrieval capabilities for target units. - This change enhances the flexibility of the query, allowing for more detailed filtering and reporting of target unit data.
|
These changes enhance the flexibility and accuracy of the weekly financial report generation process.