-
Notifications
You must be signed in to change notification settings - Fork 32
Support overriding the version of fields #41
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
WalkthroughThis change introduces a mechanism to override the API version specifically for fields-related operations within the Fields module. Two new class methods, Sequence Diagram(s)sequenceDiagram
participant Client
participant ActivityResource
participant FieldsModule
participant PipedriveAPI
Client->>ActivityResource: Call general operation (e.g., list activities)
ActivityResource->>PipedriveAPI: Request with API version v2
PipedriveAPI-->>ActivityResource: Response
ActivityResource-->>Client: Return data
Client->>ActivityResource: Call fields operation (e.g., Activity.fields)
ActivityResource->>FieldsModule: fields()
FieldsModule->>PipedriveAPI: Request with API version v1 (overridden)
PipedriveAPI-->>FieldsModule: Response
FieldsModule-->>ActivityResource: Return fields data
ActivityResource-->>Client: Return fields data
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~15 minutes Possibly related PRs
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🧰 Additional context used🧠 Learnings (2)📓 Common learnings📚 Learning: for the pipedrive connect library, api version management should be implemented on a per-resource ba...Applied to files:
🧬 Code Graph Analysis (1)lib/pipedrive/fields.rb (2)
🔇 Additional comments (5)
✨ Finishing Touches
🧪 Generate unit tests
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (5)
CHANGELOG.md(1 hunks)lib/pipedrive/fields.rb(3 hunks)lib/pipedrive/resources/activity.rb(1 hunks)lib/pipedrive/version.rb(1 hunks)spec/lib/pipedrive/activity_spec.rb(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: foomip
PR: getonbrd/pipedrive-connect#37
File: lib/pipedrive/api_operations/request.rb:11-13
Timestamp: 2025-04-01T14:03:34.481Z
Learning: For the Pipedrive Connect library, API version management should be implemented on a per-resource basis (by overriding the `api_version` method in individual resource classes) rather than using global configuration, to maintain consistency and ensure proper testing of each resource with newer API versions.
📚 Learning: for the pipedrive connect library, api version management should be implemented on a per-resource ba...
Learnt from: foomip
PR: getonbrd/pipedrive-connect#37
File: lib/pipedrive/api_operations/request.rb:11-13
Timestamp: 2025-04-01T14:03:34.481Z
Learning: For the Pipedrive Connect library, API version management should be implemented on a per-resource basis (by overriding the `api_version` method in individual resource classes) rather than using global configuration, to maintain consistency and ensure proper testing of each resource with newer API versions.
Applied to files:
lib/pipedrive/version.rbspec/lib/pipedrive/activity_spec.rblib/pipedrive/resources/activity.rbCHANGELOG.mdlib/pipedrive/fields.rb
🧬 Code Graph Analysis (2)
lib/pipedrive/resources/activity.rb (1)
lib/pipedrive/fields.rb (1)
use_fields_version(14-16)
lib/pipedrive/fields.rb (5)
lib/pipedrive/api_operations/request.rb (4)
api_version(16-18)supports_v2_api?(11-14)request(31-49)request(73-75)lib/pipedrive.rb (1)
api_version(47-49)lib/pipedrive/resources/activity.rb (1)
supports_v2_api?(13-15)lib/pipedrive/resources/deal.rb (1)
supports_v2_api?(11-13)lib/pipedrive/resources/product.rb (1)
supports_v2_api?(7-9)
🪛 LanguageTool
CHANGELOG.md
[grammar] ~8-~8: Use correct spacing
Context: ...p://keepachangelog.com/). ## [2.1.0] - 2025-08-06 ### Added - New use_fields_version method ...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~11-~11: There might be a mistake here.
Context: ...ide API version specifically for fields operations - Fields-specific version override functio...
(QB_NEW_EN_OTHER)
[grammar] ~12-~12: Use periods with abbreviations
Context: ...o use different API versions for fields vs general operations - `fields_api_versio...
(QB_NEW_EN_OTHER_ERROR_IDS_34)
[grammar] ~12-~12: There might be a mistake here.
Context: ...rent API versions for fields vs general operations - fields_api_version method to query the fields-specific API versio...
(QB_NEW_EN_OTHER)
[grammar] ~13-~13: There might be a problem here.
Context: ...method to query the fields-specific API version ### Changed - Activity resource now uses `use_field...
(QB_NEW_EN_MERGED_MATCH)
[grammar] ~16-~16: Make sure to use plural and singular nouns correctly
Context: ...rsion :v1` to override version only for fields operations - Activity general operations now use v2 A...
(QB_NEW_EN_OTHER_ERROR_IDS_10)
[grammar] ~17-~17: There might be a mistake here.
Context: ... Activity general operations now use v2 API while fields operations continue to use...
(QB_NEW_EN_OTHER)
[grammar] ~17-~17: Make sure to use plural and singular nouns correctly
Context: ...general operations now use v2 API while fields operations continue to use v1 API as re...
(QB_NEW_EN_OTHER_ERROR_IDS_10)
[grammar] ~17-~17: There might be a mistake here.
Context: ...s continue to use v1 API as required by Pipedrive ## [2.0.1] - 2025-04-17 - Fix bugs introdu...
(QB_NEW_EN_OTHER)
🪛 markdownlint-cli2 (0.17.2)
CHANGELOG.md
10-10: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
11-11: Lists should be surrounded by blank lines
(MD032, blanks-around-lists)
15-15: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
16-16: Lists should be surrounded by blank lines
(MD032, blanks-around-lists)
🪛 RuboCop (1.76.1)
lib/pipedrive/fields.rb
[convention] 66-66: private should be inlined in method definitions.
(Style/AccessModifierDeclarations)
🔇 Additional comments (7)
lib/pipedrive/version.rb (1)
4-4: LGTM: Appropriate semantic version bumpThe minor version increment correctly reflects the addition of new backward-compatible functionality for fields version overrides.
lib/pipedrive/resources/activity.rb (1)
6-10: Excellent implementation with clear documentationThe addition of
use_fields_version :v1with explanatory comments and API documentation reference makes the intent clear. This properly implements the new fields version override mechanism while maintaining v2 for general operations.spec/lib/pipedrive/activity_spec.rb (1)
32-80: Excellent comprehensive test coverageThe test suite thoroughly validates the new fields version override functionality with proper coverage of:
- API version behavior under different global settings
- Independent fields version override
- Method availability verification
- Clear context separation and descriptive test names
This provides strong confidence in the implementation's correctness.
lib/pipedrive/fields.rb (4)
13-16: Well-implemented version override setterThe
use_fields_versionmethod provides a clean interface for setting fields-specific API versions, following established patterns in the codebase.
18-28: Solid API version override implementationThe
api_versionoverride maintains backward compatibility while allowing version customization. The fallback logic correctly handles different scenarios and preserves the originalsupports_v2_api?behavior.
30-33: Clean fields version query methodThe
fields_api_versionmethod provides a clear interface for querying the fields-specific version, falling back appropriately to the general API version.
43-43: Good use of specialized request methodUsing
fields_requestinstead of directrequestcall properly implements the version override mechanism for fields operations.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
♻️ Duplicate comments (1)
lib/pipedrive/fields.rb (1)
63-63: Consider inlining the private modifier per Ruby conventions.The private modifier should be inlined with the method definition according to Ruby style conventions.
Apply this diff to inline the private modifier:
- # Fields-specific request method that uses fields_api_version - private def fields_request(method, url, params = {}) + # Fields-specific request method that uses fields_api_version + private def fields_request(method, url, params = {})
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
lib/pipedrive/fields.rb(3 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: foomip
PR: getonbrd/pipedrive-connect#37
File: lib/pipedrive/api_operations/request.rb:11-13
Timestamp: 2025-04-01T14:03:34.481Z
Learning: For the Pipedrive Connect library, API version management should be implemented on a per-resource basis (by overriding the `api_version` method in individual resource classes) rather than using global configuration, to maintain consistency and ensure proper testing of each resource with newer API versions.
📚 Learning: for the pipedrive connect library, api version management should be implemented on a per-resource ba...
Learnt from: foomip
PR: getonbrd/pipedrive-connect#37
File: lib/pipedrive/api_operations/request.rb:11-13
Timestamp: 2025-04-01T14:03:34.481Z
Learning: For the Pipedrive Connect library, API version management should be implemented on a per-resource basis (by overriding the `api_version` method in individual resource classes) rather than using global configuration, to maintain consistency and ensure proper testing of each resource with newer API versions.
Applied to files:
lib/pipedrive/fields.rb
🔇 Additional comments (4)
lib/pipedrive/fields.rb (4)
13-16: LGTM! Clean implementation of fields version override.The method provides a clear interface for setting fields-specific API versions, aligning with the per-resource version management approach mentioned in the retrieved learnings.
18-24: Well-designed version override with proper fallback.The method correctly prioritizes explicit version settings while maintaining backward compatibility through the super fallback. This supports the flexible version management needed for the fields functionality.
26-29: Correct version resolution hierarchy.The method appropriately prioritizes the fields-specific version over the general API version, providing clear and predictable behavior for version resolution.
39-39: Clean delegation to version-aware request method.The change appropriately delegates to the specialized
fields_requestmethod while maintaining the same interface and behavior for the public API.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
shesho
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Address issue #40
Summary by CodeRabbit
New Features
Bug Fixes
Tests
Documentation
Chores