[WEB-3987] chore: project export funtionality enhancement#7002
[WEB-3987] chore: project export funtionality enhancement#7002sriramveeraghanta merged 12 commits intopreviewfrom
Conversation
|
""" WalkthroughThe export task for issues was refactored to fetch complete model instances with related objects using Django's ORM, manually constructing enriched dictionaries with nested and related data for each issue. Export row generation functions and headers were updated to accommodate the new data structure, and additional related fields were included in all export formats. Changes
Suggested labels
Suggested reviewers
Poem
✨ 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 (
|
sangeethailango
changed the title
sangeethailango
changed the title
sangeethailango
changed the title
|
Pull Request Linked with Plane Work Items
Comment Automatically Generated by Plane |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (5)
apiserver/plane/bgtasks/export_task.py (5)
18-24: Remove unused importsSeveral imported modules are not used in the code, making it harder to understand dependencies and potentially increasing load time.
-from django.db.models import Value -from django.db.models.functions import Coalesce -from django.db.models import Q -from django.contrib.postgres.aggregates import ArrayAgg -from django.db.models import OuterRef, Func, F, Count -from django.db.models.functions import Concat -from django.db import models🧰 Tools
🪛 Ruff (0.8.2)
18-18:
django.db.models.Valueimported but unusedRemove unused import:
django.db.models.Value(F401)
19-19:
django.db.models.functions.Coalesceimported but unusedRemove unused import:
django.db.models.functions.Coalesce(F401)
20-20:
django.db.models.Qimported but unusedRemove unused import:
django.db.models.Q(F401)
21-21:
django.contrib.postgres.aggregates.ArrayAggimported but unusedRemove unused import:
django.contrib.postgres.aggregates.ArrayAgg(F401)
22-22:
django.db.models.OuterRefimported but unusedRemove unused import
(F401)
22-22:
django.db.models.Funcimported but unusedRemove unused import
(F401)
22-22:
django.db.models.Fimported but unusedRemove unused import
(F401)
22-22:
django.db.models.Countimported but unusedRemove unused import
(F401)
23-23:
django.db.models.functions.Concatimported but unusedRemove unused import:
django.db.models.functions.Concat(F401)
24-24:
django.db.modelsimported but unusedRemove unused import:
django.db.models(F401)
162-195: Enhancement to include rich information in exportsThe table row generation has been updated to include additional fields like comments, estimate, links, assignees, and attachment information, providing a more comprehensive export.
However, you should use the
.get()method for all dictionary keys that might not be present to avoid potential KeyError exceptions:- issue["description"], + issue.get("description", ""), - issue["state_name"], + issue.get("state_name", ""),🧰 Tools
🪛 Ruff (0.8.2)
184-184: Line too long (92 > 88)
(E501)
300-306: Remove debug print statementsDebug print statements should be removed before deploying to production.
- print(issue, "Issue from xlsx") row = generate_table_row(issue) - print(row, "Row")
369-369: Long line exceeds recommended lengthThis line exceeds the recommended length of 88 characters.
- "created_by": f"{issue.created_by.first_name} {issue.created_by.last_name}", + "created_by": ( + f"{issue.created_by.first_name} {issue.created_by.last_name}" + ),🧰 Tools
🪛 Ruff (0.8.2)
369-369: Line too long (92 > 88)
(E501)
379-379: Long line exceeds recommended lengthThis line exceeds the recommended length of 88 characters.
- "created_by": f"{comment.created_by.first_name} {comment.created_by.last_name}", + "created_by": ( + f"{comment.created_by.first_name} {comment.created_by.last_name}" + ),🧰 Tools
🪛 Ruff (0.8.2)
379-379: Line too long (104 > 88)
(E501)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apiserver/plane/bgtasks/export_task.py(6 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
apiserver/plane/bgtasks/export_task.py (2)
apiserver/plane/db/models/issue.py (1)
Issue(104-253)apiserver/plane/db/models/asset.py (2)
FileAsset(24-104)EntityTypeContext(29-39)
🪛 Ruff (0.8.2)
apiserver/plane/bgtasks/export_task.py
18-18: django.db.models.Value imported but unused
Remove unused import: django.db.models.Value
(F401)
19-19: django.db.models.functions.Coalesce imported but unused
Remove unused import: django.db.models.functions.Coalesce
(F401)
20-20: django.db.models.Q imported but unused
Remove unused import: django.db.models.Q
(F401)
21-21: django.contrib.postgres.aggregates.ArrayAgg imported but unused
Remove unused import: django.contrib.postgres.aggregates.ArrayAgg
(F401)
22-22: django.db.models.OuterRef imported but unused
Remove unused import
(F401)
22-22: django.db.models.Func imported but unused
Remove unused import
(F401)
22-22: django.db.models.F imported but unused
Remove unused import
(F401)
22-22: django.db.models.Count imported but unused
Remove unused import
(F401)
23-23: django.db.models.functions.Concat imported but unused
Remove unused import: django.db.models.functions.Concat
(F401)
24-24: django.db.models imported but unused
Remove unused import: django.db.models
(F401)
184-184: Line too long (92 > 88)
(E501)
369-369: Line too long (92 > 88)
(E501)
379-379: Line too long (104 > 88)
(E501)
🔇 Additional comments (5)
apiserver/plane/bgtasks/export_task.py (5)
28-28: LGTM: Appropriate model imports addedThe imports have been updated to include
IssueandFileAssetmodels that are now being used in the export functionality.
198-225: Enhanced JSON export with additional fieldsThe JSON row generation now includes comprehensive information about comments, estimates, links, subscribers, and attachments.
318-343: Improved query optimization with select_related and prefetch_relatedUsing
select_relatedandprefetch_relatedis an excellent optimization that reduces the number of database queries needed when fetching related objects.
345-422: LGTM: Comprehensive issue data constructionThe code now creates a rich data structure with all relevant issue information, including related entities like labels, comments, assignees, and attachments.
🧰 Tools
🪛 Ruff (0.8.2)
369-369: Line too long (92 > 88)
(E501)
379-379: Line too long (104 > 88)
(E501)
424-450: Export headers updated to match new data structureHeaders have been appropriately updated to include all the new fields that are being exported.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
apiserver/plane/bgtasks/export_task.py (2)
461-461:⚠️ Potential issueFix filter operation on list object.
There's a critical error in this line.
issues_datais a list, not a queryset, so it doesn't have a.filter()method.- issues = issues_data.filter(project__id=project_id) + issues = [issue for issue in issues_data if issue["project_id"] == project_id]
399-409: 🛠️ Refactor suggestionOptimize duplicate FileAsset queries.
You're making two separate queries to
FileAssetfor the same issue - one for count and one for the URL list. This could be optimized to improve performance.- "attachment_count": FileAsset.objects.filter( - issue_id=issue.id, - entity_type=FileAsset.EntityTypeContext.ISSUE_ATTACHMENT, - ).count(), - "attachment_links": [ - f"/api/assets/v2/workspaces/{issue.workspace.slug}/projects/{issue.project_id}/issues/{issue.id}/attachments/{asset.id}/" - for asset in FileAsset.objects.filter( - issue_id=issue.id, - entity_type=FileAsset.EntityTypeContext.ISSUE_ATTACHMENT, - ) - ], + # Get attachments in a single query + attachments = list(FileAsset.objects.filter( + issue_id=issue.id, + entity_type=FileAsset.EntityTypeContext.ISSUE_ATTACHMENT, + )), + "attachment_count": len(attachments), + "attachment_links": [ + f"/api/assets/v2/workspaces/{issue.workspace.slug}/projects/{issue.project_id}/issues/{issue.id}/attachments/{asset.id}/" + for asset in attachments + ],
🧹 Nitpick comments (3)
apiserver/plane/bgtasks/export_task.py (3)
18-24: Remove unused imports.Several imported modules are not used in the codebase according to static analysis:
Valuefromdjango.db.modelsCoalescefromdjango.db.models.functionsQfromdjango.db.modelsArrayAggfromdjango.contrib.postgres.aggregatesOuterRef,Func,F,Countfromdjango.db.modelsConcatfromdjango.db.models.functionsmodelsfromdjango.db-from django.db.models import Value -from django.db.models.functions import Coalesce -from django.db.models import Q -from django.contrib.postgres.aggregates import ArrayAgg -from django.db.models import OuterRef, Func, F, Count -from django.db.models.functions import Concat -from django.db import models🧰 Tools
🪛 Ruff (0.8.2)
18-18:
django.db.models.Valueimported but unusedRemove unused import:
django.db.models.Value(F401)
19-19:
django.db.models.functions.Coalesceimported but unusedRemove unused import:
django.db.models.functions.Coalesce(F401)
20-20:
django.db.models.Qimported but unusedRemove unused import:
django.db.models.Q(F401)
21-21:
django.contrib.postgres.aggregates.ArrayAggimported but unusedRemove unused import:
django.contrib.postgres.aggregates.ArrayAgg(F401)
22-22:
django.db.models.OuterRefimported but unusedRemove unused import
(F401)
22-22:
django.db.models.Funcimported but unusedRemove unused import
(F401)
22-22:
django.db.models.Fimported but unusedRemove unused import
(F401)
22-22:
django.db.models.Countimported but unusedRemove unused import
(F401)
23-23:
django.db.models.functions.Concatimported but unusedRemove unused import:
django.db.models.functions.Concat(F401)
24-24:
django.db.modelsimported but unusedRemove unused import:
django.db.models(F401)
368-368: Line length exceeds style guidelines.These lines exceed the recommended maximum length of 88 characters. Consider breaking them into multiple lines for better readability.
- "created_by": f"{issue.created_by.first_name} {issue.created_by.last_name}", + "created_by": ( + f"{issue.created_by.first_name} {issue.created_by.last_name}" + ), - "created_by": f"{comment.created_by.first_name} {comment.created_by.last_name}", + "created_by": ( + f"{comment.created_by.first_name} {comment.created_by.last_name}" + ),Also applies to: 378-378
🧰 Tools
🪛 Ruff (0.8.2)
368-368: Line too long (92 > 88)
(E501)
374-383: Consider prefetching comment related data.Instead of iterating through comments with
.all().distinct()within the loop, it would be more efficient to optimize the prefetch operation to include comment authors..prefetch_related( "labels", "issue_cycle__cycle", "issue_module__module", - "issue_comments", + models.Prefetch( + "issue_comments", + queryset=IssueComment.objects.select_related("created_by").distinct() + ), "assignees", "issue_subscribers", "issue_link", )🧰 Tools
🪛 Ruff (0.8.2)
378-378: Line too long (104 > 88)
(E501)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apiserver/plane/bgtasks/export_task.py(6 hunks)
🧰 Additional context used
🪛 Ruff (0.8.2)
apiserver/plane/bgtasks/export_task.py
18-18: django.db.models.Value imported but unused
Remove unused import: django.db.models.Value
(F401)
19-19: django.db.models.functions.Coalesce imported but unused
Remove unused import: django.db.models.functions.Coalesce
(F401)
20-20: django.db.models.Q imported but unused
Remove unused import: django.db.models.Q
(F401)
21-21: django.contrib.postgres.aggregates.ArrayAgg imported but unused
Remove unused import: django.contrib.postgres.aggregates.ArrayAgg
(F401)
22-22: django.db.models.OuterRef imported but unused
Remove unused import
(F401)
22-22: django.db.models.Func imported but unused
Remove unused import
(F401)
22-22: django.db.models.F imported but unused
Remove unused import
(F401)
22-22: django.db.models.Count imported but unused
Remove unused import
(F401)
23-23: django.db.models.functions.Concat imported but unused
Remove unused import: django.db.models.functions.Concat
(F401)
24-24: django.db.models imported but unused
Remove unused import: django.db.models
(F401)
184-184: Line too long (92 > 88)
(E501)
368-368: Line too long (92 > 88)
(E501)
378-378: Line too long (104 > 88)
(E501)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Analyze (javascript)
- GitHub Check: Analyze (python)
🔇 Additional comments (4)
apiserver/plane/bgtasks/export_task.py (4)
346-422: Well structured data enrichment with related objects.The refactoring to use
select_relatedandprefetch_relatedto fetch complete model instances and build enriched dictionaries is a good approach for including comprehensive data in exports. This provides more flexibility in handling related data compared to using.values().🧰 Tools
🪛 Ruff (0.8.2)
368-368: Line too long (92 > 88)
(E501)
378-378: Line too long (104 > 88)
(E501)
443-449: CSV header extended with additional fields.Good job extending the header to include the new fields you're exporting. This will ensure all data appears correctly in the export.
182-192: Good implementation of complex comment formatting.The string formatting for the comments is well implemented, including the comment text, creation date, and author information.
🧰 Tools
🪛 Ruff (0.8.2)
184-184: Line too long (92 > 88)
(E501)
469-469: Non-multiple export now uses enriched issues_data.The implementation correctly uses the enriched
issues_datalist for the single export case, ensuring consistent data structure across all export formats.
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
apiserver/plane/bgtasks/export_task.py (1)
399-409: 🛠️ Refactor suggestionOptimize attachment queries to avoid duplicating database calls
You're making two separate queries to
FileAssetfor the same issue - one for count and one for the URL list. This could be optimized by fetching the attachments once and reusing them.- "attachment_count": FileAsset.objects.filter( - issue_id=issue.id, - entity_type=FileAsset.EntityTypeContext.ISSUE_ATTACHMENT, - ).count(), - "attachment_links": [ - f"/api/assets/v2/workspaces/{issue.workspace.slug}/projects/{issue.project_id}/issues/{issue.id}/attachments/{asset.id}/" - for asset in FileAsset.objects.filter( - issue_id=issue.id, - entity_type=FileAsset.EntityTypeContext.ISSUE_ATTACHMENT, - ) - ], + # Get attachments in a single query + attachments = list(FileAsset.objects.filter( + issue_id=issue.id, + entity_type=FileAsset.EntityTypeContext.ISSUE_ATTACHMENT, + )), + "attachment_count": len(attachments), + "attachment_links": [ + f"/api/assets/v2/workspaces/{issue.workspace.slug}/projects/{issue.project_id}/issues/{issue.id}/attachments/{asset.id}/" + for asset in attachments + ],
🧹 Nitpick comments (2)
apiserver/plane/bgtasks/export_task.py (2)
184-184: Line exceeds recommended length limitThis line and a few others exceed the 88 character limit typically recommended for Python code.
Consider breaking up the long string formatting into multiple lines:
- f"{comment['comment']} ({comment['created_at']} by {comment['created_by']})" + (f"{comment['comment']} " + f"({comment['created_at']} by {comment['created_by']})")🧰 Tools
🪛 Ruff (0.8.2)
184-184: Line too long (92 > 88)
(E501)
183-189: Consider adding comment formatting options for better readabilityThe current implementation concatenates all comments into a single string, which might become unwieldy for issues with many comments.
Consider limiting the number of comments or their length in exports:
", ".join( [ f"{comment['comment']} ({comment['created_at']} by {comment['created_by']})" for comment in issue["comments"] + # Limit to the first 3 comments or similar + [:3] ] ) if issue["comments"] else "",🧰 Tools
🪛 Ruff (0.8.2)
184-184: Line too long (92 > 88)
(E501)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apiserver/plane/bgtasks/export_task.py(6 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
apiserver/plane/bgtasks/export_task.py (2)
apiserver/plane/db/models/issue.py (1)
Issue(104-253)apiserver/plane/db/models/asset.py (2)
FileAsset(24-104)EntityTypeContext(29-39)
🪛 Ruff (0.8.2)
apiserver/plane/bgtasks/export_task.py
18-18: django.db.models.Value imported but unused
Remove unused import: django.db.models.Value
(F401)
19-19: django.db.models.functions.Coalesce imported but unused
Remove unused import: django.db.models.functions.Coalesce
(F401)
20-20: django.db.models.Q imported but unused
Remove unused import: django.db.models.Q
(F401)
21-21: django.contrib.postgres.aggregates.ArrayAgg imported but unused
Remove unused import: django.contrib.postgres.aggregates.ArrayAgg
(F401)
22-22: django.db.models.OuterRef imported but unused
Remove unused import
(F401)
22-22: django.db.models.Func imported but unused
Remove unused import
(F401)
22-22: django.db.models.F imported but unused
Remove unused import
(F401)
22-22: django.db.models.Count imported but unused
Remove unused import
(F401)
23-23: django.db.models.functions.Concat imported but unused
Remove unused import: django.db.models.functions.Concat
(F401)
24-24: django.db.models imported but unused
Remove unused import: django.db.models
(F401)
184-184: Line too long (92 > 88)
(E501)
368-368: Line too long (92 > 88)
(E501)
378-378: Line too long (104 > 88)
(E501)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Analyze (javascript)
- GitHub Check: Analyze (python)
🔇 Additional comments (3)
apiserver/plane/bgtasks/export_task.py (3)
325-341: Good use of select_related and prefetch_related for performanceThe use of
select_relatedandprefetch_relatedis a good optimization to reduce database queries by fetching related objects in a minimal number of queries.
344-421: Improved data enrichment for exports with comprehensive issue detailsThe new approach of building enriched issue dictionaries with related data significantly improves the export functionality by including more context in exports (comments, assignees, attachments, etc.).
🧰 Tools
🪛 Ruff (0.8.2)
368-368: Line too long (92 > 88)
(E501)
378-378: Line too long (104 > 88)
(E501)
423-449: Comprehensive header additions for improved exportsThe addition of new fields to the export headers is well-aligned with the enriched data structure, providing more complete and useful exports for users.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
apiserver/plane/bgtasks/export_task.py (2)
413-421:⚠️ Potential issueCycle information is overwritten for issues with multiple cycles.
The current implementation will only retain information from the last cycle in the loop. If an issue can belong to multiple cycles (as suggested by the model relationships), this implementation will discard all but the last cycle's information.
This is the same issue identified in a previous review. Consider changing to accumulate all cycles in a list:
- # Get prefetched cycles and modules - cycles = list(issue.issue_cycle.all()) - - # Update cycle data - for cycle in cycles: - issue_data["cycle_name"] = cycle.cycle.name - issue_data["cycle_start_date"] = dateConverter(cycle.cycle.start_date) - issue_data["cycle_end_date"] = dateConverter(cycle.cycle.end_date) + # Get prefetched cycles + cycles = list(issue.issue_cycle.all()) + + if cycles: + # Either accumulate all cycles in a list + issue_data["cycles"] = [ + { + "cycle_name": cycle.cycle.name, + "cycle_start_date": dateConverter(cycle.cycle.start_date), + "cycle_end_date": dateConverter(cycle.cycle.end_date), + } + for cycle in cycles + ] + # Or just use the first/latest cycle if only one should be shown + latest_cycle = cycles[0] # or use sorted(cycles, key=lambda x: x.cycle.start_date)[-1] for latest + issue_data["cycle_name"] = latest_cycle.cycle.name + issue_data["cycle_start_date"] = dateConverter(latest_cycle.cycle.start_date) + issue_data["cycle_end_date"] = dateConverter(latest_cycle.cycle.end_date)
461-466: 💡 Verification agent🧩 Analysis chain
Changed filtering approach on issues data.
The filtering of issues by project ID now uses a list comprehension instead of a queryset filter, as the data is now a list of dictionaries rather than a queryset.
While this works, it could be more efficient to group the issues by project ID in a single pass, especially for large datasets:
🏁 Script executed:
#!/bin/bash # Check if there are often many projects and issues in the system echo "Checking number of projects and issues in the system..." rg -n "class Project" -A 3 -B 3 rg -n "Issue.objects" | wc -lLength of output: 42624
🏁 Script executed:
#!/bin/bash # Show context around issues_data usage in export_task.py rg -n 'issues_data' -A 5 -B 5 apiserver/plane/bgtasks/export_task.pyLength of output: 1684
🏁 Script executed:
#!/bin/bash # Locate assignment of project_ids in export_task.py rg -n 'project_ids' -A 3 -B 3 apiserver/plane/bgtasks/export_task.pyLength of output: 926
Group issues by project in one pass for better performance
Instead of filtering the full
issues_datalist for eachproject_id, build a lookup once and reuse it:--- a/apiserver/plane/bgtasks/export_task.py +++ b/apiserver/plane/bgtasks/export_task.py @@ 458,14 +458,23 @@ def issue_export_task(provider, workspace_id, project_ids, token_id, multiple, slug): files = [] if multiple: - for project_id in project_ids: - issues = [ - issue - for issue in issues_data - if str(issue["project_id"]) == str(project_id) - ] - - exporter = EXPORTER_MAPPER.get(provider) - if exporter is not None: - exporter(header, project_id, issues, files) + # build a mapping of project_id → list of issues + from collections import defaultdict + issues_by_project = defaultdict(list) + for issue in issues_data: + pid = str(issue["project_id"]) + issues_by_project[pid].append(issue) + + for project_id in project_ids: + issues = issues_by_project.get(str(project_id), []) + exporter = EXPORTER_MAPPER.get(provider) + if exporter: + exporter(header, project_id, issues, files) else: exporter = EXPORTER_MAPPER.get(provider) if exporter is not None: exporter(header, workspace_id, issues_data, files)This reduces the work from O(n × m) to O(n + m), yielding significant speedups when exporting many projects or issues.
🧹 Nitpick comments (1)
apiserver/plane/bgtasks/export_task.py (1)
179-179: Long lines exceed recommended line length.Several lines exceed the recommended line length of 88 characters, which may reduce readability:
- Line 179: f-string for comment formatting
- Line 375: String concatenation for created_by
- Line 385: String concatenation for comment's created_by
Consider breaking these into multiple lines or using shorter variable names.
Also applies to: 375-375, 385-385
🧰 Tools
🪛 Ruff (0.8.2)
179-179: Line too long (92 > 88)
(E501)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apiserver/plane/bgtasks/export_task.py(7 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
apiserver/plane/bgtasks/export_task.py (2)
apiserver/plane/db/models/issue.py (1)
Issue(104-253)apiserver/plane/db/models/asset.py (1)
FileAsset(24-104)
🪛 Ruff (0.8.2)
apiserver/plane/bgtasks/export_task.py
179-179: Line too long (92 > 88)
(E501)
375-375: Line too long (92 > 88)
(E501)
385-385: Line too long (104 > 88)
(E501)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Analyze (javascript)
🔇 Additional comments (9)
apiserver/plane/bgtasks/export_task.py (9)
18-20: Appropriate imports added for the refactoring.The imports for
django.db.models.Fandcollections.defaultdictare correctly added to support the new data processing approach. Thedefaultdictis particularly useful for efficiently grouping attachments by issue ID.
23-23: Properly added imports for the required models.The
FileAssetmodel is now explicitly imported to support the new attachment handling functionality.
157-191: Enhanced data export with comprehensive related information.The
generate_table_rowfunction has been updated to include additional valuable information in the export:
- Comments with their text, creation date, and author
- Issue estimates
- Links associated with the issue
- Assignees listed by name
- Subscriber count
- Attachment count and links
This provides a more complete picture of each issue in the exported data.
🧰 Tools
🪛 Ruff (0.8.2)
179-179: Line too long (92 > 88)
(E501)
194-221: JSON export enhanced with comprehensive issue data.Similar to the table row generation, the JSON export now includes the full set of issue information, providing a more complete data export.
314-339: Improved query efficiency with select_related and prefetch_related.The refactored code now properly uses Django's
select_relatedandprefetch_relatedto efficiently fetch related data in a single query, rather than triggering additional queries when accessing related objects. This is a significant optimization that reduces database load.
341-348: Efficient batch processing of file attachments.Instead of querying file assets for each issue individually, the code now fetches all relevant assets in a single query and organizes them by issue ID using a
defaultdict. This approach significantly reduces the number of database queries.
349-411: Comprehensive issue data preparation with related objects.The code now constructs a rich dictionary for each issue, including related data like labels, comments, links, assignees, and attachments. This approach provides more complete information in the exports while maintaining good query performance through prefetching.
🧰 Tools
🪛 Ruff (0.8.2)
375-375: Line too long (92 > 88)
(E501)
385-385: Line too long (104 > 88)
(E501)
444-450: Export headers updated to include additional data.The export headers have been appropriately updated to include all the new fields, ensuring that the exports contain complete information.
475-475: Updated exporter function call to use processed data.The exporter function call correctly uses the processed
issues_datalist instead of the raw queryset.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
apiserver/plane/bgtasks/export_task.py (1)
413-421:⚠️ Potential issueMultiple cycles are not being captured properly.
This code still overwrites cycle data in each iteration, causing only the last cycle to be retained. Since an issue can belong to multiple cycles (based on
unique_together=["issue", "cycle", "deleted_at"]), all cycles should be captured.Apply this change to accumulate all cycles in a list instead:
- # Get prefetched cycles and modules - cycles = list(issue.issue_cycle.all()) - - # Update cycle data - for cycle in cycles: - issue_data["cycle_name"] = cycle.cycle.name - issue_data["cycle_start_date"] = dateConverter(cycle.cycle.start_date) - issue_data["cycle_end_date"] = dateConverter(cycle.cycle.end_date) + # Get prefetched cycles as a list of dictionaries + cycles = list(issue.issue_cycle.all()) + if cycles: + issue_data["cycles"] = [ + { + "cycle_name": ci.cycle.name, + "cycle_start_date": dateConverter(ci.cycle.start_date), + "cycle_end_date": dateConverter(ci.cycle.end_date), + } + for ci in cycles + ] + # For backward compatibility, keep the first cycle data at the top level + issue_data["cycle_name"] = cycles[0].cycle.name + issue_data["cycle_start_date"] = dateConverter(cycles[0].cycle.start_date) + issue_data["cycle_end_date"] = dateConverter(cycles[0].cycle.end_date)Then update the
generate_table_rowandgenerate_json_rowfunctions to handle multiple cycles.
🧹 Nitpick comments (3)
apiserver/plane/bgtasks/export_task.py (3)
178-184: Improve readability of long strings and fix line length issues.The comment formatting logic creates lines that exceed the recommended line length of 88 characters.
Consider reformatting to improve readability:
- ", ".join( - [ - f"{comment['comment']} ({comment['created_at']} by {comment['created_by']})" - for comment in issue["comments"] - ] - ) - if issue["comments"] - else "", + ", ".join([ + f"{comment['comment']} " + f"({comment['created_at']} by {comment['created_by']})" + for comment in issue["comments"] + ]) if issue["comments"] else "",🧰 Tools
🪛 Ruff (0.8.2)
179-179: Line too long (92 > 88)
(E501)
375-375: Improve approach to user name formatting.The full name construction appears multiple times in the code and could be extracted into a helper function.
Consider adding a helper function at the top of the file:
def format_user_name(user): """Format user's full name or return empty string if user is None.""" if not user: return "" return f"{user.first_name} {user.last_name}".strip() or user.emailThen use it throughout the code where user names are formatted.
🧰 Tools
🪛 Ruff (0.8.2)
375-375: Line too long (92 > 88)
(E501)
205-205: JSON key name does not match data structure.The JSON key is still named "Assignee" but now contains a list of assignees.
Apply this change for consistency:
- "Assignee": issue["assignees"], + "Assignees": issue["assignees"],This will also need to be updated in the
update_json_rowfunction at lines 236-237 and 239.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apiserver/plane/bgtasks/export_task.py(7 hunks)
🧰 Additional context used
🪛 Ruff (0.8.2)
apiserver/plane/bgtasks/export_task.py
179-179: Line too long (92 > 88)
(E501)
375-375: Line too long (92 > 88)
(E501)
385-385: Line too long (104 > 88)
(E501)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Analyze (javascript)
- GitHub Check: Analyze (python)
🔇 Additional comments (5)
apiserver/plane/bgtasks/export_task.py (5)
341-348: Efficient query optimization for attachments.Good job using a single query to fetch all file assets and using
defaultdictto organize them by issue ID. This is more efficient than making separate queries for each issue.
315-339: Improved data fetching with related objects.The use of
select_relatedandprefetch_relatedis excellent for reducing database queries. You've included all necessary relationships which will significantly improve performance of the export task.
461-467: Efficient per-project grouping optimization.Great improvement using
defaultdictfor grouping issues by project ID. This changes the grouping cost from O(n × p) to O(n) with no extra DB queries.
444-451: Update to comprehensive CSV headers.Great addition of new columns to the exports including comments, estimate, links, and attachment information. This provides a more complete view of issue data to users.
19-20: Good import cleanup and use of defaultdict.Nice job removing unused imports and adding
defaultdictwhich is used effectively for grouping data.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
apiserver/plane/bgtasks/export_task.py (2)
409-417:⚠️ Potential issueFix cycle data handling to support multiple cycles.
The current implementation overwrites cycle data in each iteration of the loop, causing only the last cycle to be exported. If an issue can belong to multiple cycles, this implementation will lose data.
- # Get prefetched cycles and modules - cycles = list(issue.issue_cycle.all()) - - # Update cycle data - for cycle in cycles: - issue_data["cycle_name"] = cycle.cycle.name - issue_data["cycle_start_date"] = dateConverter(cycle.cycle.start_date) - issue_data["cycle_end_date"] = dateConverter(cycle.cycle.end_date) + # Get prefetched cycles and modules + cycles = list(issue.issue_cycle.all()) + + # Store cycles as a list to support multiple cycles per issue + if cycles: + issue_data["cycles"] = [ + { + "cycle_name": cycle.cycle.name, + "cycle_start_date": dateConverter(cycle.cycle.start_date), + "cycle_end_date": dateConverter(cycle.cycle.end_date), + } + for cycle in cycles + ] + # For backward compatibility, keep the first cycle in the flat fields + issue_data["cycle_name"] = cycles[0].cycle.name + issue_data["cycle_start_date"] = dateConverter(cycles[0].cycle.start_date) + issue_data["cycle_end_date"] = dateConverter(cycles[0].cycle.end_date) + else: + issue_data["cycles"] = [] + issue_data["cycle_name"] = "" + issue_data["cycle_start_date"] = "" + issue_data["cycle_end_date"] = ""
383-383: 💡 Verification agent🧩 Analysis chain
Add null check for created_by field.
The
created_byfield is potentially nullable in theIssuemodel, which could lead to aNoneTypeattribute error.- "created_by": f"{issue.created_by.first_name} {issue.created_by.last_name}", + "created_by": f"{issue.created_by.first_name} {issue.created_by.last_name}" if issue.created_by else "",
🏁 Script executed:
#!/bin/bash # Check if created_by can be null in the Issue model rg -n "created_by\s*=" -A5 -B5 apiserver/plane/db/models/issue.py # Check base model definition that Issue likely inherits from rg -n "class BaseModel" -A20 apiserver/plane/db/models/base.pyLength of output: 900
🏁 Script executed:
#!/bin/bash # Locate the AuditModel to inspect where created_by is declared rg -n 'class AuditModel' -R apiserver/plane/db/models # Find the field definition of created_by across all models rg -n 'created_by\s*=' -R apiserver/plane/db/modelsLength of output: 919
🏁 Script executed:
#!/bin/bash # Locate the file that defines AuditModel audit_file=$(rg -l "class AuditModel" apiserver/plane/db/models) echo "Found AuditModel in: $audit_file" # Show its definition and any created_by field sed -n '1,200p' "$audit_file" | sed -n '/class AuditModel/,/class /p' rg -n "created_by\s*=" "$audit_file"Length of output: 292
🏁 Script executed:
#!/bin/bash # Search for where AuditModel is defined grep -R "class AuditModel" -n apiserver/plane/db/models # Search all occurrences of created_by field declarations in the models directory grep -R "created_by\s*=" -n apiserver/plane/db/modelsLength of output: 261
🏁 Script executed:
#!/bin/bash # List all model files to locate where AuditModel might be defined ls -1 apiserver/plane/db/models # Show the top of base.py to find the import of AuditModel sed -n '1,50p' apiserver/plane/db/models/base.pyLength of output: 1519
🏁 Script executed:
#!/bin/bash # Locate where AuditModel is defined in mixins grep -R "class AuditModel" -n apiserver/plane/db/mixins # Inspect the file defining AuditModel for created_by field declaration grep -R "created_by\s*=" -n apiserver/plane/db/mixinsLength of output: 231
🏁 Script executed:
#!/bin/bash # List the contents of the db directory to locate mixins ls -1 apiserver/plane/db # Search for AuditModel definition in the db directory grep -R "class AuditModel" -n apiserver/plane/db # If mixins.py exists, print its content to inspect created_by declaration if [ -f apiserver/plane/db/mixins.py ]; then sed -n '1,200p' apiserver/plane/db/mixins.py fiLength of output: 2682
Ensure
created_byis null-safe to prevent NoneType errorsThe
created_byfield is declared nullable (null=True) inUserAuditModeland can be unset when there’s no authenticated user. Guard againstNonebefore accessingfirst_name/last_name.• File:
apiserver/plane/bgtasks/export_task.py
Line: ~383Suggested change:
- "created_by": f"{issue.created_by.first_name} {issue.created_by.last_name}", + "created_by": ( + f"{issue.created_by.first_name} {issue.created_by.last_name}" + if issue.created_by else "" + ),🧰 Tools
🪛 Ruff (0.8.2)
383-383: Line too long (92 > 88)
(E501)
🧹 Nitpick comments (2)
apiserver/plane/bgtasks/export_task.py (2)
351-358: Consider using consistent naming for issue-related fields.There's a potential source of confusion with variable naming. You're annotating
issue_idaswork_item_idbut then referencing it in the context of attachments.- file_assets = FileAsset.objects.filter( - issue_id__in=workspace_issues.values_list("id", flat=True) - ).annotate(work_item_id=F("issue_id"), asset_id=F("id")) - - attachment_dict = defaultdict(list) - for asset in file_assets: - attachment_dict[asset.work_item_id].append(asset.asset_id) + file_assets = FileAsset.objects.filter( + issue_id__in=workspace_issues.values_list("id", flat=True) + ).annotate(asset_id=F("id")) + + attachment_dict = defaultdict(list) + for asset in file_assets: + attachment_dict[asset.issue_id].append(asset.asset_id)
389-389: Consider breaking long lines for better readability.Some lines exceed the standard line length of 88 characters, making the code harder to read.
- "created_by": f"{comment.created_by.first_name} {comment.created_by.last_name}", + "created_by": ( + f"{comment.created_by.first_name} {comment.created_by.last_name}" + if comment.created_by else "" + ),🧰 Tools
🪛 Ruff (0.8.2)
389-389: Line too long (104 > 88)
(E501)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apiserver/plane/bgtasks/export_task.py(7 hunks)
🧰 Additional context used
🪛 Ruff (0.8.2)
apiserver/plane/bgtasks/export_task.py
179-179: Line too long (92 > 88)
(E501)
383-383: Line too long (92 > 88)
(E501)
389-389: Line too long (104 > 88)
(E501)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Analyze (javascript)
- GitHub Check: Analyze (python)
🔇 Additional comments (5)
apiserver/plane/bgtasks/export_task.py (5)
13-24: Good job cleaning up imports and adding necessary models.The addition of the Django ORM tools like
Prefetchand use ofdefaultdictfor grouping shows a thoughtful approach to optimization. Also including the necessary models (FileAsset,Label,User) directly improves code clarity.
314-349: Excellent optimization using select_related and prefetch_related.This is a significant performance improvement. Using
select_relatedandprefetch_relatedreduces the number of database queries by fetching related objects in a single query or in a more efficient batch, which is crucial for exporting potentially large amounts of data.
457-463: Excellent optimization using defaultdict for project grouping.Using
defaultdictto group issues by project ID in a single pass is more efficient than the previous approach. This ensures that issues are only iterated once rather than having to check each issue for each project ID.
440-447: Added comprehensive headers for enhanced export data.The addition of new columns in the CSV/Excel export headers like "Comments", "Estimate", "Link", "Assignees", "Subscribers Count", etc. improves the data exports by including more relevant information.
177-191:Details
✅ Verification successful
Long text content in CSV cells may have format issues.
When exporting comments with complex text that includes formatting characters like commas, quotes, or newlines, you might encounter CSV formatting issues despite using
csv.QUOTE_ALL.Consider testing with complex comment data to ensure proper escaping in CSV exports. You might need to pre-process comment text to replace problematic characters.
🏁 Script executed:
#!/bin/bash # Check if there's any comment sanitization in the codebase rg -n "def.*stripped" apiserver/plane/db/models/Length of output: 50
🏁 Script executed:
#!/bin/bash # Locate where the CSV writer is instantiated and check quoting settings rg -n "csv\.writer" -n apiserver/plane/bgtasks/export_task.py rg -n "quoting" -n apiserver/plane/bgtasks/export_task.pyLength of output: 283
CSV quoting is already correctly configured
The export task instantiates the writer with
quoting=csv.QUOTE_ALL, so the built-in Python CSV writer will automatically wrap every field in quotes and escape internal commas, double-quotes (as""), and newlines per the CSV specification. No additional preprocessing of comment text is needed.If you do encounter CSV consumers that don’t properly support quoted newlines, consider normalizing line breaks (e.g. replacing
\n/\rwith spaces) at the point of consumption rather than in the export itself.🧰 Tools
🪛 Ruff (0.8.2)
179-179: Line too long (92 > 88)
(E501)
…7002) * chore: comment details of work item * chore: attachment count and attachment name * chore: issue link and subscriber count * chore: list of assignees * chore: asset_url as attachment_links * chore: code refactor * fix: cannot export Excel * chore: remove print statements * fix: filtering in list * chore: optimize attachment_count and attachment_link query * chore: optimize fetching issue details for multiple select * chore: use Prefetch to avoid duplicates
* chore: comment details of work item * chore: attachment count and attachment name * chore: issue link and subscriber count * chore: list of assignees * chore: asset_url as attachment_links * chore: code refactor * fix: cannot export Excel * chore: remove print statements * fix: filtering in list * chore: optimize attachment_count and attachment_link query * chore: optimize fetching issue details for multiple select * chore: use Prefetch to avoid duplicates
4 participants
Description
This PR will add more fields to the project export and refactor the code.
Type of Change
Summary by CodeRabbit
Reference
WEB-2790