Fetch pull requests events #159
Merged
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
WalkthroughThis update introduces a new implementation for fetching GitHub pull requests across organization repositories, including related reviews, comments, issues, and releases. It adds a formatter for structuring pull request data, integrates the new fetcher into the warehouse ingestion pipeline, schedules automated executions, and provides comprehensive RSpec tests for the new functionality. Changes
Sequence Diagram(s)sequenceDiagram
participant Scheduler
participant Script
participant Fetcher as FetchPullRequestsFromGithub
participant GitHubAPI
participant Formatter
participant Storage
Scheduler->>Script: Trigger scheduled PR fetch script
Script->>Fetcher: Initialize with config and storage
Fetcher->>GitHubAPI: Authenticate and fetch repositories
Fetcher->>GitHubAPI: Fetch releases per repository
loop For each repository
Fetcher->>GitHubAPI: Fetch pull requests
loop For each pull request
Fetcher->>GitHubAPI: Fetch reviews, comments, related issues
Fetcher->>Formatter: Format PR data with context
end
end
Fetcher->>Storage: Write formatted PRs in paginated batches
Possibly related PRs
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 (
|
cammv21
force-pushed
the
fetch-pull-requests-events
branch
from
00b493a to
6bc6d53
Compare
FelipeGuzmanSierra
requested changes
Jul 17, 2025
Collaborator
FelipeGuzmanSierra
left a comment
FelipeGuzmanSierra
left a comment
There was a problem hiding this comment.
I left a few comments, please, let me know if you want to discuss them
cammv21
force-pushed
the
fetch-pull-requests-events
branch
from
6bc6d53 to
6170242
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (1)
src/implementations/fetch_pull_requests_from_github.rb (1)
5-5: Use HTTParty instead of Octokit as per coding guidelinesThe coding guidelines specify using HTTParty for HTTP requests. Consider refactoring to use HTTParty with GitHub's REST API instead of the Octokit client.
Also applies to: 78-78
🧹 Nitpick comments (1)
src/use_cases_execution/warehouse/github/fetch_kommit_co_pull_requests_from_github.rb (1)
32-34: Enhance error handling beyond loggingCurrently, errors are only logged to stdout. Consider implementing proper error tracking or alerting for production environments.
rescue StandardError => e Logger.new($stdout).info(e.message) + # Consider adding error tracking/alerting here + # e.g., send to error monitoring service end
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
spec/implementations/warehouse/github/fetch_pull_requests_from_github_spec.rb(1 hunks)src/implementations/fetch_pull_requests_from_github.rb(1 hunks)src/implementations/warehouse_ingester.rb(2 hunks)src/use_cases_execution/schedules.rb(1 hunks)src/use_cases_execution/warehouse/github/fetch_kommit_co_pull_requests_from_github.rb(1 hunks)src/use_cases_execution/warehouse/github/fetch_kommitters_pull_requests_from_github.rb(1 hunks)src/use_cases_execution/warehouse/notion/warehouse_ingester.rb(1 hunks)src/utils/warehouse/github/base.rb(3 hunks)src/utils/warehouse/github/pull_requests_format.rb(1 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
**/*.rb
Instructions used from:
Sources:
📄 CodeRabbit Inference Engine
- CLAUDE.md
src/use_cases_execution/schedules.rb
Instructions used from:
Sources:
📄 CodeRabbit Inference Engine
- CLAUDE.md
src/implementations/**/*.rb
Instructions used from:
Sources:
📄 CodeRabbit Inference Engine
- CLAUDE.md
spec/implementations/**/*.rb
Instructions used from:
Sources:
📄 CodeRabbit Inference Engine
- CLAUDE.md
🧠 Learnings (6)
src/use_cases_execution/warehouse/github/fetch_kommitters_pull_requests_from_github.rb (6)
Learnt from: CR
PR: kommitters/bas_use_cases#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-02T20:14:00.049Z
Learning: Applies to db/build_shared_storage.sql : Update database schema in `db/build_shared_storage.sql` if needed when adding a new use case
Learnt from: CR
PR: kommitters/bas_use_cases#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-02T20:14:00.049Z
Learning: Applies to src/use_cases_execution/*/*.rb : Create use case directory in `src/use_cases_execution/[name]/` with `config.rb` and 4 pipeline files when adding a new use case
Learnt from: juanhiginio
PR: kommitters/bas_use_cases#158
File: spec/implementations/deploy_process_in_operaton/deploy_process_in_operaton_spec.rb:33-42
Timestamp: 2025-07-15T03:46:00.728Z
Learning: In the bas_use_cases repository, implementation test files in `spec/implementations/` follow a minimal testing pattern with a single test that verifies `execute` doesn't return nil, using mocked shared storage dependencies. This is the established convention across all implementation files rather than comprehensive edge case testing.
Learnt from: CR
PR: kommitters/bas_use_cases#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-02T20:14:00.049Z
Learning: Applies to src/implementations/**/*.rb : Create implementations in `src/implementations/` (if they don't already exist) when adding a new use case
Learnt from: juanpabloxk
PR: kommitters/bas_use_cases#104
File: src/use_cases_execution/schedules.rb:119-127
Timestamp: 2025-06-06T20:08:44.949Z
Learning: In the bas_use_cases repository, the user juanpabloxk prefers to keep schedule constants consolidated in a single file (src/use_cases_execution/schedules.rb) rather than extracting them to separate files, even when it exceeds RuboCop's 100-line module limit.
Learnt from: CR
PR: kommitters/bas_use_cases#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-02T20:14:00.049Z
Learning: Applies to src/use_cases_execution/*/config.rb : Database connection configuration should be standardized across all use cases in each `config.rb`
src/use_cases_execution/schedules.rb (2)
Learnt from: CR
PR: kommitters/bas_use_cases#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-02T20:14:00.049Z
Learning: Applies to src/use_cases_execution/schedules.rb : Add schedule constants to `src/use_cases_execution/schedules.rb` when adding a new use case
Learnt from: juanpabloxk
PR: kommitters/bas_use_cases#104
File: src/use_cases_execution/schedules.rb:119-127
Timestamp: 2025-06-06T20:08:44.949Z
Learning: In the bas_use_cases repository, the user juanpabloxk prefers to keep schedule constants consolidated in a single file (src/use_cases_execution/schedules.rb) rather than extracting them to separate files, even when it exceeds RuboCop's 100-line module limit.
src/implementations/warehouse_ingester.rb (5)
Learnt from: CR
PR: kommitters/bas_use_cases#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-02T20:14:00.049Z
Learning: Applies to src/implementations/**/*.rb : Create implementations in `src/implementations/` (if they don't already exist) when adding a new use case
Learnt from: CR
PR: kommitters/bas_use_cases#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-02T20:14:00.049Z
Learning: Applies to src/use_cases_execution/*/config.rb : Database connection configuration should be standardized across all use cases in each `config.rb`
Learnt from: juanhiginio
PR: kommitters/bas_use_cases#158
File: spec/implementations/deploy_process_in_operaton/deploy_process_in_operaton_spec.rb:33-42
Timestamp: 2025-07-15T03:46:00.728Z
Learning: In the bas_use_cases repository, implementation test files in `spec/implementations/` follow a minimal testing pattern with a single test that verifies `execute` doesn't return nil, using mocked shared storage dependencies. This is the established convention across all implementation files rather than comprehensive edge case testing.
Learnt from: juanpabloxk
PR: kommitters/bas_use_cases#104
File: src/use_cases_execution/schedules.rb:119-127
Timestamp: 2025-06-06T20:08:44.949Z
Learning: In the bas_use_cases repository, the user juanpabloxk prefers to keep schedule constants consolidated in a single file (src/use_cases_execution/schedules.rb) rather than extracting them to separate files, even when it exceeds RuboCop's 100-line module limit.
Learnt from: CR
PR: kommitters/bas_use_cases#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-02T20:14:00.049Z
Learning: Applies to **/*.rb : Use explicit imports for modules (e.g., `Date`, `DateTime`); do not rely on Rails autoloading
src/use_cases_execution/warehouse/github/fetch_kommit_co_pull_requests_from_github.rb (7)
Learnt from: CR
PR: kommitters/bas_use_cases#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-02T20:14:00.049Z
Learning: Applies to src/implementations/**/*.rb : Create implementations in `src/implementations/` (if they don't already exist) when adding a new use case
Learnt from: juanhiginio
PR: kommitters/bas_use_cases#158
File: spec/implementations/deploy_process_in_operaton/deploy_process_in_operaton_spec.rb:33-42
Timestamp: 2025-07-15T03:46:00.728Z
Learning: In the bas_use_cases repository, implementation test files in `spec/implementations/` follow a minimal testing pattern with a single test that verifies `execute` doesn't return nil, using mocked shared storage dependencies. This is the established convention across all implementation files rather than comprehensive edge case testing.
Learnt from: CR
PR: kommitters/bas_use_cases#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-02T20:14:00.049Z
Learning: Applies to src/use_cases_execution/*/*.rb : Create use case directory in `src/use_cases_execution/[name]/` with `config.rb` and 4 pipeline files when adding a new use case
Learnt from: CR
PR: kommitters/bas_use_cases#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-02T20:14:00.049Z
Learning: Applies to db/build_shared_storage.sql : Update database schema in `db/build_shared_storage.sql` if needed when adding a new use case
Learnt from: juanpabloxk
PR: kommitters/bas_use_cases#104
File: src/use_cases_execution/schedules.rb:119-127
Timestamp: 2025-06-06T20:08:44.949Z
Learning: In the bas_use_cases repository, the user juanpabloxk prefers to keep schedule constants consolidated in a single file (src/use_cases_execution/schedules.rb) rather than extracting them to separate files, even when it exceeds RuboCop's 100-line module limit.
Learnt from: CR
PR: kommitters/bas_use_cases#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-02T20:14:00.049Z
Learning: Applies to src/use_cases_execution/schedules.rb : Add schedule constants to `src/use_cases_execution/schedules.rb` when adding a new use case
Learnt from: CR
PR: kommitters/bas_use_cases#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-02T20:14:00.049Z
Learning: Applies to src/use_cases_execution/*/config.rb : Database connection configuration should be standardized across all use cases in each `config.rb`
src/implementations/fetch_pull_requests_from_github.rb (3)
Learnt from: CR
PR: kommitters/bas_use_cases#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-02T20:14:00.049Z
Learning: Applies to src/implementations/**/*.rb : Create implementations in `src/implementations/` (if they don't already exist) when adding a new use case
Learnt from: kren-prog
PR: kommitters/bas_use_cases#99
File: src/utils/workspace_helpers/google_chat_space_request_manager.rb:49-59
Timestamp: 2025-06-09T21:43:25.685Z
Learning: In the bas_use_cases Ruby project, avoid assigning HTTParty.post() results to a variable if the method will just return that variable immediately, as it's considered redundant. Only assign to a response variable when the response needs to be processed (e.g., printed) before returning.
Learnt from: juanhiginio
PR: kommitters/bas_use_cases#158
File: spec/implementations/deploy_process_in_operaton/deploy_process_in_operaton_spec.rb:33-42
Timestamp: 2025-07-15T03:46:00.728Z
Learning: In the bas_use_cases repository, implementation test files in `spec/implementations/` follow a minimal testing pattern with a single test that verifies `execute` doesn't return nil, using mocked shared storage dependencies. This is the established convention across all implementation files rather than comprehensive edge case testing.
spec/implementations/warehouse/github/fetch_pull_requests_from_github_spec.rb (3)
Learnt from: CR
PR: kommitters/bas_use_cases#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-02T20:14:00.049Z
Learning: Applies to spec/implementations/**/*.rb : Write comprehensive tests in `spec/implementations/[name]/` when adding a new use case
Learnt from: juanhiginio
PR: kommitters/bas_use_cases#158
File: spec/implementations/deploy_process_in_operaton/deploy_process_in_operaton_spec.rb:33-42
Timestamp: 2025-07-15T03:46:00.728Z
Learning: In the bas_use_cases repository, implementation test files in `spec/implementations/` follow a minimal testing pattern with a single test that verifies `execute` doesn't return nil, using mocked shared storage dependencies. This is the established convention across all implementation files rather than comprehensive edge case testing.
Learnt from: CR
PR: kommitters/bas_use_cases#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-02T20:14:00.049Z
Learning: Applies to src/implementations/**/*.rb : Create implementations in `src/implementations/` (if they don't already exist) when adding a new use case
🧬 Code Graph Analysis (3)
src/utils/warehouse/github/pull_requests_format.rb (1)
src/utils/warehouse/github/base.rb (9)
extract_id(29-31)extract_repository_id(53-55)extract_related_issues(77-81)extract_release_id(83-89)format_pg_array(102-106)format_reviews_as_json(91-100)extract_title(73-75)extract_created_at(45-47)extract_merged_at(69-71)
src/implementations/fetch_pull_requests_from_github.rb (1)
src/utils/warehouse/github/pull_requests_format.rb (2)
format(12-28)format(15-27)
spec/implementations/warehouse/github/fetch_pull_requests_from_github_spec.rb (1)
src/implementations/fetch_pull_requests_from_github.rb (3)
process(41-176)process(48-58)write(63-70)
⏰ 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). (1)
- GitHub Check: Tests
🔇 Additional comments (10)
src/use_cases_execution/warehouse/notion/warehouse_ingester.rb (1)
26-26: LGTM! Fetcher addition follows established pattern.The addition of
FetchPullRequestsFromGithubto the fetchers array is consistent with the existing GitHub fetchers and maintains the logical ordering.src/implementations/warehouse_ingester.rb (2)
16-16: LGTM! Service require follows established pattern.The require statement for the github_pull_request service is consistent with other GitHub service imports.
68-69: LGTM! Service mapping follows established pattern.The SERVICES hash entry for
github_pull_requestfollows the same structure as other GitHub services, with appropriate service class and external key mapping.src/use_cases_execution/schedules.rb (1)
161-161: LGTM! Scheduling follows established pattern.The scheduling time and path structure are consistent with other GitHub fetchers in the warehouse sync pipeline.
src/utils/warehouse/github/pull_requests_format.rb (1)
1-32: LGTM! Format class follows established pattern.The
PullRequestsFormatclass properly inherits fromBaseand implements a comprehensiveformatmethod that maps all relevant pull request fields. The implementation is consistent with other format classes in the codebase and correctly uses the extraction methods from the base class.src/use_cases_execution/warehouse/github/fetch_kommitters_pull_requests_from_github.rb (1)
1-35: LGTM! Execution script follows established pattern.The script properly follows the established pattern for GitHub fetcher execution scripts with correct configuration, error handling, and integration with the shared storage system.
spec/implementations/warehouse/github/fetch_pull_requests_from_github_spec.rb (1)
9-130: Excellent comprehensive test coverage!The test suite thoroughly covers authentication failures, successful data fetching, formatting, and pagination logic. This goes beyond the minimal testing pattern and provides robust coverage.
src/utils/warehouse/github/base.rb (2)
83-89: Review release association logicThe current logic finds the first release published after the PR was merged. This might miss the intended release if there's a delay between merge and release, or if multiple releases happen quickly.
Consider whether this is the intended behavior. You might want to find the release that includes the PR based on commit history or tags instead of just timing.
69-75: LGTM! Safe hash access patternsGood use of hash notation with symbols for accessing data, which is safer than method calls on potentially nil objects.
src/implementations/fetch_pull_requests_from_github.rb (1)
116-125: Well-structured data aggregationGood separation of concerns - fetching all related data in a context hash before formatting. This makes the code more testable and maintainable.
src/use_cases_execution/warehouse/github/fetch_kommit_co_pull_requests_from_github.rb
Show resolved
Hide resolved
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.
Description
This PR introduces the FetchPullRequestsFromGithub service, responsible for fetching all pull requests (both open and closed) from the repositories within the kommitters and kommit-co GitHub organizations.
The implementation establishes the basic structure for the fetcher and refactors the data extraction strategy to use the Octokit library.
Fixes #156
Type of change
Checklist
Summary by CodeRabbit
New Features
Bug Fixes
Refactor
Tests