Centralize Config and unify Ingester Logic #196
Conversation
|
Warning Rate limit exceeded@cammv21 has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 15 minutes and 43 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (37)
WalkthroughThis change centralizes and unifies configuration management for warehouse ingestion scripts by introducing a comprehensive Changes
Sequence Diagram(s)sequenceDiagram
participant Script as Warehouse Script
participant Config as Config Module
participant ENV as Environment
participant File as File System
Script->>Config: require '../config'
Config->>ENV: fetch variables (DB, Notion, Worklogs, Github)
Config->>File: read PEM files (if Github)
Config-->>Script: provide nested config objects
Script->>Script: Use Config::Database, Config::Notion, etc.
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~15 minutes Assessment against linked issues
Assessment against linked issues: Out-of-scope changes
Possibly related PRs
Suggested reviewers
Poem
✨ 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
CodeRabbit Configuration File (
|
![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
🔭 Outside diff range comments (2)
src/use_cases_execution/warehouse/google_workspace/listen_to_google_docs_updates.rb (1)
60-66:loggeris undefined → exception paths will crashInside both rescue clauses you call
logger.error, butSinatra::Basedoes not expose aloggerhelper unlessrequire 'sinatra/logger'(or equivalent) is loaded.
A failing request will therefore raiseNoMethodError, masking the original problem.+require 'logger' +LOGGER = Logger.new($stdout) … -rescue JSON::ParserError => e - logger.error "Invalid JSON format: #{e.message}" +rescue JSON::ParserError => e + LOGGER.error "Invalid JSON format: #{e.message}" … -rescue StandardError => e - logger.error "Failed to process Google documents data: #{e.message}\n#{e.backtrace.join("\n")}" +rescue StandardError => e + LOGGER.error "Failed to process Google documents data: #{e.message}\n#{e.backtrace.join("\n")}"Replicate the fix (or
require 'sinatra/logger') across all Sinatra routes to avoid hidden 500s.src/use_cases_execution/warehouse/github/fetch_kommit_co_releases_from_github.rb (1)
6-6: Incorrect require path will crash the script at runtime
config.rbwas deleted from the current folder. Keepingrequire_relative 'config'will raiseLoadErrorthe first time the bot runs.-require_relative 'config' +require_relative '../config'
♻️ Duplicate comments (8)
src/use_cases_execution/warehouse/google_workspace/listen_to_google_calendar_updates.rb (1)
41-47: Same missing-logger issue as the Google-Docs route
logger.errorwill blow up for the same reason described in the previous file.
Apply the same remedy (sharedLOGGERconstant orsinatra/logger), otherwise webhook failures will never be persisted.src/use_cases_execution/warehouse/notion/fetch_milestones.rb (1)
9-22: Same duplication remark as infetch_domains.rbThe identical
read_options/write_optionsduplication applies here as well.src/use_cases_execution/warehouse/worklogs/fetch_work_logs.rb (1)
10-20: Same dual-connection observation applies hereSee earlier comment—
Config::Database::CONNECTIONis used for the sync table but noWAREHOUSE_CONNECTIONis supplied anywhere.
IfBas::SharedStorage::Postgresneeds a dedicated warehouse connection, make sure you pass it (or rename the constant to make this clear).src/use_cases_execution/warehouse/notion/fetch_weekly_scopes.rb (1)
25-27: Ensure weekly-scopes constant existsSame concern as above: verify
Config::Notion::WEEKLY_SCOPES_DATABASE_IDis present; otherwise update the constant or the config.src/use_cases_execution/warehouse/github/fetch_kommit_co_pull_requests_from_github.rb (1)
9-21: Same read/write-options duplication as in Notion fetchersSee earlier comment in
fetch_work_items.rb; extracting a small builder will remove this boilerplate from every fetch script.src/use_cases_execution/warehouse/notion/fetch_key_results.rb (1)
9-23: SharedStorage options duplication – same observation as infetch_work_items.rb.src/use_cases_execution/warehouse/notion/fetch_persons.rb (1)
9-23: Redundant boilerplate – identicalread_options/write_optionspattern found in every fetcher. Extract to a helper.src/use_cases_execution/warehouse/notion/fetch_activities.rb (1)
9-23: Consider factoring out common storage-options code – see earlier comments.
🧹 Nitpick comments (13)
src/use_cases_execution/warehouse/github/fetch_kommit_co_releases_from_github.rb (1)
3-5:bas/shared_storage/defaultis never used – drop the requireThe file only instantiates
Bas::SharedStorage::Postgres, so requiringbas/shared_storage/defaultadds unnecessary I/O and memory.
Safe to remove.-require 'bas/shared_storage/default'src/use_cases_execution/warehouse/github/fetch_kommitters_pull_requests_from_github.rb (1)
5-5: Unused dependency
bas/shared_storage/defaultis required but not referenced anywhere in the file. Removing it trims load-time overhead.-require 'bas/shared_storage/default'src/use_cases_execution/warehouse/notion/fetch_domains.rb (1)
9-22: Consider extracting common read/write option buildersEvery Notion/GitHub fetcher duplicates the same
read_options/write_optionshash literal with only thetagdiffering. A tiny helper (e.g.,Warehouse::SharedStorageOptions.for(tag)) would eliminate ≈15 identical blocks and cut maintenance effort.No action required for this PR, but worth tracking.
src/use_cases_execution/warehouse/github/fetch_kommit_co_issues_from_github.rb (1)
5-5: Remove unused require
Bas::SharedStorage::Defaultis not referenced – the extra require is redundant.-require 'bas/shared_storage/default'src/use_cases_execution/warehouse/notion/fetch_hired_persons.rb (1)
24-27: Constant naming may not match centralized config
Config::Notion::HIRED_PERSONS_DATABASE_IDandConfig::Notion::SECRETmust exist inwarehouse/config.rb.
If the constant is still calledHIRED_PERSONS_NOTION_DATABASE_ID(old pattern) the script will raiseNameError.Consider aligning all Notion IDs to the pattern
<ENTITY>_DATABASE_IDand drop the redundantNOTIONterm now that they live underConfig::Notion.src/use_cases_execution/warehouse/notion/fetch_projects.rb (1)
11-22: Potential drift between constant namesDouble-check that
Config::Notion::PROJECTS_DATABASE_IDmatches the name chosen in the central config file.
Several other scripts still referencePROJECTS_NOTION_DATABASE_ID; if that alias was kept for backward compatibility, pick one and delete the other to avoid dead code.src/use_cases_execution/warehouse/github/fetch_kommitters_releases_from_github.rb (2)
5-6: Unnecessary require inflates boot time
require 'bas/shared_storage/default'is never referenced in this script. Drop it to avoid loading unused code.-require 'bas/shared_storage/default'
23-29: Guard against missing GitHub sub-config keysThe new indirection via
github_config = Config::Github::KOMMITERSis neat, but it will explode withKeyErrorif any key is absent.
Consider defensive defaults or a small validation step:github_config = Config::Github::KOMMITERS -%w[private_pem app_id organization].each { |k| raise "Missing #{k}" unless github_config.key?(k.to_sym) }src/use_cases_execution/warehouse/notion/fetch_work_items.rb (1)
9-23: Extract common read/write-options builder to cut duplicationThe
read_options/write_optionshashes repeat verbatim across every fetcher. Consider a helper such asWarehouse::SharedStorageOptions.build(tag)(or a factory insideConfig) to return the two hashes.
Benefits: single-point maintenance and clearer intent.src/use_cases_execution/warehouse/github/fetch_kommit_co_pull_requests_from_github.rb (1)
5-5: Unused require
require 'bas/shared_storage/default'is not referenced in this script;Bas::SharedStorage::Postgresis used instead. Drop the extra require to avoid loading unused code.-require 'bas/shared_storage/default'src/use_cases_execution/warehouse/notion/fetch_documents.rb (1)
11-16: Missing type hint / docstring forread_optionsEvery execution script in
src/use_cases_execution/*/**/*.rbshould briefly document the expected schema of the shared-storage options (see coding guidelines).
Consider adding a one-liner comment explaining the structure, e.g. key meanings and accepted values.
Purely a readability nit—logic is correct.src/use_cases_execution/warehouse/config.rb (2)
3-4: Avoid loading Dotenv in production runtime
require 'dotenv/load'parses.envon every process start, which is unnecessary (and sometimes disallowed) in production containers where env vars are injected.
Consider:require 'dotenv/load' if ENV['RACK_ENV'] != 'production'or removing the line and letting the hosting environment supply variables.
52-54: Surface missing environment variables early
ENV.fetchwill raise at runtime. To fail fast during boot, you can validate all required keys once:REQUIRED_ENV_VARS = %w[ DB_HOST DB_PORT POSTGRES_DB POSTGRES_USER POSTGRES_PASSWORD WAREHOUSE_POSTGRES_DB NOTION_SECRET ... ].freeze missing = REQUIRED_ENV_VARS.reject { |k| ENV.key?(k) } raise "Missing env vars: #{missing.join(', ')}" unless missing.empty?Helps operators identify misconfiguration quickly.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (34)
spec/services/postgres/activity_spec.rb(1 hunks)spec/services/postgres/document_spec.rb(1 hunks)spec/services/postgres/key_results_history_spec.rb(1 hunks)spec/services/postgres/project_spec.rb(1 hunks)spec/services/postgres/weekly_scope_spec.rb(1 hunks)src/use_cases_execution/schedules.rb(1 hunks)src/use_cases_execution/warehouse/config.rb(1 hunks)src/use_cases_execution/warehouse/github/config.rb(0 hunks)src/use_cases_execution/warehouse/github/fetch_kommit_co_issues_from_github.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_kommit_co_releases_from_github.rb(1 hunks)src/use_cases_execution/warehouse/github/fetch_kommitters_issues_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/github/fetch_kommitters_releases_from_github.rb(1 hunks)src/use_cases_execution/warehouse/google/config.rb(0 hunks)src/use_cases_execution/warehouse/google/warehouse_ingester.rb(0 hunks)src/use_cases_execution/warehouse/google_workspace/config.rb(0 hunks)src/use_cases_execution/warehouse/google_workspace/listen_to_google_calendar_updates.rb(2 hunks)src/use_cases_execution/warehouse/google_workspace/listen_to_google_docs_updates.rb(1 hunks)src/use_cases_execution/warehouse/notion/config.rb(0 hunks)src/use_cases_execution/warehouse/notion/fetch_activities.rb(1 hunks)src/use_cases_execution/warehouse/notion/fetch_documents.rb(1 hunks)src/use_cases_execution/warehouse/notion/fetch_domains.rb(1 hunks)src/use_cases_execution/warehouse/notion/fetch_hired_persons.rb(1 hunks)src/use_cases_execution/warehouse/notion/fetch_key_results.rb(1 hunks)src/use_cases_execution/warehouse/notion/fetch_milestones.rb(1 hunks)src/use_cases_execution/warehouse/notion/fetch_persons.rb(1 hunks)src/use_cases_execution/warehouse/notion/fetch_projects.rb(1 hunks)src/use_cases_execution/warehouse/notion/fetch_weekly_scopes.rb(1 hunks)src/use_cases_execution/warehouse/notion/fetch_work_items.rb(1 hunks)src/use_cases_execution/warehouse/notion/garbage_collector.rb(1 hunks)src/use_cases_execution/warehouse/warehouse_ingester.rb(2 hunks)src/use_cases_execution/warehouse/worklogs/config.rb(0 hunks)src/use_cases_execution/warehouse/worklogs/fetch_work_logs.rb(1 hunks)
💤 Files with no reviewable changes (6)
- src/use_cases_execution/warehouse/google/config.rb
- src/use_cases_execution/warehouse/google_workspace/config.rb
- src/use_cases_execution/warehouse/notion/config.rb
- src/use_cases_execution/warehouse/github/config.rb
- src/use_cases_execution/warehouse/worklogs/config.rb
- src/use_cases_execution/warehouse/google/warehouse_ingester.rb
🧰 Additional context used
📓 Path-based instructions (4)
**/*.rb
📄 CodeRabbit Inference Engine (CLAUDE.md)
**/*.rb: Use HTTParty for HTTP requests
Do not use Rails conventions; use explicit imports for modules (e.g., Date, DateTime)
Follow Ruby conventions and best practices
Add relevant documentation comments to explain how users can use and understand the code
Add an empty line at the end of files
Use clear variable naming
Implement proper error handling
Files:
spec/services/postgres/activity_spec.rbspec/services/postgres/key_results_history_spec.rbspec/services/postgres/project_spec.rbspec/services/postgres/weekly_scope_spec.rbsrc/use_cases_execution/warehouse/notion/garbage_collector.rbspec/services/postgres/document_spec.rbsrc/use_cases_execution/warehouse/google_workspace/listen_to_google_calendar_updates.rbsrc/use_cases_execution/warehouse/google_workspace/listen_to_google_docs_updates.rbsrc/use_cases_execution/warehouse/github/fetch_kommit_co_releases_from_github.rbsrc/use_cases_execution/warehouse/notion/fetch_milestones.rbsrc/use_cases_execution/schedules.rbsrc/use_cases_execution/warehouse/github/fetch_kommitters_releases_from_github.rbsrc/use_cases_execution/warehouse/github/fetch_kommitters_pull_requests_from_github.rbsrc/use_cases_execution/warehouse/notion/fetch_domains.rbsrc/use_cases_execution/warehouse/notion/fetch_hired_persons.rbsrc/use_cases_execution/warehouse/notion/fetch_work_items.rbsrc/use_cases_execution/warehouse/github/fetch_kommit_co_pull_requests_from_github.rbsrc/use_cases_execution/warehouse/notion/fetch_documents.rbsrc/use_cases_execution/warehouse/notion/fetch_projects.rbsrc/use_cases_execution/warehouse/warehouse_ingester.rbsrc/use_cases_execution/warehouse/notion/fetch_key_results.rbsrc/use_cases_execution/warehouse/worklogs/fetch_work_logs.rbsrc/use_cases_execution/warehouse/notion/fetch_persons.rbsrc/use_cases_execution/warehouse/notion/fetch_activities.rbsrc/use_cases_execution/warehouse/config.rbsrc/use_cases_execution/warehouse/github/fetch_kommitters_issues_from_github.rbsrc/use_cases_execution/warehouse/notion/fetch_weekly_scopes.rbsrc/use_cases_execution/warehouse/github/fetch_kommit_co_issues_from_github.rb
src/use_cases_execution/*/**/*.rb
📄 CodeRabbit Inference Engine (CLAUDE.md)
src/use_cases_execution/*/**/*.rb: Use case orchestration files that wire implementations together should be placed in src/use_cases_execution/[use_case]/
All use cases must use Bas::SharedStorage with PostgreSQL as the coordination layer, with each pipeline step reading from and writing to the database using specific tags (FetchX, FormatX, NotifyX, GarbageCollector)
Files:
src/use_cases_execution/warehouse/notion/garbage_collector.rbsrc/use_cases_execution/warehouse/google_workspace/listen_to_google_calendar_updates.rbsrc/use_cases_execution/warehouse/google_workspace/listen_to_google_docs_updates.rbsrc/use_cases_execution/warehouse/github/fetch_kommit_co_releases_from_github.rbsrc/use_cases_execution/warehouse/notion/fetch_milestones.rbsrc/use_cases_execution/warehouse/github/fetch_kommitters_releases_from_github.rbsrc/use_cases_execution/warehouse/github/fetch_kommitters_pull_requests_from_github.rbsrc/use_cases_execution/warehouse/notion/fetch_domains.rbsrc/use_cases_execution/warehouse/notion/fetch_hired_persons.rbsrc/use_cases_execution/warehouse/notion/fetch_work_items.rbsrc/use_cases_execution/warehouse/github/fetch_kommit_co_pull_requests_from_github.rbsrc/use_cases_execution/warehouse/notion/fetch_documents.rbsrc/use_cases_execution/warehouse/notion/fetch_projects.rbsrc/use_cases_execution/warehouse/warehouse_ingester.rbsrc/use_cases_execution/warehouse/notion/fetch_key_results.rbsrc/use_cases_execution/warehouse/worklogs/fetch_work_logs.rbsrc/use_cases_execution/warehouse/notion/fetch_persons.rbsrc/use_cases_execution/warehouse/notion/fetch_activities.rbsrc/use_cases_execution/warehouse/config.rbsrc/use_cases_execution/warehouse/github/fetch_kommitters_issues_from_github.rbsrc/use_cases_execution/warehouse/notion/fetch_weekly_scopes.rbsrc/use_cases_execution/warehouse/github/fetch_kommit_co_issues_from_github.rb
src/use_cases_execution/schedules.rb
📄 CodeRabbit Inference Engine (CLAUDE.md)
src/use_cases_execution/schedules.rb: Centralized scheduling configuration should be placed in src/use_cases_execution/schedules.rb
Schedules must be defined using time, day, or interval keys in src/use_cases_execution/schedules.rb
Files:
src/use_cases_execution/schedules.rb
src/use_cases_execution/*/config.rb
📄 CodeRabbit Inference Engine (CLAUDE.md)
Database connection configuration must be standardized in each config.rb file within use case directories
Files:
src/use_cases_execution/warehouse/config.rb
🧠 Learnings (19)
📓 Common learnings
Learnt from: CR
PR: kommitters/bas_use_cases#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-21T22:59:03.954Z
Learning: Applies to src/use_cases_execution/schedules.rb : Centralized scheduling configuration should be placed in src/use_cases_execution/schedules.rb
Learnt from: CR
PR: kommitters/bas_use_cases#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-21T22:59:03.954Z
Learning: Applies to src/use_cases_execution/*/config.rb : Database connection configuration must be standardized in each config.rb file within use case directories
📚 Learning: applies to src/use_cases_execution/*/config.rb : database connection configuration must be standardi...
Learnt from: CR
PR: kommitters/bas_use_cases#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-21T22:59:03.954Z
Learning: Applies to src/use_cases_execution/*/config.rb : Database connection configuration must be standardized in each config.rb file within use case directories
Applied to files:
spec/services/postgres/activity_spec.rbspec/services/postgres/key_results_history_spec.rbspec/services/postgres/project_spec.rbspec/services/postgres/weekly_scope_spec.rbsrc/use_cases_execution/warehouse/notion/garbage_collector.rbspec/services/postgres/document_spec.rbsrc/use_cases_execution/warehouse/google_workspace/listen_to_google_calendar_updates.rbsrc/use_cases_execution/warehouse/google_workspace/listen_to_google_docs_updates.rbsrc/use_cases_execution/warehouse/github/fetch_kommit_co_releases_from_github.rbsrc/use_cases_execution/warehouse/notion/fetch_milestones.rbsrc/use_cases_execution/warehouse/github/fetch_kommitters_releases_from_github.rbsrc/use_cases_execution/warehouse/github/fetch_kommitters_pull_requests_from_github.rbsrc/use_cases_execution/warehouse/notion/fetch_domains.rbsrc/use_cases_execution/warehouse/notion/fetch_hired_persons.rbsrc/use_cases_execution/warehouse/notion/fetch_work_items.rbsrc/use_cases_execution/warehouse/github/fetch_kommit_co_pull_requests_from_github.rbsrc/use_cases_execution/warehouse/notion/fetch_documents.rbsrc/use_cases_execution/warehouse/notion/fetch_projects.rbsrc/use_cases_execution/warehouse/warehouse_ingester.rbsrc/use_cases_execution/warehouse/notion/fetch_key_results.rbsrc/use_cases_execution/warehouse/worklogs/fetch_work_logs.rbsrc/use_cases_execution/warehouse/notion/fetch_persons.rbsrc/use_cases_execution/warehouse/notion/fetch_activities.rbsrc/use_cases_execution/warehouse/config.rbsrc/use_cases_execution/warehouse/github/fetch_kommitters_issues_from_github.rbsrc/use_cases_execution/warehouse/notion/fetch_weekly_scopes.rbsrc/use_cases_execution/warehouse/github/fetch_kommit_co_issues_from_github.rb
📚 Learning: applies to src/use_cases_execution/*/**/*.rb : all use cases must use bas::sharedstorage with postgr...
Learnt from: CR
PR: kommitters/bas_use_cases#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-21T22:59:03.954Z
Learning: Applies to src/use_cases_execution/*/**/*.rb : All use cases must use Bas::SharedStorage with PostgreSQL as the coordination layer, with each pipeline step reading from and writing to the database using specific tags (FetchX, FormatX, NotifyX, GarbageCollector)
Applied to files:
spec/services/postgres/activity_spec.rbspec/services/postgres/key_results_history_spec.rbspec/services/postgres/project_spec.rbspec/services/postgres/weekly_scope_spec.rbsrc/use_cases_execution/warehouse/notion/garbage_collector.rbspec/services/postgres/document_spec.rbsrc/use_cases_execution/warehouse/google_workspace/listen_to_google_calendar_updates.rbsrc/use_cases_execution/warehouse/google_workspace/listen_to_google_docs_updates.rbsrc/use_cases_execution/warehouse/github/fetch_kommit_co_releases_from_github.rbsrc/use_cases_execution/warehouse/notion/fetch_milestones.rbsrc/use_cases_execution/schedules.rbsrc/use_cases_execution/warehouse/github/fetch_kommitters_releases_from_github.rbsrc/use_cases_execution/warehouse/github/fetch_kommitters_pull_requests_from_github.rbsrc/use_cases_execution/warehouse/notion/fetch_domains.rbsrc/use_cases_execution/warehouse/notion/fetch_hired_persons.rbsrc/use_cases_execution/warehouse/notion/fetch_work_items.rbsrc/use_cases_execution/warehouse/github/fetch_kommit_co_pull_requests_from_github.rbsrc/use_cases_execution/warehouse/notion/fetch_documents.rbsrc/use_cases_execution/warehouse/notion/fetch_projects.rbsrc/use_cases_execution/warehouse/warehouse_ingester.rbsrc/use_cases_execution/warehouse/notion/fetch_key_results.rbsrc/use_cases_execution/warehouse/worklogs/fetch_work_logs.rbsrc/use_cases_execution/warehouse/notion/fetch_persons.rbsrc/use_cases_execution/warehouse/notion/fetch_activities.rbsrc/use_cases_execution/warehouse/config.rbsrc/use_cases_execution/warehouse/github/fetch_kommitters_issues_from_github.rbsrc/use_cases_execution/warehouse/notion/fetch_weekly_scopes.rbsrc/use_cases_execution/warehouse/github/fetch_kommit_co_issues_from_github.rb
📚 Learning: in the bas_use_cases project, sequel migrations consistently use `trueclass` instead of `boolean` fo...
Learnt from: juanhiginio
PR: kommitters/bas_use_cases#158
File: db/migrations/20250714191503_create_operaton_process_deployed_table.rb:9-9
Timestamp: 2025-07-15T03:15:22.811Z
Learning: In the bas_use_cases project, Sequel migrations consistently use `TrueClass` instead of `Boolean` for boolean column declarations.
Applied to files:
spec/services/postgres/activity_spec.rbspec/services/postgres/key_results_history_spec.rbspec/services/postgres/project_spec.rbspec/services/postgres/weekly_scope_spec.rbspec/services/postgres/document_spec.rb
📚 Learning: in the bas_use_cases repository, implementation test files in `spec/implementations/` follow a minim...
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.750Z
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.
Applied to files:
spec/services/postgres/activity_spec.rbspec/services/postgres/key_results_history_spec.rbspec/services/postgres/project_spec.rbspec/services/postgres/weekly_scope_spec.rbsrc/use_cases_execution/warehouse/notion/garbage_collector.rbspec/services/postgres/document_spec.rbsrc/use_cases_execution/warehouse/github/fetch_kommitters_pull_requests_from_github.rbsrc/use_cases_execution/warehouse/github/fetch_kommit_co_pull_requests_from_github.rbsrc/use_cases_execution/warehouse/notion/fetch_projects.rbsrc/use_cases_execution/warehouse/notion/fetch_key_results.rb
📚 Learning: applies to db/build_shared_storage.sql : database schema changes should be made in db/build_shared_s...
Learnt from: CR
PR: kommitters/bas_use_cases#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-21T22:59:03.954Z
Learning: Applies to db/build_shared_storage.sql : Database schema changes should be made in db/build_shared_storage.sql
Applied to files:
spec/services/postgres/key_results_history_spec.rbsrc/use_cases_execution/warehouse/notion/garbage_collector.rbspec/services/postgres/document_spec.rbsrc/use_cases_execution/warehouse/google_workspace/listen_to_google_docs_updates.rbsrc/use_cases_execution/warehouse/notion/fetch_milestones.rbsrc/use_cases_execution/warehouse/notion/fetch_domains.rbsrc/use_cases_execution/warehouse/notion/fetch_hired_persons.rbsrc/use_cases_execution/warehouse/notion/fetch_work_items.rbsrc/use_cases_execution/warehouse/notion/fetch_documents.rbsrc/use_cases_execution/warehouse/notion/fetch_projects.rbsrc/use_cases_execution/warehouse/notion/fetch_key_results.rbsrc/use_cases_execution/warehouse/worklogs/fetch_work_logs.rbsrc/use_cases_execution/warehouse/notion/fetch_persons.rbsrc/use_cases_execution/warehouse/notion/fetch_weekly_scopes.rb
📚 Learning: applies to src/use_cases_execution/schedules.rb : centralized scheduling configuration should be pla...
Learnt from: CR
PR: kommitters/bas_use_cases#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-21T22:59:03.954Z
Learning: Applies to src/use_cases_execution/schedules.rb : Centralized scheduling configuration should be placed in src/use_cases_execution/schedules.rb
Applied to files:
spec/services/postgres/weekly_scope_spec.rbsrc/use_cases_execution/warehouse/notion/garbage_collector.rbsrc/use_cases_execution/warehouse/google_workspace/listen_to_google_calendar_updates.rbsrc/use_cases_execution/warehouse/google_workspace/listen_to_google_docs_updates.rbsrc/use_cases_execution/warehouse/notion/fetch_milestones.rbsrc/use_cases_execution/schedules.rbsrc/use_cases_execution/warehouse/github/fetch_kommitters_releases_from_github.rbsrc/use_cases_execution/warehouse/github/fetch_kommitters_pull_requests_from_github.rbsrc/use_cases_execution/warehouse/notion/fetch_domains.rbsrc/use_cases_execution/warehouse/notion/fetch_hired_persons.rbsrc/use_cases_execution/warehouse/notion/fetch_work_items.rbsrc/use_cases_execution/warehouse/github/fetch_kommit_co_pull_requests_from_github.rbsrc/use_cases_execution/warehouse/notion/fetch_documents.rbsrc/use_cases_execution/warehouse/notion/fetch_projects.rbsrc/use_cases_execution/warehouse/warehouse_ingester.rbsrc/use_cases_execution/warehouse/notion/fetch_key_results.rbsrc/use_cases_execution/warehouse/worklogs/fetch_work_logs.rbsrc/use_cases_execution/warehouse/notion/fetch_persons.rbsrc/use_cases_execution/warehouse/notion/fetch_activities.rbsrc/use_cases_execution/warehouse/config.rbsrc/use_cases_execution/warehouse/github/fetch_kommitters_issues_from_github.rbsrc/use_cases_execution/warehouse/notion/fetch_weekly_scopes.rbsrc/use_cases_execution/warehouse/github/fetch_kommit_co_issues_from_github.rb
📚 Learning: applies to **/*.rb : use clear variable naming...
Learnt from: CR
PR: kommitters/bas_use_cases#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-21T22:59:03.954Z
Learning: Applies to **/*.rb : Use clear variable naming
Applied to files:
src/use_cases_execution/warehouse/notion/garbage_collector.rb
📚 Learning: applies to **/*.rb : do not use rails conventions; use explicit imports for modules (e.g., date, dat...
Learnt from: CR
PR: kommitters/bas_use_cases#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-21T22:59:03.954Z
Learning: Applies to **/*.rb : Do not use Rails conventions; use explicit imports for modules (e.g., Date, DateTime)
Applied to files:
src/use_cases_execution/warehouse/notion/garbage_collector.rbsrc/use_cases_execution/warehouse/google_workspace/listen_to_google_calendar_updates.rbsrc/use_cases_execution/warehouse/notion/fetch_milestones.rbsrc/use_cases_execution/warehouse/worklogs/fetch_work_logs.rb
📚 Learning: applies to src/implementations/**/*.rb : reusable bot implementations should be placed in src/implem...
Learnt from: CR
PR: kommitters/bas_use_cases#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-21T22:59:03.954Z
Learning: Applies to src/implementations/**/*.rb : Reusable bot implementations should be placed in src/implementations/ and inherit from Bas::Bot::Base
Applied to files:
src/use_cases_execution/warehouse/notion/garbage_collector.rbsrc/use_cases_execution/warehouse/google_workspace/listen_to_google_calendar_updates.rbsrc/use_cases_execution/warehouse/github/fetch_kommitters_releases_from_github.rbsrc/use_cases_execution/warehouse/github/fetch_kommitters_pull_requests_from_github.rbsrc/use_cases_execution/warehouse/github/fetch_kommit_co_pull_requests_from_github.rbsrc/use_cases_execution/warehouse/warehouse_ingester.rbsrc/use_cases_execution/warehouse/github/fetch_kommitters_issues_from_github.rbsrc/use_cases_execution/warehouse/github/fetch_kommit_co_issues_from_github.rb
📚 Learning: files under src/utils/warehouse/google_workspace/** should declare the module hierarchy utils::wareh...
Learnt from: cammv21
PR: kommitters/bas_use_cases#175
File: src/utils/warehouse/google_workspace/base.rb:5-7
Timestamp: 2025-07-25T00:18:55.020Z
Learning: Files under src/utils/warehouse/google_workspace/** should declare the module hierarchy Utils::Warehouse::GoogleWorkspace to mirror the directory structure.
Applied to files:
src/use_cases_execution/warehouse/google_workspace/listen_to_google_calendar_updates.rbsrc/use_cases_execution/warehouse/google_workspace/listen_to_google_docs_updates.rbsrc/use_cases_execution/warehouse/warehouse_ingester.rbsrc/use_cases_execution/warehouse/config.rb
📚 Learning: in the bas_use_cases repository, the user juanpabloxk prefers to keep schedule constants consolidate...
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.
Applied to files:
src/use_cases_execution/warehouse/google_workspace/listen_to_google_calendar_updates.rbsrc/use_cases_execution/schedules.rbsrc/use_cases_execution/warehouse/github/fetch_kommitters_releases_from_github.rbsrc/use_cases_execution/warehouse/github/fetch_kommitters_pull_requests_from_github.rbsrc/use_cases_execution/warehouse/github/fetch_kommit_co_pull_requests_from_github.rbsrc/use_cases_execution/warehouse/worklogs/fetch_work_logs.rbsrc/use_cases_execution/warehouse/notion/fetch_persons.rbsrc/use_cases_execution/warehouse/config.rbsrc/use_cases_execution/warehouse/github/fetch_kommitters_issues_from_github.rbsrc/use_cases_execution/warehouse/github/fetch_kommit_co_issues_from_github.rb
📚 Learning: applies to src/use_cases_execution/schedules.rb : schedules must be defined using time, day, or inte...
Learnt from: CR
PR: kommitters/bas_use_cases#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-21T22:59:03.954Z
Learning: Applies to src/use_cases_execution/schedules.rb : Schedules must be defined using time, day, or interval keys in src/use_cases_execution/schedules.rb
Applied to files:
src/use_cases_execution/warehouse/google_workspace/listen_to_google_calendar_updates.rbsrc/use_cases_execution/schedules.rb
📚 Learning: in the bas_use_cases project, this is not a rails project. the correct way to log messages is using ...
Learnt from: juanpabloxk
PR: kommitters/bas_use_cases#104
File: src/implementations/notify_github_issues_to_notion.rb:50-63
Timestamp: 2025-06-06T20:15:51.163Z
Learning: In the bas_use_cases project, this is not a Rails project. The correct way to log messages is using `Logger.new($stdout).info("message")` instead of `Rails.logger.error`.
Applied to files:
src/use_cases_execution/warehouse/google_workspace/listen_to_google_calendar_updates.rbsrc/use_cases_execution/warehouse/github/fetch_kommitters_releases_from_github.rbsrc/use_cases_execution/warehouse/github/fetch_kommitters_pull_requests_from_github.rbsrc/use_cases_execution/warehouse/github/fetch_kommit_co_pull_requests_from_github.rbsrc/use_cases_execution/warehouse/warehouse_ingester.rbsrc/use_cases_execution/warehouse/worklogs/fetch_work_logs.rbsrc/use_cases_execution/warehouse/github/fetch_kommitters_issues_from_github.rbsrc/use_cases_execution/warehouse/github/fetch_kommit_co_issues_from_github.rb
📚 Learning: applies to **/*.rb : follow ruby conventions and best practices...
Learnt from: CR
PR: kommitters/bas_use_cases#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-21T22:59:03.954Z
Learning: Applies to **/*.rb : Follow Ruby conventions and best practices
Applied to files:
src/use_cases_execution/warehouse/google_workspace/listen_to_google_calendar_updates.rb
📚 Learning: applies to **/*.rb : use httparty for http requests...
Learnt from: CR
PR: kommitters/bas_use_cases#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-21T22:59:03.954Z
Learning: Applies to **/*.rb : Use HTTParty for HTTP requests
Applied to files:
src/use_cases_execution/warehouse/google_workspace/listen_to_google_calendar_updates.rb
📚 Learning: applies to **/*.rb : add relevant documentation comments to explain how users can use and understand...
Learnt from: CR
PR: kommitters/bas_use_cases#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-21T22:59:03.954Z
Learning: Applies to **/*.rb : Add relevant documentation comments to explain how users can use and understand the code
Applied to files:
src/use_cases_execution/warehouse/google_workspace/listen_to_google_docs_updates.rbsrc/use_cases_execution/warehouse/notion/fetch_documents.rbsrc/use_cases_execution/warehouse/worklogs/fetch_work_logs.rb
📚 Learning: in the wip limit use cases, `src/use_cases_execution/wip_limit/fetch_domains_wip_count.rb` should us...
Learnt from: kren-prog
PR: kommitters/bas_use_cases#108
File: src/use_cases_execution/wip_limit/fetch_domains_wip_limit.rb:23-23
Timestamp: 2025-06-06T22:44:09.353Z
Learning: In the WIP limit use cases, `src/use_cases_execution/wip_limit/fetch_domains_wip_count.rb` should use `WIP_COUNT_NOTION_DATABASE_ID` while `src/use_cases_execution/wip_limit/fetch_domains_wip_limit.rb` should use `WIP_LIMIT_NOTION_DATABASE_ID`. These are intentionally different environment variables for different purposes.
Applied to files:
src/use_cases_execution/warehouse/notion/fetch_milestones.rbsrc/use_cases_execution/warehouse/notion/fetch_domains.rbsrc/use_cases_execution/warehouse/notion/fetch_hired_persons.rbsrc/use_cases_execution/warehouse/notion/fetch_work_items.rbsrc/use_cases_execution/warehouse/notion/fetch_documents.rbsrc/use_cases_execution/warehouse/notion/fetch_projects.rbsrc/use_cases_execution/warehouse/notion/fetch_key_results.rbsrc/use_cases_execution/warehouse/worklogs/fetch_work_logs.rbsrc/use_cases_execution/warehouse/notion/fetch_persons.rbsrc/use_cases_execution/warehouse/notion/fetch_activities.rbsrc/use_cases_execution/warehouse/notion/fetch_weekly_scopes.rb
📚 Learning: applies to env.yml : environment variables should be configured via env.yml, copying from env.yml.ex...
Learnt from: CR
PR: kommitters/bas_use_cases#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-21T22:59:03.954Z
Learning: Applies to env.yml : Environment variables should be configured via env.yml, copying from env.yml.example
Applied to files:
src/use_cases_execution/warehouse/config.rb
🧬 Code Graph Analysis (1)
src/use_cases_execution/warehouse/google_workspace/listen_to_google_docs_updates.rb (1)
src/use_cases_execution/warehouse/google_workspace/listen_to_google_calendar_updates.rb (2)
initialize(12-49)initialize(13-20)
🪛 GitHub Actions: CI
src/use_cases_execution/warehouse/config.rb
[warning] 15-15: Warning: already initialized constant Config::Database::CONNECTION.
[warning] 23-23: Warning: already initialized constant Config::Database::WAREHOUSE_CONNECTION.
[warning] 36-36: Warning: already initialized constant Config::Notion::SECRET.
🔇 Additional comments (15)
spec/services/postgres/activity_spec.rb (1)
16-16: Concise hash syntax is fineThe switch to a single-line
configliteral is purely stylistic and does not affect the test behaviour.spec/services/postgres/project_spec.rb (1)
15-15: Looks good—stylistic simplification onlyThe inline hash keeps the setup readable and consistent with the other specs.
spec/services/postgres/key_results_history_spec.rb (1)
15-15: No issues with the one-linerMaintains the same connection details while matching the new formatting convention.
spec/services/postgres/document_spec.rb (1)
14-14: Formatting change acknowledgedSingle-line
configimproves brevity without altering semantics.spec/services/postgres/weekly_scope_spec.rb (1)
14-14: Uniform config style—OKThe shortened hash mirrors the pattern across specs; no functional impact.
src/use_cases_execution/warehouse/notion/garbage_collector.rb (1)
25-27: Align reader and writer database connectionsEnsure that both
shared_storage_readerandshared_storage_writertarget the same database. Right now, the reader usesBas::SharedStorage::Default(which pulls from ENV vars) while the writer usesConfig::Database::CONNECTION. If those sources differ, reads and writes could end up on different databases and corrupt your pipeline.Please verify how
Bas::SharedStorage::Defaultderives its connection and update the reader to explicitly use the same connection as the writer. For example:• File:
src/use_cases_execution/warehouse/notion/garbage_collector.rb(lines 25–27)- shared_storage_reader = Bas::SharedStorage::Default.new + shared_storage_reader = Bas::SharedStorage::Postgres.new( + read_options: { + connection: Config::Database::CONNECTION, + db_table: 'warehouse_sync' + } + )src/use_cases_execution/schedules.rb (1)
160-163: Confirm orchestrator supportsintervalfor the unified ingesterThe new unified path looks correct. Ensure the scheduler interprets
interval: 3_600_000(ms) in the same way it interpretstime:/day:.
If not, the central ingester will never run.No code change required; just double-check the orchestrator’s DSL.
src/use_cases_execution/warehouse/github/fetch_kommitters_issues_from_github.rb (1)
23-29: Nice consolidation of GitHub configExtracting
github_config = Config::Github::KOMMITERSremoves three hard-wired constants and keeps the options hash concise.src/use_cases_execution/warehouse/notion/fetch_hired_persons.rb (1)
10-20: Verify distinct DB connection constantsFile: src/use_cases_execution/warehouse/notion/fetch_hired_persons.rb
Lines: 10–20connection: Config::Database::CONNECTION, db_table: 'warehouse_sync', avoid_process: true, where: 'archived=$1 AND tag=$2 ORDER BY inserted_at DESC', params: [false, 'FetchHiredPersonsFromNotionDatabase'] } write_options = { connection: Config::Database::CONNECTION, db_table: 'warehouse_sync', tag: 'FetchHiredPersonsFromNotionDatabase'I wasn’t able to locate definitions for Config::Database::CONNECTION or Config::Database::WAREHOUSE_CONNECTION in the codebase. Please confirm:
- Do both constants resolve to the same database connection?
- If yes, unify them (or alias one to the other) to reduce cognitive overhead.
- If they intentionally point at different databases, add a clarifying comment at each use site to document their distinct roles.
src/use_cases_execution/warehouse/worklogs/fetch_work_logs.rb (1)
24-26: Confirm Worklogs constants exist in new namespace
Config::Worklogs::URLandConfig::Worklogs::API_SECRETmust be defined in the central config.
Run the same grep check to be safe.src/use_cases_execution/warehouse/notion/fetch_work_items.rb (1)
7-7: Config require path looks correct
require_relative '../config'resolves one level up towarehouse/config.rb, matching the new layout.src/use_cases_execution/warehouse/github/fetch_kommit_co_pull_requests_from_github.rb (1)
23-29: Guard against missing keys inConfig::Github::KOMMIT_COIf any of
:private_pem,:app_id, or:organizationis absent the script will raise at runtime.
Optionally add a quick upfront check:%i[private_pem app_id organization].each do |k| raise "Missing github_config key #{k}" unless github_config.key?(k) endsrc/use_cases_execution/warehouse/notion/fetch_key_results.rb (1)
24-28: Config namespacing looks goodSwitch to
Config::Notion::*is consistent with the new central module.src/use_cases_execution/warehouse/warehouse_ingester.rb (2)
27-29: Validate new tag names are implemented
FetchGoogleDocumentsFromWorkspaceandFetchCalendarEventsFromWebhookwere added to the tag list, but no corresponding implementation files were part of this PR.
Please confirm the classes exist and adhere to the same contract as the other fetchers; otherwise the ingester will raise at runtime.
46-47: Ambiguous choice of connection
options[:db]now receivesConfig::Database::WAREHOUSE_CONNECTIONwhereas the read/write options still targetConfig::Database::CONNECTION.
If these point to different databases you may ingest from one and mark rows processed in another, losing traceability. Double-check that this split is intentional.
cammv21
force-pushed
the
unify-warehouse-ingesters
branch
2 times, most recently
from
a3e8cce to
821a645
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (36)
spec/services/postgres/activity_spec.rb(1 hunks)spec/services/postgres/document_spec.rb(1 hunks)spec/services/postgres/key_results_history_spec.rb(1 hunks)spec/services/postgres/project_spec.rb(1 hunks)spec/services/postgres/weekly_scope_spec.rb(1 hunks)spec/use_cases_execution/google_workspace/listen_to_google_calendar_updates_spec.rb(1 hunks)spec/use_cases_execution/google_workspace/listen_to_google_docs_updates_spec.rb(1 hunks)src/use_cases_execution/schedules.rb(1 hunks)src/use_cases_execution/warehouse/config.rb(1 hunks)src/use_cases_execution/warehouse/github/config.rb(0 hunks)src/use_cases_execution/warehouse/github/fetch_kommit_co_issues_from_github.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_kommit_co_releases_from_github.rb(1 hunks)src/use_cases_execution/warehouse/github/fetch_kommitters_issues_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/github/fetch_kommitters_releases_from_github.rb(1 hunks)src/use_cases_execution/warehouse/google/config.rb(0 hunks)src/use_cases_execution/warehouse/google/warehouse_ingester.rb(0 hunks)src/use_cases_execution/warehouse/google_workspace/config.rb(0 hunks)src/use_cases_execution/warehouse/google_workspace/listen_to_google_calendar_updates.rb(2 hunks)src/use_cases_execution/warehouse/google_workspace/listen_to_google_docs_updates.rb(1 hunks)src/use_cases_execution/warehouse/notion/config.rb(0 hunks)src/use_cases_execution/warehouse/notion/fetch_activities.rb(1 hunks)src/use_cases_execution/warehouse/notion/fetch_documents.rb(1 hunks)src/use_cases_execution/warehouse/notion/fetch_domains.rb(1 hunks)src/use_cases_execution/warehouse/notion/fetch_hired_persons.rb(1 hunks)src/use_cases_execution/warehouse/notion/fetch_key_results.rb(1 hunks)src/use_cases_execution/warehouse/notion/fetch_milestones.rb(1 hunks)src/use_cases_execution/warehouse/notion/fetch_persons.rb(1 hunks)src/use_cases_execution/warehouse/notion/fetch_projects.rb(1 hunks)src/use_cases_execution/warehouse/notion/fetch_weekly_scopes.rb(1 hunks)src/use_cases_execution/warehouse/notion/fetch_work_items.rb(1 hunks)src/use_cases_execution/warehouse/notion/garbage_collector.rb(1 hunks)src/use_cases_execution/warehouse/warehouse_ingester.rb(2 hunks)src/use_cases_execution/warehouse/worklogs/config.rb(0 hunks)src/use_cases_execution/warehouse/worklogs/fetch_work_logs.rb(1 hunks)
💤 Files with no reviewable changes (6)
- src/use_cases_execution/warehouse/google/warehouse_ingester.rb
- src/use_cases_execution/warehouse/github/config.rb
- src/use_cases_execution/warehouse/notion/config.rb
- src/use_cases_execution/warehouse/google_workspace/config.rb
- src/use_cases_execution/warehouse/google/config.rb
- src/use_cases_execution/warehouse/worklogs/config.rb
✅ Files skipped from review due to trivial changes (7)
- spec/use_cases_execution/google_workspace/listen_to_google_calendar_updates_spec.rb
- spec/services/postgres/project_spec.rb
- spec/services/postgres/key_results_history_spec.rb
- src/use_cases_execution/warehouse/notion/garbage_collector.rb
- src/use_cases_execution/warehouse/notion/fetch_hired_persons.rb
- src/use_cases_execution/schedules.rb
- src/use_cases_execution/warehouse/config.rb
🚧 Files skipped from review as they are similar to previous changes (22)
- spec/services/postgres/activity_spec.rb
- spec/services/postgres/weekly_scope_spec.rb
- spec/services/postgres/document_spec.rb
- src/use_cases_execution/warehouse/google_workspace/listen_to_google_docs_updates.rb
- src/use_cases_execution/warehouse/warehouse_ingester.rb
- src/use_cases_execution/warehouse/github/fetch_kommit_co_issues_from_github.rb
- src/use_cases_execution/warehouse/notion/fetch_key_results.rb
- src/use_cases_execution/warehouse/notion/fetch_persons.rb
- src/use_cases_execution/warehouse/google_workspace/listen_to_google_calendar_updates.rb
- src/use_cases_execution/warehouse/github/fetch_kommitters_pull_requests_from_github.rb
- src/use_cases_execution/warehouse/notion/fetch_weekly_scopes.rb
- src/use_cases_execution/warehouse/github/fetch_kommitters_issues_from_github.rb
- src/use_cases_execution/warehouse/notion/fetch_activities.rb
- src/use_cases_execution/warehouse/github/fetch_kommit_co_releases_from_github.rb
- src/use_cases_execution/warehouse/notion/fetch_documents.rb
- src/use_cases_execution/warehouse/worklogs/fetch_work_logs.rb
- src/use_cases_execution/warehouse/notion/fetch_milestones.rb
- src/use_cases_execution/warehouse/notion/fetch_domains.rb
- src/use_cases_execution/warehouse/github/fetch_kommitters_releases_from_github.rb
- src/use_cases_execution/warehouse/notion/fetch_projects.rb
- src/use_cases_execution/warehouse/notion/fetch_work_items.rb
- src/use_cases_execution/warehouse/github/fetch_kommit_co_pull_requests_from_github.rb
🧰 Additional context used
📓 Path-based instructions (1)
**/*.rb
📄 CodeRabbit Inference Engine (CLAUDE.md)
**/*.rb: Use HTTParty for HTTP requests
Do not use Rails conventions; use explicit imports for modules (e.g., Date, DateTime)
Follow Ruby conventions and best practices
Add relevant documentation comments to explain how users can use and understand the code
Add an empty line at the end of files
Use clear variable naming
Implement proper error handling
Files:
spec/use_cases_execution/google_workspace/listen_to_google_docs_updates_spec.rb
🧠 Learnings (9)
📓 Common learnings
Learnt from: CR
PR: kommitters/bas_use_cases#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-21T22:59:03.954Z
Learning: Applies to src/use_cases_execution/schedules.rb : Centralized scheduling configuration should be placed in src/use_cases_execution/schedules.rb
Learnt from: CR
PR: kommitters/bas_use_cases#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-21T22:59:03.954Z
Learning: Applies to src/use_cases_execution/*/config.rb : Database connection configuration must be standardized in each config.rb file within use case directories
📚 Learning: applies to src/use_cases_execution/*/config.rb : database connection configuration must be standardi...
Learnt from: CR
PR: kommitters/bas_use_cases#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-21T22:59:03.954Z
Learning: Applies to src/use_cases_execution/*/config.rb : Database connection configuration must be standardized in each config.rb file within use case directories
Applied to files:
spec/use_cases_execution/google_workspace/listen_to_google_docs_updates_spec.rb
📚 Learning: files under src/utils/warehouse/google_workspace/** should declare the module hierarchy utils::wareh...
Learnt from: cammv21
PR: kommitters/bas_use_cases#175
File: src/utils/warehouse/google_workspace/base.rb:5-7
Timestamp: 2025-07-25T00:18:55.020Z
Learning: Files under src/utils/warehouse/google_workspace/** should declare the module hierarchy Utils::Warehouse::GoogleWorkspace to mirror the directory structure.
Applied to files:
spec/use_cases_execution/google_workspace/listen_to_google_docs_updates_spec.rb
📚 Learning: in the bas_use_cases repository, implementation test files in `spec/implementations/` follow a minim...
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.750Z
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.
Applied to files:
spec/use_cases_execution/google_workspace/listen_to_google_docs_updates_spec.rb
📚 Learning: applies to src/use_cases_execution/*/**/*.rb : all use cases must use bas::sharedstorage with postgr...
Learnt from: CR
PR: kommitters/bas_use_cases#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-21T22:59:03.954Z
Learning: Applies to src/use_cases_execution/*/**/*.rb : All use cases must use Bas::SharedStorage with PostgreSQL as the coordination layer, with each pipeline step reading from and writing to the database using specific tags (FetchX, FormatX, NotifyX, GarbageCollector)
Applied to files:
spec/use_cases_execution/google_workspace/listen_to_google_docs_updates_spec.rb
📚 Learning: applies to src/use_cases_execution/schedules.rb : centralized scheduling configuration should be pla...
Learnt from: CR
PR: kommitters/bas_use_cases#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-21T22:59:03.954Z
Learning: Applies to src/use_cases_execution/schedules.rb : Centralized scheduling configuration should be placed in src/use_cases_execution/schedules.rb
Applied to files:
spec/use_cases_execution/google_workspace/listen_to_google_docs_updates_spec.rb
📚 Learning: applies to spec/implementations/**/*.rb : tests should mirror the implementation structure and be pl...
Learnt from: CR
PR: kommitters/bas_use_cases#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-21T22:59:03.954Z
Learning: Applies to spec/implementations/**/*.rb : Tests should mirror the implementation structure and be placed in spec/implementations/
Applied to files:
spec/use_cases_execution/google_workspace/listen_to_google_docs_updates_spec.rb
📚 Learning: applies to env.yml : environment variables should be configured via env.yml, copying from env.yml.ex...
Learnt from: CR
PR: kommitters/bas_use_cases#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-21T22:59:03.954Z
Learning: Applies to env.yml : Environment variables should be configured via env.yml, copying from env.yml.example
Applied to files:
spec/use_cases_execution/google_workspace/listen_to_google_docs_updates_spec.rb
📚 Learning: applies to src/use_cases_execution/*/**/*.rb : use case orchestration files that wire implementation...
Learnt from: CR
PR: kommitters/bas_use_cases#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-21T22:59:03.954Z
Learning: Applies to src/use_cases_execution/*/**/*.rb : Use case orchestration files that wire implementations together should be placed in src/use_cases_execution/[use_case]/
Applied to files:
spec/use_cases_execution/google_workspace/listen_to_google_docs_updates_spec.rb
🪛 GitHub Actions: CI
spec/use_cases_execution/google_workspace/listen_to_google_docs_updates_spec.rb
[error] 1-1: KeyError: key not found: "WAREHOUSE_POSTGRES_DB" while loading this spec. Missing environment variable required by src/use_cases_execution/warehouse/config.rb.
spec/use_cases_execution/google_workspace/listen_to_google_docs_updates_spec.rb
Outdated
Show resolved
Hide resolved
cammv21
force-pushed
the
unify-warehouse-ingesters
branch
from
821a645 to
c7d717d
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
.env.test (3)
73-73: Remove trailing whitespaceA trailing space sneaks past linters and makes future diffs noisy.
-KOMMIT_CO_GITHUB_APP_ID=KOMMIT_CO_GITHUB_APP_ID +KOMMIT_CO_GITHUB_APP_ID=KOMMIT_CO_GITHUB_APP_ID
10-10: Use consistent placeholder patternLine 10 sets
NOTION_CLOSED_ISSUES_DATABASE_ID=DATABASE_ID, diverging from the “key = key” convention used elsewhere. Recommend aligning it for predictability (and to avoid mistaken check-ins of real IDs).-NOTION_CLOSED_ISSUES_DATABASE_ID=DATABASE_ID +NOTION_CLOSED_ISSUES_DATABASE_ID=NOTION_CLOSED_ISSUES_DATABASE_ID
1-75: (Optional) Alphabetise keys to silencedotenv-linternoiseOrdering does not affect runtime, but keeping the list sorted eliminates 70+ linter warnings and makes variables easier to scan.
No diff provided; apply if the team values the tidy file.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (37)
.env.test(1 hunks)spec/services/postgres/activity_spec.rb(1 hunks)spec/services/postgres/document_spec.rb(1 hunks)spec/services/postgres/key_results_history_spec.rb(1 hunks)spec/services/postgres/project_spec.rb(1 hunks)spec/services/postgres/weekly_scope_spec.rb(1 hunks)spec/use_cases_execution/google_workspace/listen_to_google_calendar_updates_spec.rb(0 hunks)spec/use_cases_execution/google_workspace/listen_to_google_docs_updates_spec.rb(0 hunks)src/use_cases_execution/schedules.rb(1 hunks)src/use_cases_execution/warehouse/config.rb(1 hunks)src/use_cases_execution/warehouse/github/config.rb(0 hunks)src/use_cases_execution/warehouse/github/fetch_kommit_co_issues_from_github.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_kommit_co_releases_from_github.rb(1 hunks)src/use_cases_execution/warehouse/github/fetch_kommitters_issues_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/github/fetch_kommitters_releases_from_github.rb(1 hunks)src/use_cases_execution/warehouse/google/config.rb(0 hunks)src/use_cases_execution/warehouse/google/warehouse_ingester.rb(0 hunks)src/use_cases_execution/warehouse/google_workspace/config.rb(0 hunks)src/use_cases_execution/warehouse/google_workspace/listen_to_google_calendar_updates.rb(2 hunks)src/use_cases_execution/warehouse/google_workspace/listen_to_google_docs_updates.rb(1 hunks)src/use_cases_execution/warehouse/notion/config.rb(0 hunks)src/use_cases_execution/warehouse/notion/fetch_activities.rb(1 hunks)src/use_cases_execution/warehouse/notion/fetch_documents.rb(1 hunks)src/use_cases_execution/warehouse/notion/fetch_domains.rb(1 hunks)src/use_cases_execution/warehouse/notion/fetch_hired_persons.rb(1 hunks)src/use_cases_execution/warehouse/notion/fetch_key_results.rb(1 hunks)src/use_cases_execution/warehouse/notion/fetch_milestones.rb(1 hunks)src/use_cases_execution/warehouse/notion/fetch_persons.rb(1 hunks)src/use_cases_execution/warehouse/notion/fetch_projects.rb(1 hunks)src/use_cases_execution/warehouse/notion/fetch_weekly_scopes.rb(1 hunks)src/use_cases_execution/warehouse/notion/fetch_work_items.rb(1 hunks)src/use_cases_execution/warehouse/notion/garbage_collector.rb(1 hunks)src/use_cases_execution/warehouse/warehouse_ingester.rb(2 hunks)src/use_cases_execution/warehouse/worklogs/config.rb(0 hunks)src/use_cases_execution/warehouse/worklogs/fetch_work_logs.rb(1 hunks)
💤 Files with no reviewable changes (8)
- spec/use_cases_execution/google_workspace/listen_to_google_calendar_updates_spec.rb
- spec/use_cases_execution/google_workspace/listen_to_google_docs_updates_spec.rb
- src/use_cases_execution/warehouse/google_workspace/config.rb
- src/use_cases_execution/warehouse/google/config.rb
- src/use_cases_execution/warehouse/notion/config.rb
- src/use_cases_execution/warehouse/google/warehouse_ingester.rb
- src/use_cases_execution/warehouse/worklogs/config.rb
- src/use_cases_execution/warehouse/github/config.rb
✅ Files skipped from review due to trivial changes (3)
- spec/services/postgres/project_spec.rb
- src/use_cases_execution/schedules.rb
- src/use_cases_execution/warehouse/config.rb
🚧 Files skipped from review as they are similar to previous changes (25)
- spec/services/postgres/activity_spec.rb
- spec/services/postgres/key_results_history_spec.rb
- spec/services/postgres/weekly_scope_spec.rb
- spec/services/postgres/document_spec.rb
- src/use_cases_execution/warehouse/github/fetch_kommitters_pull_requests_from_github.rb
- src/use_cases_execution/warehouse/google_workspace/listen_to_google_calendar_updates.rb
- src/use_cases_execution/warehouse/notion/fetch_milestones.rb
- src/use_cases_execution/warehouse/github/fetch_kommitters_releases_from_github.rb
- src/use_cases_execution/warehouse/notion/fetch_weekly_scopes.rb
- src/use_cases_execution/warehouse/google_workspace/listen_to_google_docs_updates.rb
- src/use_cases_execution/warehouse/notion/garbage_collector.rb
- src/use_cases_execution/warehouse/notion/fetch_key_results.rb
- src/use_cases_execution/warehouse/notion/fetch_work_items.rb
- src/use_cases_execution/warehouse/github/fetch_kommitters_issues_from_github.rb
- src/use_cases_execution/warehouse/warehouse_ingester.rb
- src/use_cases_execution/warehouse/notion/fetch_domains.rb
- src/use_cases_execution/warehouse/github/fetch_kommit_co_releases_from_github.rb
- src/use_cases_execution/warehouse/github/fetch_kommit_co_pull_requests_from_github.rb
- src/use_cases_execution/warehouse/notion/fetch_projects.rb
- src/use_cases_execution/warehouse/notion/fetch_persons.rb
- src/use_cases_execution/warehouse/github/fetch_kommit_co_issues_from_github.rb
- src/use_cases_execution/warehouse/worklogs/fetch_work_logs.rb
- src/use_cases_execution/warehouse/notion/fetch_activities.rb
- src/use_cases_execution/warehouse/notion/fetch_documents.rb
- src/use_cases_execution/warehouse/notion/fetch_hired_persons.rb
🧰 Additional context used
🧠 Learnings (4)
📓 Common learnings
Learnt from: CR
PR: kommitters/bas_use_cases#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-21T22:59:03.954Z
Learning: Applies to src/use_cases_execution/schedules.rb : Centralized scheduling configuration should be placed in src/use_cases_execution/schedules.rb
Learnt from: CR
PR: kommitters/bas_use_cases#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-21T22:59:03.954Z
Learning: Applies to src/use_cases_execution/*/config.rb : Database connection configuration must be standardized in each config.rb file within use case directories
📚 Learning: applies to env.yml : environment variables should be configured via env.yml, copying from env.yml.ex...
Learnt from: CR
PR: kommitters/bas_use_cases#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-21T22:59:03.954Z
Learning: Applies to env.yml : Environment variables should be configured via env.yml, copying from env.yml.example
Applied to files:
.env.test
📚 Learning: in infrastructure/aws/variables.tf, the user juanpabloxk prefers to keep default values for sensitiv...
Learnt from: juanpabloxk
PR: kommitters/bas_use_cases#125
File: infrastructure/aws/variables.tf:30-35
Timestamp: 2025-06-23T18:46:06.922Z
Learning: In infrastructure/aws/variables.tf, the user juanpabloxk prefers to keep default values for sensitive variables like database_password to avoid having to provide them for Terraform operations like `plan` and `destroy` where they might not be necessary for the workflow.
Applied to files:
.env.test
📚 Learning: in the wip limit use cases, `src/use_cases_execution/wip_limit/fetch_domains_wip_count.rb` should us...
Learnt from: kren-prog
PR: kommitters/bas_use_cases#108
File: src/use_cases_execution/wip_limit/fetch_domains_wip_limit.rb:23-23
Timestamp: 2025-06-06T22:44:09.353Z
Learning: In the WIP limit use cases, `src/use_cases_execution/wip_limit/fetch_domains_wip_count.rb` should use `WIP_COUNT_NOTION_DATABASE_ID` while `src/use_cases_execution/wip_limit/fetch_domains_wip_limit.rb` should use `WIP_LIMIT_NOTION_DATABASE_ID`. These are intentionally different environment variables for different purposes.
Applied to files:
.env.test
🪛 Gitleaks (8.27.2)
.env.test
46-46: Discovered a potential Discord client secret, risking compromised Discord bot integrations and data leaks.
(discord-client-secret)
🪛 dotenv-linter (3.3.0)
.env.test
[warning] 5-5: [UnorderedKey] The POSTGRES_PASSWORD key should go before the POSTGRES_USER key
[warning] 7-7: [UnorderedKey] The DISCORD_BOT_NAME key should go before the POSTGRES_DB key
[warning] 8-8: [UnorderedKey] The GITHUB_TOKEN key should go before the POSTGRES_DB key
[warning] 9-9: [UnorderedKey] The NOTION_SECRET key should go before the POSTGRES_DB key
[warning] 10-10: [UnorderedKey] The NOTION_CLOSED_ISSUES_DATABASE_ID key should go before the NOTION_SECRET key
[warning] 11-11: [UnorderedKey] The OSS_NOTION_DATABASE_ID key should go before the POSTGRES_DB key
[warning] 12-12: [UnorderedKey] The BIRTHDAY_TABLE key should go before the DB_HOST key
[warning] 13-13: [UnorderedKey] The BIRTHDAY_NOTION_DATABASE_ID key should go before the BIRTHDAY_TABLE key
[warning] 14-14: [UnorderedKey] The BIRTHDAY_DISCORD_WEBHOOK key should go before the BIRTHDAY_NOTION_DATABASE_ID key
[warning] 15-15: [UnorderedKey] The NEXT_WEEK_BIRTHDAY_DISCORD_WEBHOOK key should go before the NOTION_CLOSED_ISSUES_DATABASE_ID key
[warning] 16-16: [UnorderedKey] The OPENAI_SECRET key should go before the OSS_NOTION_DATABASE_ID key
[warning] 17-17: [UnorderedKey] The PTO_TABLE key should go before the WAREHOUSE_POSTGRES_DB key
[warning] 18-18: [UnorderedKey] The PTO_NOTION_DATABASE_ID key should go before the PTO_TABLE key
[warning] 19-19: [UnorderedKey] The PTO_OPENAI_ASSISTANT key should go before the PTO_TABLE key
[warning] 20-20: [UnorderedKey] The PTO_DISCORD_WEBHOOK key should go before the PTO_NOTION_DATABASE_ID key
[warning] 21-21: [UnorderedKey] The NEXT_WEEK_PTO_OPENAI_ASSISTANT key should go before the NOTION_CLOSED_ISSUES_DATABASE_ID key
[warning] 22-22: [UnorderedKey] The NEXT_WEEK_PTO_DISCORD_WEBHOOK key should go before the NEXT_WEEK_PTO_OPENAI_ASSISTANT key
[warning] 24-24: [UnorderedKey] The WIP_COUNT_NOTION_DATABASE_ID key should go before the WIP_TABLE key
[warning] 25-25: [UnorderedKey] The WIP_LIMIT_NOTION_DATABASE_ID key should go before the WIP_TABLE key
[warning] 26-26: [UnorderedKey] The WIP_LIMIT_DISCORD_WEBHOOK key should go before the WIP_LIMIT_NOTION_DATABASE_ID key
[warning] 27-27: [UnorderedKey] The SUPPORT_EMAIL_TABLE key should go before the WAREHOUSE_POSTGRES_DB key
[warning] 28-28: [UnorderedKey] The SUPPORT_EMAIL_ACCOUNT key should go before the SUPPORT_EMAIL_TABLE key
[warning] 29-29: [UnorderedKey] The SUPPORT_EMAIL_REFRESH_TOKEN key should go before the SUPPORT_EMAIL_TABLE key
[warning] 30-30: [UnorderedKey] The SUPPORT_EMAIL_CLIENT_ID key should go before the SUPPORT_EMAIL_REFRESH_TOKEN key
[warning] 31-31: [UnorderedKey] The SUPPORT_EMAIL_CLIENT_SECRET key should go before the SUPPORT_EMAIL_REFRESH_TOKEN key
[warning] 32-32: [UnorderedKey] The SUPPORT_EMAIL_INBOX key should go before the SUPPORT_EMAIL_REFRESH_TOKEN key
[warning] 33-33: [UnorderedKey] The SUPPORT_EMAIL_RECEPTOR key should go before the SUPPORT_EMAIL_REFRESH_TOKEN key
[warning] 34-34: [UnorderedKey] The SUPPORT_EMAIL_DISCORD_WEBHOOK key should go before the SUPPORT_EMAIL_INBOX key
[warning] 35-35: [UnorderedKey] The REVIEW_IMAGES_TABLE key should go before the SUPPORT_EMAIL_ACCOUNT key
[warning] 36-36: [UnorderedKey] The REVIEW_IMAGE_OPENAI_ASSISTANT key should go before the SUPPORT_EMAIL_ACCOUNT key
[warning] 37-37: [UnorderedKey] The REVIEW_TEXT_TABLE key should go before the SUPPORT_EMAIL_ACCOUNT key
[warning] 38-38: [UnorderedKey] The REVIEW_TEXT_OPENAI_ASSISTANT key should go before the REVIEW_TEXT_TABLE key
[warning] 39-39: [UnorderedKey] The DO_TABLE key should go before the GITHUB_TOKEN key
[warning] 40-40: [UnorderedKey] The DIGITAL_OCEAN_SECRET key should go before the DISCORD_BOT_NAME key
[warning] 41-41: [UnorderedKey] The DIGITAL_OCEAN_THRESHOLD key should go before the DISCORD_BOT_NAME key
[warning] 42-42: [UnorderedKey] The DIGITAL_OCEAN_DISCORD_WEBHOOK key should go before the DIGITAL_OCEAN_SECRET key
[warning] 43-43: [UnorderedKey] The WHATSAPP_WEBHOOK_TOKEN key should go before the WIP_COUNT_NOTION_DATABASE_ID key
[warning] 44-44: [UnorderedKey] The WHATSAPP_TOKEN key should go before the WHATSAPP_WEBHOOK_TOKEN key
[warning] 45-45: [UnorderedKey] The NOTION_EXPIRED_PROJECTS_DATABASE_ID key should go before the NOTION_SECRET key
[warning] 46-46: [UnorderedKey] The EXPIRED_PROJECTS_DISCORD_WEBHOOK key should go before the GITHUB_TOKEN key
[warning] 47-47: [UnorderedKey] The API_SECURITY_SCORECARDS_URL key should go before the BIRTHDAY_DISCORD_WEBHOOK key
[warning] 48-48: [UnorderedKey] The GOOGLE_CHAT_WEBHOOK_BIRTHDAY key should go before the NEXT_WEEK_BIRTHDAY_DISCORD_WEBHOOK key
[warning] 49-49: [UnorderedKey] The GOOGLE_CHAT_WEBHOOK_PTO key should go before the NEXT_WEEK_BIRTHDAY_DISCORD_WEBHOOK key
[warning] 50-50: [UnorderedKey] The EXPIRED_PROJECTS_WORKSPACE_WEBHOOK key should go before the GITHUB_TOKEN key
[warning] 51-51: [UnorderedKey] The WIP_LIMIT_WORKSPACE_WEBHOOK key should go before the WIP_TABLE key
[warning] 52-52: [UnorderedKey] The DIGITAL_OCEAN_WORKSPACE_WEBHOOK key should go before the DISCORD_BOT_NAME key
[warning] 53-53: [UnorderedKey] The SUPPORT_EMAIL_WORKSPACE_WEBHOOK key should go before the WAREHOUSE_POSTGRES_DB key
[warning] 55-55: [UnorderedKey] The WORK_LOGS_API_SECRET key should go before the WORK_LOGS_URL key
[warning] 56-56: [UnorderedKey] The ADMIN_DM_ID key should go before the API_SECURITY_SCORECARDS_URL key
[warning] 57-57: [UnorderedKey] The OPS_DM_ID key should go before the OSS_NOTION_DATABASE_ID key
[warning] 58-58: [UnorderedKey] The ENGINEERING_DM_ID key should go before the EXPIRED_PROJECTS_DISCORD_WEBHOOK key
[warning] 59-59: [UnorderedKey] The BIZDEV_DM_ID key should go before the DB_HOST key
[warning] 60-60: [UnorderedKey] The PROJECTS_NOTION_DATABASE_ID key should go before the PTO_DISCORD_WEBHOOK key
[warning] 61-61: [UnorderedKey] The ACTIVITIES_NOTION_DATABASE_ID key should go before the ADMIN_DM_ID key
[warning] 62-62: [UnorderedKey] The WORK_ITEMS_NOTION_DATABASE_ID key should go before the WORK_LOGS_API_SECRET key
[warning] 63-63: [UnorderedKey] The DOMAINS_NOTION_DATABASE_ID key should go before the DO_TABLE key
[warning] 64-64: [UnorderedKey] The MILESTONES_NOTION_DATABASE_ID key should go before the NEXT_WEEK_BIRTHDAY_DISCORD_WEBHOOK key
[warning] 65-65: [UnorderedKey] The DOCUMENTS_NOTION_DATABASE_ID key should go before the DOMAINS_NOTION_DATABASE_ID key
[warning] 66-66: [UnorderedKey] The WEBHOOK_TOKEN key should go before the WHATSAPP_TOKEN key
[warning] 67-67: [UnorderedKey] The WEEKLY_SCOPES_NOTION_DATABASE_ID key should go before the WHATSAPP_TOKEN key
[warning] 68-68: [UnorderedKey] The PERSONS_NOTION_DATABASE_ID key should go before the POSTGRES_DB key
[warning] 69-69: [UnorderedKey] The KEY_RESULTS_NOTION_DATABASE_ID key should go before the MILESTONES_NOTION_DATABASE_ID key
[warning] 70-70: [UnorderedKey] The HIRED_PEOPLE_NOTION_DATABASE_ID key should go before the KEY_RESULTS_NOTION_DATABASE_ID key
[warning] 71-71: [UnorderedKey] The OPERATON_BASE_URL key should go before the OPS_DM_ID key
[warning] 72-72: [UnorderedKey] The OPERATON_POLLER_WORKER_ID key should go before the OPS_DM_ID key
[warning] 73-73: [TrailingWhitespace] Trailing whitespace detected
[warning] 73-73: [UnorderedKey] The KOMMIT_CO_GITHUB_APP_ID key should go before the MILESTONES_NOTION_DATABASE_ID key
[warning] 74-74: [UnorderedKey] The KOMMITERS_GITHUB_APP_ID key should go before the KOMMIT_CO_GITHUB_APP_ID key
cammv21
force-pushed
the
unify-warehouse-ingesters
branch
from
c7d717d to
8ef5995
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
spec/use_cases_execution/google_workspace/env_helper.rb (2)
34-42: LGTM! Well-implemented file mocking strategy.The RSpec configuration properly mocks PEM key file reads while preserving other file operations. This prevents test dependencies on physical key files.
Consider improving the comment formatting:
-# 4. Mock file reads to avoid dependency on physical .pem files. +# Mock file reads to avoid dependency on physical .pem files.The "4." numbering appears to be a leftover from a different context.
43-43: Add missing empty line at end of file.According to the coding guidelines, Ruby files should end with an empty line.
end +Additionally, consider adding a brief documentation comment at the beginning of the file to explain its purpose:
# frozen_string_literal: true + +# Environment helper for Google Workspace use case execution tests. +# Sets up test environment variables and mocks file dependencies.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (37)
spec/services/postgres/activity_spec.rb(1 hunks)spec/services/postgres/document_spec.rb(1 hunks)spec/services/postgres/key_results_history_spec.rb(1 hunks)spec/services/postgres/project_spec.rb(1 hunks)spec/services/postgres/weekly_scope_spec.rb(1 hunks)spec/use_cases_execution/google_workspace/env_helper.rb(1 hunks)spec/use_cases_execution/google_workspace/listen_to_google_calendar_updates_spec.rb(1 hunks)spec/use_cases_execution/google_workspace/listen_to_google_docs_updates_spec.rb(1 hunks)src/use_cases_execution/schedules.rb(1 hunks)src/use_cases_execution/warehouse/config.rb(1 hunks)src/use_cases_execution/warehouse/github/config.rb(0 hunks)src/use_cases_execution/warehouse/github/fetch_kommit_co_issues_from_github.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_kommit_co_releases_from_github.rb(1 hunks)src/use_cases_execution/warehouse/github/fetch_kommitters_issues_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/github/fetch_kommitters_releases_from_github.rb(1 hunks)src/use_cases_execution/warehouse/google/config.rb(0 hunks)src/use_cases_execution/warehouse/google/warehouse_ingester.rb(0 hunks)src/use_cases_execution/warehouse/google_workspace/config.rb(0 hunks)src/use_cases_execution/warehouse/google_workspace/listen_to_google_calendar_updates.rb(2 hunks)src/use_cases_execution/warehouse/google_workspace/listen_to_google_docs_updates.rb(1 hunks)src/use_cases_execution/warehouse/notion/config.rb(0 hunks)src/use_cases_execution/warehouse/notion/fetch_activities.rb(1 hunks)src/use_cases_execution/warehouse/notion/fetch_documents.rb(1 hunks)src/use_cases_execution/warehouse/notion/fetch_domains.rb(1 hunks)src/use_cases_execution/warehouse/notion/fetch_hired_persons.rb(1 hunks)src/use_cases_execution/warehouse/notion/fetch_key_results.rb(1 hunks)src/use_cases_execution/warehouse/notion/fetch_milestones.rb(1 hunks)src/use_cases_execution/warehouse/notion/fetch_persons.rb(1 hunks)src/use_cases_execution/warehouse/notion/fetch_projects.rb(1 hunks)src/use_cases_execution/warehouse/notion/fetch_weekly_scopes.rb(1 hunks)src/use_cases_execution/warehouse/notion/fetch_work_items.rb(1 hunks)src/use_cases_execution/warehouse/notion/garbage_collector.rb(1 hunks)src/use_cases_execution/warehouse/warehouse_ingester.rb(2 hunks)src/use_cases_execution/warehouse/worklogs/config.rb(0 hunks)src/use_cases_execution/warehouse/worklogs/fetch_work_logs.rb(1 hunks)
💤 Files with no reviewable changes (6)
- src/use_cases_execution/warehouse/google/config.rb
- src/use_cases_execution/warehouse/github/config.rb
- src/use_cases_execution/warehouse/google_workspace/config.rb
- src/use_cases_execution/warehouse/notion/config.rb
- src/use_cases_execution/warehouse/worklogs/config.rb
- src/use_cases_execution/warehouse/google/warehouse_ingester.rb
✅ Files skipped from review due to trivial changes (6)
- spec/services/postgres/activity_spec.rb
- spec/services/postgres/project_spec.rb
- spec/services/postgres/document_spec.rb
- spec/use_cases_execution/google_workspace/listen_to_google_docs_updates_spec.rb
- spec/use_cases_execution/google_workspace/listen_to_google_calendar_updates_spec.rb
- src/use_cases_execution/warehouse/config.rb
🚧 Files skipped from review as they are similar to previous changes (24)
- spec/services/postgres/weekly_scope_spec.rb
- spec/services/postgres/key_results_history_spec.rb
- src/use_cases_execution/warehouse/notion/garbage_collector.rb
- src/use_cases_execution/warehouse/google_workspace/listen_to_google_calendar_updates.rb
- src/use_cases_execution/warehouse/notion/fetch_key_results.rb
- src/use_cases_execution/warehouse/github/fetch_kommit_co_releases_from_github.rb
- src/use_cases_execution/warehouse/notion/fetch_activities.rb
- src/use_cases_execution/warehouse/notion/fetch_weekly_scopes.rb
- src/use_cases_execution/warehouse/github/fetch_kommitters_pull_requests_from_github.rb
- src/use_cases_execution/warehouse/github/fetch_kommitters_releases_from_github.rb
- src/use_cases_execution/warehouse/notion/fetch_projects.rb
- src/use_cases_execution/warehouse/github/fetch_kommit_co_pull_requests_from_github.rb
- src/use_cases_execution/warehouse/google_workspace/listen_to_google_docs_updates.rb
- src/use_cases_execution/warehouse/github/fetch_kommitters_issues_from_github.rb
- src/use_cases_execution/warehouse/notion/fetch_milestones.rb
- src/use_cases_execution/warehouse/warehouse_ingester.rb
- src/use_cases_execution/warehouse/notion/fetch_domains.rb
- src/use_cases_execution/warehouse/notion/fetch_persons.rb
- src/use_cases_execution/schedules.rb
- src/use_cases_execution/warehouse/notion/fetch_documents.rb
- src/use_cases_execution/warehouse/notion/fetch_work_items.rb
- src/use_cases_execution/warehouse/worklogs/fetch_work_logs.rb
- src/use_cases_execution/warehouse/notion/fetch_hired_persons.rb
- src/use_cases_execution/warehouse/github/fetch_kommit_co_issues_from_github.rb
🧰 Additional context used
📓 Path-based instructions (1)
**/*.rb
📄 CodeRabbit Inference Engine (CLAUDE.md)
**/*.rb: Use HTTParty for HTTP requests
Do not use Rails conventions; use explicit imports for modules (e.g., Date, DateTime)
Follow Ruby conventions and best practices
Add relevant documentation comments to explain how users can use and understand the code
Add an empty line at the end of files
Use clear variable naming
Implement proper error handling
Files:
spec/use_cases_execution/google_workspace/env_helper.rb
🧠 Learnings (12)
📓 Common learnings
Learnt from: CR
PR: kommitters/bas_use_cases#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-21T22:59:03.954Z
Learning: Applies to src/use_cases_execution/schedules.rb : Centralized scheduling configuration should be placed in src/use_cases_execution/schedules.rb
Learnt from: CR
PR: kommitters/bas_use_cases#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-21T22:59:03.954Z
Learning: Applies to src/use_cases_execution/*/config.rb : Database connection configuration must be standardized in each config.rb file within use case directories
📚 Learning: in the bas_use_cases repository, implementation test files in `spec/implementations/` follow a minim...
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.750Z
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.
Applied to files:
spec/use_cases_execution/google_workspace/env_helper.rb
📚 Learning: files under src/utils/warehouse/google_workspace/** should declare the module hierarchy utils::wareh...
Learnt from: cammv21
PR: kommitters/bas_use_cases#175
File: src/utils/warehouse/google_workspace/base.rb:5-7
Timestamp: 2025-07-25T00:18:55.020Z
Learning: Files under src/utils/warehouse/google_workspace/** should declare the module hierarchy Utils::Warehouse::GoogleWorkspace to mirror the directory structure.
Applied to files:
spec/use_cases_execution/google_workspace/env_helper.rb
📚 Learning: applies to spec/implementations/**/*.rb : tests should mirror the implementation structure and be pl...
Learnt from: CR
PR: kommitters/bas_use_cases#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-21T22:59:03.954Z
Learning: Applies to spec/implementations/**/*.rb : Tests should mirror the implementation structure and be placed in spec/implementations/
Applied to files:
spec/use_cases_execution/google_workspace/env_helper.rb
📚 Learning: applies to src/use_cases_execution/*/config.rb : database connection configuration must be standardi...
Learnt from: CR
PR: kommitters/bas_use_cases#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-21T22:59:03.954Z
Learning: Applies to src/use_cases_execution/*/config.rb : Database connection configuration must be standardized in each config.rb file within use case directories
Applied to files:
spec/use_cases_execution/google_workspace/env_helper.rb
📚 Learning: applies to src/use_cases_execution/*/**/*.rb : use case orchestration files that wire implementation...
Learnt from: CR
PR: kommitters/bas_use_cases#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-21T22:59:03.954Z
Learning: Applies to src/use_cases_execution/*/**/*.rb : Use case orchestration files that wire implementations together should be placed in src/use_cases_execution/[use_case]/
Applied to files:
spec/use_cases_execution/google_workspace/env_helper.rb
📚 Learning: applies to src/use_cases_execution/*/**/*.rb : all use cases must use bas::sharedstorage with postgr...
Learnt from: CR
PR: kommitters/bas_use_cases#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-21T22:59:03.954Z
Learning: Applies to src/use_cases_execution/*/**/*.rb : All use cases must use Bas::SharedStorage with PostgreSQL as the coordination layer, with each pipeline step reading from and writing to the database using specific tags (FetchX, FormatX, NotifyX, GarbageCollector)
Applied to files:
spec/use_cases_execution/google_workspace/env_helper.rb
📚 Learning: applies to **/*.rb : add relevant documentation comments to explain how users can use and understand...
Learnt from: CR
PR: kommitters/bas_use_cases#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-21T22:59:03.954Z
Learning: Applies to **/*.rb : Add relevant documentation comments to explain how users can use and understand the code
Applied to files:
spec/use_cases_execution/google_workspace/env_helper.rb
📚 Learning: in the bas_use_cases repository, the user juanpabloxk prefers to keep schedule constants consolidate...
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.
Applied to files:
spec/use_cases_execution/google_workspace/env_helper.rb
📚 Learning: applies to env.yml : environment variables should be configured via env.yml, copying from env.yml.ex...
Learnt from: CR
PR: kommitters/bas_use_cases#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-21T22:59:03.954Z
Learning: Applies to env.yml : Environment variables should be configured via env.yml, copying from env.yml.example
Applied to files:
spec/use_cases_execution/google_workspace/env_helper.rb
📚 Learning: applies to src/use_cases_execution/schedules.rb : centralized scheduling configuration should be pla...
Learnt from: CR
PR: kommitters/bas_use_cases#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-21T22:59:03.954Z
Learning: Applies to src/use_cases_execution/schedules.rb : Centralized scheduling configuration should be placed in src/use_cases_execution/schedules.rb
Applied to files:
spec/use_cases_execution/google_workspace/env_helper.rb
📚 Learning: in the bas_use_cases ruby project, methods must not exceed 10 lines due to rubocop standards complia...
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, methods must not exceed 10 lines due to Rubocop standards compliance.
Applied to files:
spec/use_cases_execution/google_workspace/env_helper.rb
🔇 Additional comments (3)
spec/use_cases_execution/google_workspace/env_helper.rb (3)
1-25: LGTM! Comprehensive test environment setup.The environment variable configuration is well-structured and covers all necessary services. The use of
frozen_string_literalfollows Ruby best practices, and the test values are appropriately generic for testing purposes.
27-30: LGTM! Appropriate test dependencies.The required libraries are standard and necessary for testing Ruby web applications with RSpec.
32-32: LGTM! Correctly references centralized configuration.The require statement properly references the new centralized warehouse configuration module, aligning with the PR's consolidation objectives.
cammv21
force-pushed
the
unify-warehouse-ingesters
branch
from
8ef5995 to
d8b77ff
Compare
cammv21
force-pushed
the
unify-warehouse-ingesters
branch
2 times, most recently
from
45c7363 to
c406722
Compare
cammv21
force-pushed
the
unify-warehouse-ingesters
branch
from
c406722 to
e69c6c3
Compare
juanpabloxk
left a comment
juanpabloxk
left a comment
There was a problem hiding this comment.
Looks good to me. Great refactor 👌🏼
Description
This PR resolves the technical debt outlined in issue #194 by performing a major refactoring of the warehouse ingestion system. The primary motivation is to improve maintainability, reduce code duplication, and create a scalable foundation for adding new data sources.
The changes can be broken down into two main efforts:
1. Configuration Centralization:
2. Ingester Logic Unification:
Fixes #194
Type of change
Please delete options that are not relevant.
Checklist
Summary by CodeRabbit
New Features
Refactor
Chores
Style