Fix broken endpoints #39
Conversation
|
Warning Rate limit exceeded@j4rs has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 6 minutes and 29 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 (2)
WalkthroughThe changes update several aspects of the codebase related to API base URL construction, test coverage, and development tooling. The Sequence Diagram(s)sequenceDiagram
participant Client
participant ResourceClass
participant RequestClass
Client->>ResourceClass: Call .api_base_url
alt supports v2 API
ResourceClass->>RequestClass: .api_base_url (returns BASE_URL + "/api/v2")
else does not support v2 API
ResourceClass->>RequestClass: .api_base_url (returns BASE_URL + "/v1")
end
RequestClass-->>ResourceClass: Return constructed base URL
ResourceClass-->>Client: Return base URL
Possibly related PRs
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (7)
.gitignore(1 hunks)lib/pipedrive.rb(1 hunks)lib/pipedrive/api_operations/request.rb(2 hunks)spec/lib/pipedrive/activity_spec.rb(1 hunks)spec/lib/pipedrive/pipedrive_spec.rb(1 hunks)spec/lib/pipedrive/resource_spec.rb(1 hunks)spec/spec_helper.rb(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (2)
lib/pipedrive/api_operations/request.rb (1)
lib/pipedrive.rb (1)
api_version(47-49)
spec/lib/pipedrive/resource_spec.rb (1)
lib/pipedrive/api_operations/request.rb (1)
api_base_url(25-27)
🪛 RuboCop (1.73)
lib/pipedrive/api_operations/request.rb
[convention] 21-21: Add empty line after guard clause.
(Layout/EmptyLineAfterGuardClause)
🔇 Additional comments (8)
.gitignore (1)
13-13: Good addition to gitignoreAdding
.byebug_historyto gitignore is an appropriate practice when using the byebug debugger, ensuring debug artifacts don't get committed to version control.spec/spec_helper.rb (1)
9-9: Appropriate addition of byebug for testingAdding the byebug require statement is helpful for debugging during test development. This complements the gitignore update to exclude the debug history file.
lib/pipedrive.rb (1)
27-27: This change addresses the core endpoint path issueModifying the
BASE_URLconstant by removing the/apisegment is a key part of fixing the endpoint discrepancy described in the PR. This change helps ensure that version 1 of the API correctly uses endpoints prefixed with/v1/...instead of the incorrect/api/v1.Let's verify this change works with the existing API version handling:
#!/bin/bash # Search for how this BASE_URL is used in conjunction with API version prefixes rg -A 5 "BASE_URL" --type rubyspec/lib/pipedrive/activity_spec.rb (1)
33-33: Fixed test description to match actual behaviorThis change corrects the test description to accurately reflect what's being tested. The test asserts that the API version is
:v2, but the description incorrectly stated "returns v1 api version". Good catch to improve test clarity.spec/lib/pipedrive/pipedrive_spec.rb (1)
117-117: Updated hash syntax in error message expectationThis change updates the expected error data string format from Ruby's older hashrocket syntax (
{:abc=>123}) to the newer symbol key syntax ({abc: 123}). This reflects modern Ruby coding style.Let's verify this matches how errors are actually formatted in the codebase:
#!/bin/bash # Look for how error data is formatted in the error classes or handlers rg "error.*data.*hash" --type rubyspec/lib/pipedrive/resource_spec.rb (1)
34-54: Tests for the new URL construction logic look good.The new test suite for
api_base_urlproperly covers both scenarios for API version construction. It verifies that resources supporting v2 API receive the correct URL prefixapi/v2and those that don't support v2 API correctly use justv1. This aligns perfectly with the PR's objective to fix the broken endpoints and ensure compatibility with version 1 of the API.lib/pipedrive/api_operations/request.rb (2)
25-27: Base URL construction looks correct.This method properly combines the
BASE_URLconstant with the appropriate version prefix, ensuring that v1 API calls use "/v1/" and v2 API calls use "/api/v2" as intended by the PR.
51-51: URL construction has been centralized correctly.Updated to use the new
api_base_urlmethod instead of directly constructing the URL. This change aligns with the PR's objective to fix endpoint paths and ensures consistent URL construction throughout the codebase.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Lite
📒 Files selected for processing (1)
.github/workflows/ci.yml(1 hunks)
🔇 Additional comments (5)
.github/workflows/ci.yml (5)
1-2: Workflow naming is descriptive
The workflow name "Run tests" clearly conveys its purpose.
8-11: Job configuration is solid
Usingruns-on: ubuntu-latestis standard for cross‑platform CI.
12-14: Checkout step is up-to-date
actions/checkout@v4correctly checks out the repository code.
21-23: RSpec invocation is correct
Runningbundle exec rspecwill execute your test suite. If you later introduce other test tasks (linting, coverage), you can chain them here.
3-7:❓ Verification inconclusive
Verify branch triggers align with your default branch
The workflow triggers onpushtomaster. If your repository’s default branch ismain(or others), consider adding it here to ensure tests run on all primary branches.You can update the triggers as follows:
on: - pull_request: - push: - branches: [master] + pull_request: + push: + branches: [master, main]Please confirm your repo’s default branch name(s) and adjust accordingly.
Align CI workflow triggers with your repository’s default branch
Thepushtrigger currently only includesmaster. If your default branch ismain(or another name), add it here so CI runs on all primary branches:on: - pull_request: - push: - branches: [master] + pull_request: + push: + branches: [master, main]Please confirm your repo’s default branch name(s) and adjust the
brancheslist accordingly.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
.github/workflows/ci.yml (1)
1-28: 🧹 Nitpick (assertive)Great job setting up the CI workflow!
The GitHub Actions workflow is well-structured and follows good practices:
- Triggers on both PRs and pushes to master
- Tests against both Ruby 2.7 (older) and 3.4 (newer)
- Uses bundler caching for faster builds
- Has clear step names and organization
Consider adding an intermediate Ruby version (like 3.0 or 3.1) to your matrix for broader compatibility coverage:
- ruby-version: [2.7, 3.4] + ruby-version: [2.7, 3.0, 3.4]
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Lite
📒 Files selected for processing (2)
.github/workflows/ci.yml(1 hunks)lib/pipedrive/api_operations/request.rb(2 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
lib/pipedrive/api_operations/request.rb (1)
lib/pipedrive.rb (1)
api_version(47-49)
🔇 Additional comments (2)
lib/pipedrive/api_operations/request.rb (2)
26-28: Good URL construction patternThis method appropriately centralizes the base URL construction logic, which makes the code more maintainable and addresses the endpoint path issue.
52-52: Correct refactoring of the API clientGood job refactoring the API client to use the new
api_base_urlmethod instead of manually constructing the URL. This change ensures consistent URL construction throughout the codebase.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Lite
📒 Files selected for processing (2)
CHANGELOG.md(1 hunks)lib/pipedrive/version.rb(1 hunks)
🔇 Additional comments (1)
lib/pipedrive/version.rb (1)
4-4: Bump version constant to 2.0.1
The version has been correctly updated from 2.0.0 to 2.0.1 to reflect the v1 API endpoint fix and CI workflow addition.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
V1 uses
/v1/...instead of/api/v1, so version 2.0 of this gem is breaking v1.Summary by CodeRabbit
Bug Fixes
Refactor
Tests
Chores
.gitignoreto exclude.byebug_historyfiles.Documentation