Implement a script that generates a detailed metrics for a project #1737
+350
−4
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Signed-off-by: Ahmed Gouda <[email protected]>
Signed-off-by: Ahmed Gouda <[email protected]>
Summary by CodeRabbit
Summary by CodeRabbit
WalkthroughA new Django management command and supporting Makefile target were added to generate a PDF report containing detailed health metrics for an OWASP project. The implementation uses ReportLab for PDF creation, updates dependencies, adds a test for the command, and ensures PDF files are ignored by version control. Changes
Assessment against linked issues
Assessment against linked issues: Out-of-scope changesNo out-of-scope changes were found. ✨ Finishing Touches
🧪 Generate unit tests
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
Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
backend/apps/owasp/management/commands/owasp_generate_project_health_metrics_pdf.py (1)
45-46: Consider making PDF layout constants configurable.The hard-coded positioning values (300, 700, 680) could be extracted as constants for better maintainability.
+# PDF layout constants +PAGE_WIDTH = 300 +TITLE_Y_POSITION = 700 +SCORE_Y_POSITION = 680 - pdf.drawCentredString(300, 700, f"Health Metrics Report for {metrics.project.name}") - pdf.drawCentredString(300, 680, f"Health Score: {metrics.score:.2f}") + pdf.drawCentredString(PAGE_WIDTH, TITLE_Y_POSITION, f"Health Metrics Report for {metrics.project.name}") + pdf.drawCentredString(PAGE_WIDTH, SCORE_Y_POSITION, f"Health Score: {metrics.score:.2f}")
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
backend/poetry.lockis excluded by!**/*.lock
📒 Files selected for processing (5)
.gitignore(1 hunks)backend/apps/owasp/Makefile(1 hunks)backend/apps/owasp/management/commands/owasp_generate_project_health_metrics_pdf.py(1 hunks)backend/pyproject.toml(1 hunks)backend/tests/apps/owasp/management/commands/owasp_generate_project_health_metrics_test.py(1 hunks)
🧰 Additional context used
🪛 GitHub Actions: Run CI/CD
backend/apps/owasp/management/commands/owasp_generate_project_health_metrics_pdf.py
[warning] 9-9: CSpell: Unknown word 'pdfgen'
[warning] 81-81: CSpell: Unknown word 'BOTTOMPADDING'
backend/tests/apps/owasp/management/commands/owasp_generate_project_health_metrics_test.py
[warning] 13-13: CSpell: Unknown word 'pdfgen'
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: CodeQL (javascript-typescript)
- GitHub Check: CodeQL (python)
🔇 Additional comments (8)
backend/pyproject.toml (1)
57-57: LGTM! Appropriate dependency for PDF generation.The reportlab dependency addition is well-suited for the new PDF metrics report functionality. The version constraint follows the project's existing patterns.
.gitignore (1)
13-13: LGTM! Appropriate exclusion of generated PDF files.Adding
.gitignoreis correct since the new command generates PDF reports that shouldn't be tracked in version control.backend/apps/owasp/Makefile (1)
21-23: LGTM! Well-structured Makefile target.The new target follows the established patterns in the Makefile with appropriate parameterization and user feedback. The integration with the existing
exec-backend-commandtarget is consistent.backend/apps/owasp/management/commands/owasp_generate_project_health_metrics_pdf.py (3)
9-9: Pipeline warning about 'pdfgen' is a false positive.The CSpell warning about 'pdfgen' is incorrect - this is a legitimate reportlab module name for PDF generation.
81-81: Pipeline warning about 'BOTTOMPADDING' is a false positive.The CSpell warning about 'BOTTOMPADDING' is incorrect - this is a legitimate reportlab TableStyle parameter.
64-64: No action needed:contributors_countis defined and covered by tests
- The
ProjectHealthMetricsmodel declarescontributors_countinbackend/apps/owasp/models/project_health_metrics.py.- The PDF generation test (
backend/tests/apps/owasp/management/commands/owasp_generate_project_health_metrics_test.py) references and validatesmetrics.contributors_count.Ignore the original suggestion.
Likely an incorrect or invalid review comment.
backend/tests/apps/owasp/management/commands/owasp_generate_project_health_metrics_test.py (2)
13-13: Pipeline warning about 'pdfgen' is a false positive.The CSpell warning about 'pdfgen' is incorrect - this is a legitimate reportlab module name for PDF generation.
74-74: Update test table data to include contributors_count.The test table data should include the contributors_count value to match the main command implementation.
["Stars", metrics.stars_count], + ["Contributors", metrics.contributors_count], ["Unassigned Issues", metrics.unassigned_issues_count],Likely an incorrect or invalid review comment.
Comment on lines
41
to
102
| buffer = BytesIO() | ||
| pdf = canvas.Canvas(buffer, pagesize=letter) | ||
| pdf.setTitle(f"Health Metrics Report for {metrics.project.name}") | ||
| pdf.setFont("Helvetica", 12) | ||
| pdf.drawCentredString(300, 700, f"Health Metrics Report for {metrics.project.name}") | ||
| pdf.drawCentredString(300, 680, f"Health Score: {metrics.score:.2f}") | ||
| table_data = [ | ||
| ["Metric", "Value"], | ||
| ["Project Age (days)", metrics.age_days], | ||
| ["Last Commit (days)", metrics.last_commit_days], | ||
| ["Last Commit Requirement (days)", metrics.last_commit_days_requirement], | ||
| ["Last Pull Request (days)", metrics.last_pull_request_days], | ||
| ["Last Release (days)", metrics.last_release_days], | ||
| ["Last Release Requirement (days)", metrics.last_release_days_requirement], | ||
| ["OWASP Page Last Update (days)", metrics.owasp_page_last_update_days], | ||
| ["Open Issues", metrics.open_issues_count], | ||
| ["Total Issues", metrics.total_issues_count], | ||
| ["Open Pull Requests", metrics.open_pull_requests_count], | ||
| ["Total Pull Requests", metrics.total_pull_requests_count], | ||
| ["Recent Releases", metrics.recent_releases_count], | ||
| ["Total Releases", metrics.total_releases_count], | ||
| ["Forks", metrics.forks_count], | ||
| ["Stars", metrics.stars_count], | ||
| ["Contributors", metrics.contributors_count], | ||
| ["Unassigned Issues", metrics.unassigned_issues_count], | ||
| ["Unanswered Issues", metrics.unanswered_issues_count], | ||
| ["Has funding issues", "No" if metrics.is_funding_requirements_compliant else "Yes"], | ||
| [ | ||
| "Has leadership issues", | ||
| "No" if metrics.is_leader_requirements_compliant else "Yes", | ||
| ], | ||
| ] | ||
| table = Table(table_data, colWidths="*") | ||
| table.setStyle( | ||
| TableStyle( | ||
| [ | ||
| ("BACKGROUND", (0, 0), (-1, 0), "lightgrey"), | ||
| ("TEXTCOLOR", (0, 0), (-1, 0), "black"), | ||
| ("ALIGN", (0, 0), (-1, -1), "CENTER"), | ||
| ("FONTNAME", (0, 0), (-1, 0), "Helvetica-Bold"), | ||
| ("BOTTOMPADDING", (0, 0), (-1, 0), 5), | ||
| ("BACKGROUND", (0, 1), (-1, -1), "white"), | ||
| ] | ||
| ) | ||
| ) | ||
| table.wrapOn(pdf, 500, 300) | ||
| table.drawOn(pdf, 50, 280) | ||
| pdf.drawCentredString( | ||
| 300, 100, f"Report Generated on: {timezone.now().strftime('%Y-%m-%d %H:%M:%S')}" | ||
| ) | ||
| pdf.showPage() | ||
| pdf.save() | ||
|
|
||
| pdf_path = ( | ||
| Path(settings.base.Base.BASE_DIR) | ||
| / "reports" | ||
| / f"{project_key}_health_metrics_report.pdf" | ||
| ) | ||
|
|
||
| pdf_path.parent.mkdir(parents=True, exist_ok=True) | ||
| pdf_path.write_bytes(buffer.getvalue()) | ||
| buffer.close() |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve resource management with proper cleanup.
The BytesIO buffer should be properly closed in a finally block or using a context manager to prevent resource leaks if exceptions occur.
- buffer = BytesIO()
- pdf = canvas.Canvas(buffer, pagesize=letter)
+ buffer = BytesIO()
+ try:
+ pdf = canvas.Canvas(buffer, pagesize=letter)
+ # ... existing PDF generation code ...
+ pdf.save()
+
+ pdf_path = (
+ Path(settings.BASE_DIR)
+ / "reports"
+ / f"{project_key}_health_metrics_report.pdf"
+ )
+
+ pdf_path.parent.mkdir(parents=True, exist_ok=True)
+ pdf_path.write_bytes(buffer.getvalue())
+ finally:
+ buffer.close()Committable suggestion skipped: line range outside the PR's diff.
🧰 Tools
🪛 GitHub Actions: Run CI/CD
[warning] 81-81: CSpell: Unknown word 'BOTTOMPADDING'
🤖 Prompt for AI Agents
In
backend/apps/owasp/management/commands/owasp_generate_project_health_metrics_pdf.py
around lines 41 to 102, the BytesIO buffer is not properly closed if an
exception occurs, risking resource leaks. Refactor the code to use a context
manager (with statement) for the BytesIO buffer to ensure it is automatically
closed after use, even if an error happens during PDF generation or file
writing.
backend/apps/owasp/management/commands/owasp_generate_project_health_metrics_pdf.py
Outdated
Show resolved
Hide resolved
backend/tests/apps/owasp/management/commands/owasp_generate_project_health_metrics_test.py
Outdated
Show resolved
Hide resolved
|
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
backend/apps/owasp/management/commands/owasp_generate_project_health_metrics_pdf.py (1)
41-98: Fix resource management and add error handling.The BytesIO buffer lacks proper resource management and the PDF generation has no error handling, which could lead to resource leaks and unhandled exceptions.
Apply this refactor to address both issues:
- buffer = BytesIO() - pdf = canvas.Canvas(buffer, pagesize=letter) - pdf.setTitle(f"Health Metrics Report for {metrics.project.name}") - pdf.setFont("Helvetica", 12) - pdf.drawCentredString(300, 700, f"Health Metrics Report for {metrics.project.name}") - pdf.drawCentredString(300, 680, f"Health Score: {metrics.score:.2f}") - table_data = [ - ["Metric", "Value"], - ["Project Age (days)", metrics.age_days], - ["Last Commit (days)", metrics.last_commit_days], - ["Last Commit Requirement (days)", metrics.last_commit_days_requirement], - ["Last Pull Request (days)", metrics.last_pull_request_days], - ["Last Release (days)", metrics.last_release_days], - ["Last Release Requirement (days)", metrics.last_release_days_requirement], - ["OWASP Page Last Update (days)", metrics.owasp_page_last_update_days], - ["Open Issues", metrics.open_issues_count], - ["Total Issues", metrics.total_issues_count], - ["Open Pull Requests", metrics.open_pull_requests_count], - ["Total Pull Requests", metrics.total_pull_requests_count], - ["Recent Releases", metrics.recent_releases_count], - ["Total Releases", metrics.total_releases_count], - ["Forks", metrics.forks_count], - ["Stars", metrics.stars_count], - ["Contributors", metrics.contributors_count], - ["Unassigned Issues", metrics.unassigned_issues_count], - ["Unanswered Issues", metrics.unanswered_issues_count], - ["Has funding issues", "No" if metrics.is_funding_requirements_compliant else "Yes"], - [ - "Has leadership issues", - "No" if metrics.is_leader_requirements_compliant else "Yes", - ], - ] - table = Table(table_data, colWidths="*") - table.setStyle( - TableStyle( - [ - ("BACKGROUND", (0, 0), (-1, 0), "lightgrey"), - ("TEXTCOLOR", (0, 0), (-1, 0), "black"), - ("ALIGN", (0, 0), (-1, -1), "CENTER"), - ("FONTNAME", (0, 0), (-1, 0), "Helvetica-Bold"), - ("BOTTOMPADDING", (0, 0), (-1, 0), 5), - ("BACKGROUND", (0, 1), (-1, -1), "white"), - ] - ) - ) - table.wrapOn(pdf, 500, 300) - table.drawOn(pdf, 50, 280) - pdf.drawCentredString( - 300, 100, f"Report Generated on: {timezone.now().strftime('%Y-%m-%d %H:%M:%S')}" - ) - pdf.showPage() - pdf.save() - - pdf_path = Path(settings.BASE_DIR) / "reports" / f"{project_key}_health_metrics_report.pdf" - - pdf_path.parent.mkdir(parents=True, exist_ok=True) - pdf_path.write_bytes(buffer.getvalue()) - buffer.close() + try: + with BytesIO() as buffer: + pdf = canvas.Canvas(buffer, pagesize=letter) + pdf.setTitle(f"Health Metrics Report for {metrics.project.name}") + pdf.setFont("Helvetica", 12) + pdf.drawCentredString(300, 700, f"Health Metrics Report for {metrics.project.name}") + pdf.drawCentredString(300, 680, f"Health Score: {metrics.score:.2f}") + table_data = [ + ["Metric", "Value"], + ["Project Age (days)", metrics.age_days], + ["Last Commit (days)", metrics.last_commit_days], + ["Last Commit Requirement (days)", metrics.last_commit_days_requirement], + ["Last Pull Request (days)", metrics.last_pull_request_days], + ["Last Release (days)", metrics.last_release_days], + ["Last Release Requirement (days)", metrics.last_release_days_requirement], + ["OWASP Page Last Update (days)", metrics.owasp_page_last_update_days], + ["Open Issues", metrics.open_issues_count], + ["Total Issues", metrics.total_issues_count], + ["Open Pull Requests", metrics.open_pull_requests_count], + ["Total Pull Requests", metrics.total_pull_requests_count], + ["Recent Releases", metrics.recent_releases_count], + ["Total Releases", metrics.total_releases_count], + ["Forks", metrics.forks_count], + ["Stars", metrics.stars_count], + ["Contributors", metrics.contributors_count], + ["Unassigned Issues", metrics.unassigned_issues_count], + ["Unanswered Issues", metrics.unanswered_issues_count], + ["Has funding issues", "No" if metrics.is_funding_requirements_compliant else "Yes"], + [ + "Has leadership issues", + "No" if metrics.is_leader_requirements_compliant else "Yes", + ], + ] + table = Table(table_data, colWidths="*") + table.setStyle( + TableStyle( + [ + ("BACKGROUND", (0, 0), (-1, 0), "lightgrey"), + ("TEXTCOLOR", (0, 0), (-1, 0), "black"), + ("ALIGN", (0, 0), (-1, -1), "CENTER"), + ("FONTNAME", (0, 0), (-1, 0), "Helvetica-Bold"), + ("BOTTOMPADDING", (0, 0), (-1, 0), 5), + ("BACKGROUND", (0, 1), (-1, -1), "white"), + ] + ) + ) + table.wrapOn(pdf, 500, 300) + table.drawOn(pdf, 50, 280) + pdf.drawCentredString( + 300, 100, f"Report Generated on: {timezone.now().strftime('%Y-%m-%d %H:%M:%S')}" + ) + pdf.showPage() + pdf.save() + + pdf_path = Path(settings.BASE_DIR) / "reports" / f"{project_key}_health_metrics_report.pdf" + pdf_path.parent.mkdir(parents=True, exist_ok=True) + pdf_path.write_bytes(buffer.getvalue()) + except Exception as e: + self.stdout.write( + self.style.ERROR(f"Failed to generate PDF report: {str(e)}") + ) + return
🧹 Nitpick comments (3)
backend/apps/owasp/management/commands/owasp_generate_project_health_metrics_pdf.py (3)
28-33: Consider validating the project-key argument.The command assumes the project key format but doesn't validate the input. Consider adding basic validation to ensure the project key is not empty or contains invalid characters.
def handle(self, *args, **options): project_key = options["project_key"] + if not project_key: + self.stdout.write(self.style.ERROR("Project key is required")) + return + metrics = ( ProjectHealthMetrics.get_latest_health_metrics() .filter(project__key=f"www-project-{project_key}") .first() )
67-71: Clarify compliance logic for better readability.The inverted logic where "No" means compliant and "Yes" means non-compliant could be confusing. Consider making this more explicit.
- ["Has funding issues", "No" if metrics.is_funding_requirements_compliant else "Yes"], - [ - "Has leadership issues", - "No" if metrics.is_leader_requirements_compliant else "Yes", - ], + ["Funding Requirements", "Compliant" if metrics.is_funding_requirements_compliant else "Non-compliant"], + [ + "Leadership Requirements", + "Compliant" if metrics.is_leader_requirements_compliant else "Non-compliant", + ],
45-46: Consider using constants for positioning values.The hard-coded positioning values make the code less maintainable. Consider defining constants for better readability and easier adjustments.
+# PDF layout constants +PAGE_WIDTH, PAGE_HEIGHT = letter +CENTER_X = PAGE_WIDTH // 2 +TITLE_Y = 700 +SUBTITLE_Y = 680 +TIMESTAMP_Y = 100 pdf.drawCentredString(300, 700, f"Health Metrics Report for {metrics.project.name}") pdf.drawCentredString(300, 680, f"Health Score: {metrics.score:.2f}")Then update the drawCentredString calls:
- pdf.drawCentredString(300, 700, f"Health Metrics Report for {metrics.project.name}") - pdf.drawCentredString(300, 680, f"Health Score: {metrics.score:.2f}") + pdf.drawCentredString(CENTER_X, TITLE_Y, f"Health Metrics Report for {metrics.project.name}") + pdf.drawCentredString(CENTER_X, SUBTITLE_Y, f"Health Score: {metrics.score:.2f}")- pdf.drawCentredString( - 300, 100, f"Report Generated on: {timezone.now().strftime('%Y-%m-%d %H:%M:%S')}" - ) + pdf.drawCentredString( + CENTER_X, TIMESTAMP_Y, f"Report Generated on: {timezone.now().strftime('%Y-%m-%d %H:%M:%S')}" + )Also applies to: 88-90
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
backend/apps/owasp/management/commands/owasp_generate_project_health_metrics_pdf.py(1 hunks)backend/tests/apps/owasp/management/commands/owasp_generate_project_health_metrics_test.py(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- backend/tests/apps/owasp/management/commands/owasp_generate_project_health_metrics_test.py
🧰 Additional context used
🧬 Code Graph Analysis (1)
backend/apps/owasp/management/commands/owasp_generate_project_health_metrics_pdf.py (4)
backend/apps/owasp/models/project_health_metrics.py (2)
ProjectHealthMetrics(16-210)get_latest_health_metrics(145-158)backend/apps/github/models/issue.py (1)
open_issues_count(183-185)backend/apps/owasp/models/project.py (6)
open_pull_requests_count(190-192)recent_releases_count(241-246)unassigned_issues_count(259-261)unanswered_issues_count(254-256)is_funding_requirements_compliant(128-130)is_leader_requirements_compliant(133-137)backend/apps/owasp/graphql/nodes/committee.py (3)
forks_count(25-27)stars_count(40-42)contributors_count(15-17)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: CodeQL (javascript-typescript)
- GitHub Check: CodeQL (python)
🔇 Additional comments (2)
backend/apps/owasp/management/commands/owasp_generate_project_health_metrics_pdf.py (2)
1-14: LGTM! Clean imports and documentation.The imports are well-organized and use standard Django conventions. The docstring clearly describes the module's purpose.
21-25: LGTM! Standard argument parsing implementation.The argument definition follows Django management command conventions correctly.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Resolves #1734
nest_health_metrics_report.pdf