feat: persons endpoint #18
Conversation
WalkthroughThis update enhances the codebase by introducing automatic draft reviews and refining pagination handling. Type definitions have been improved for clarity and safety, along with new utility types for flexible data structures. Documentation has been expanded, and testing has been bolstered to ensure robustness. Collectively, these changes streamline processes and improve API functionality, enhancing usability and maintainability. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant API
participant DraftReviewSystem
participant Paginator
User->>API: Submit draft
API->>DraftReviewSystem: Review draft automatically
DraftReviewSystem-->>API: Return review results
API-->>User: Send response with review results
User->>API: Request paginated data
API->>Paginator: Fetch data with page token
Paginator-->>API: Return paginated response
API-->>User: Send paginated data response
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 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 as PR comments)
Additionally, you can add CodeRabbit Configuration File (
|
|
@coderabbitai review |
Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files ignored due to path filters (1)
src/v1/tests/__snapshots__/persons_test.ts.snapis excluded by!**/*.snap
Files selected for processing (21)
- .coderabbit.yaml (1 hunks)
- .yamlfmt (1 hunks)
- README.md (1 hunks)
- src/v1/auth.ts (3 hunks)
- src/v1/create_search_iterator_fn.ts (1 hunks)
- src/v1/field_value_changes.ts (2 hunks)
- src/v1/field_values.ts (2 hunks)
- src/v1/index.ts (3 hunks)
- src/v1/list_entries.ts (7 hunks)
- src/v1/organizations.ts (12 hunks)
- src/v1/paged_request.ts (1 hunks)
- src/v1/paged_response.ts (1 hunks)
- src/v1/persons.ts (2 hunks)
- src/v1/tests/fixtures/persons/get.raw.response.json (1 hunks)
- src/v1/tests/fixtures/persons/paginated.iterator.combined.response.json (1 hunks)
- src/v1/tests/fixtures/persons/search.raw.response.json (1 hunks)
- src/v1/tests/persons_test.ts (1 hunks)
- src/v1/transform_interaction_date_response_raw.ts (1 hunks)
- src/v1/transform_list_entry_reference.ts (1 hunks)
- src/v1/types.ts (1 hunks)
- src/v1/urls.ts (1 hunks)
Files skipped from review due to trivial changes (3)
- src/v1/paged_response.ts
- src/v1/tests/fixtures/persons/get.raw.response.json
- src/v1/tests/fixtures/persons/search.raw.response.json
Additional comments not posted (52)
.coderabbit.yaml (1)
1-3: LGTM!The configuration for enabling drafts in auto reviews is appropriate.
.yamlfmt (1)
2-2: LGTM!The configuration for ensuring end-of-file newlines is beneficial for maintaining coding standards.
src/v1/transform_list_entry_reference.ts (1)
1-12: LGTM!The function
transformListEntryReferenceis correctly implemented using TypeScript generics and type assertions. The date transformation is handled appropriately.src/v1/types.ts (2)
6-6: LGTM!The
Replacetype definition is correct and effectively uses TypeScript's utility types.
11-17: LGTM!The
RequireOnlyOnetype definition is complex but correct, leveraging advanced TypeScript features effectively.src/v1/paged_request.ts (1)
3-16: LGTM!The
PagedRequesttype definition is correct and effectively defines properties for pagination.src/v1/transform_interaction_date_response_raw.ts (1)
14-41: LGTM!The
transformInteractionDateResponseRawfunction is correct and effectively transforms the raw interaction date responses.src/v1/tests/fixtures/persons/paginated.iterator.combined.response.json (1)
1-51: JSON structure and data look good!The JSON structure is correct and the data is appropriate for testing pagination.
src/v1/urls.ts (2)
74-76: URL construction logic looks good!The
personsUrlfunction correctly constructs URLs for accessing person resources.
82-82: Function is consistent with existing patterns.The
personFieldsUrlfunction is consistent with the existing pattern of URL construction functions.src/v1/index.ts (2)
9-9: Import statement forPersonslooks good.The import statement is correctly added and consistent with the rest of the imports.
55-55: Addition and initialization ofpersonsproperty look good.The
personsproperty is correctly added as a public readonly member and initialized in the constructor.Also applies to: 72-72
src/v1/auth.ts (3)
5-5: Import statement forDateTimeandReplacelooks good.The import statement is correctly added and consistent with the rest of the imports.
40-43: Addition ofGrantRawtype looks good.The
GrantRawtype is correctly added to encapsulate properties of the grant object and follows best practices.
46-46: Modifications toWhoAmIResponseRawandWhoAmIResponsetypes look good.The type modifications are correctly applied and follow best practices.
Also applies to: 58-59, 61-67
README.md (2)
35-35: Status change ofPersonsentity looks good.The status change from ❌ to ✅ correctly reflects the current state of the entity.
36-36: Status change ofOpportunitiesentity looks good.The status change from ✅ to ❌ correctly reflects the current state of the entity.
src/v1/tests/persons_test.ts (3)
36-44: LGTM! Verify the accuracy of mock data and snapshots.The test case for searching persons is well-structured. Ensure that the mock data in
persons/search.raw.response.jsonand the snapshot validation are accurate.Verification successful
LGTM! The test case for searching persons is well-structured. The mock data and snapshot validation are accurate.
- Mock Data:
search.raw.response.json- Snapshot:
persons_test.ts.snapScripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the accuracy of mock data and snapshots. # Test: Check the existence and content of the mock data file. fd 'persons/search.raw.response.json' | xargs cat # Test: Check the existence and content of the snapshot file. fd 'snapshots/persons_test.ts' | xargs catLength of output: 92
Script:
#!/bin/bash # Description: Verify the accuracy of mock data and snapshots. # Test: Search for the mock data file and display its content if found. fd 'search.raw.response.json' | xargs cat # Test: Search for the snapshot file and display its content if found. fd 'persons_test.ts.snap' | xargs catLength of output: 3892
111-155: LGTM! Verify the accuracy of mock data and snapshots.The test case for iterating over all persons using pagination is well-structured. Ensure that the mock data in
persons/paginated.iterator.combined.response.jsonand the snapshot validation are accurate.Verification successful
LGTM! The mock data and snapshot files are accurate and relevant to the test case.
The test case for iterating over all persons using pagination is well-structured, and the mock data in
src/v1/tests/fixtures/persons/paginated.iterator.combined.response.jsonis appropriate for the test. The snapshot validation insrc/v1/tests/persons_test.tsis correctly implemented.
- Mock data file:
src/v1/tests/fixtures/persons/paginated.iterator.combined.response.json- Snapshot file:
src/v1/tests/persons_test.tsScripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the accuracy of mock data and snapshots. # Test: Check the existence and content of the mock data file. fd 'persons/paginated.iterator.combined.response.json' | xargs cat # Test: Check the existence and content of the snapshot file. fd 'snapshots/persons_test.ts' | xargs catLength of output: 109
Script:
#!/bin/bash # Description: Verify the existence and content of the mock data and snapshots. # Test: Check the existence and content of the mock data file. mock_data_file=$(fd 'persons/paginated.iterator.combined.response.json') if [ -n "$mock_data_file" ]; then echo "Mock data file found: $mock_data_file" cat "$mock_data_file" else echo "Mock data file not found." fi # Test: Check the existence and content of the snapshot file. snapshot_file=$(fd 'snapshots/persons_test.ts') if [ -n "$snapshot_file" ]; then echo "Snapshot file found: $snapshot_file" cat "$snapshot_file" else echo "Snapshot file not found." fiLength of output: 274
Script:
#!/bin/bash # Description: Search for all JSON files and snapshot files in the repository. # Search for all JSON files to locate the mock data file. echo "Searching for all JSON files in the repository..." fd --extension json # Search for all snapshot files to locate the snapshot file. echo "Searching for all snapshot files in the repository..." fd --extension ts --exec grep -l 'assertSnapshot'Length of output: 2537
Script:
#!/bin/bash # Description: Display the content of the identified mock data and snapshot files. # Display the content of the mock data file. echo "Content of the mock data file:" cat src/v1/tests/fixtures/persons/paginated.iterator.combined.response.json # Display the content of the snapshot file. echo "Content of the snapshot file:" cat src/v1/tests/persons_test.tsLength of output: 6687
26-34: LGTM! Verify the accuracy of mock data and snapshots.The test case for fetching a specific person is well-structured. Ensure that the mock data in
persons/get.raw.response.jsonand the snapshot validation are accurate.Verification successful
LGTM! The mock data and snapshot validation are accurate.
The test case for fetching a specific person is well-structured, and the mock data in
persons/get.raw.response.jsonand the snapshot validation in__snapshots__/persons_test.ts.snapare consistent and correct.
- Mock data file:
src/v1/tests/fixtures/persons/get.raw.response.json- Snapshot file:
src/v1/tests/__snapshots__/persons_test.ts.snapScripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the accuracy of mock data and snapshots. # Test: Check the existence and content of the mock data file. fd 'persons/get.raw.response.json' | xargs cat # Test: Check the existence and content of the snapshot file. fd 'snapshots/persons_test.ts' | xargs catLength of output: 89
Script:
#!/bin/bash # Description: Verify the existence and content of mock data and snapshot files with more context. # Search for the mock data file and print its path if found. fd 'get.raw.response.json' -a # Search for the snapshot file and print its path if found. fd 'persons_test.ts' -aLength of output: 392
Script:
#!/bin/bash # Description: Verify the content of mock data and snapshot files. # Print the content of the mock data file. cat /home/jailuser/git/src/v1/tests/fixtures/persons/get.raw.response.json # Print the content of the snapshot file. cat /home/jailuser/git/src/v1/tests/__snapshots__/persons_test.ts.snapLength of output: 3400
src/v1/field_value_changes.ts (2)
82-84: LGTM! The type definition change improves type safety.The updated
FieldValueChangetype definition using theReplaceutility type is well-structured and improves type safety by replacing thechanged_atproperty with aDatetype.
Line range hint
74-81: LGTM! The type definition is correct.The
GetFieldValueChangesRequesttype definition correctly uses theRequireOnlyOneutility type to enforce that only one of the specified properties can be present at a time.src/v1/field_values.ts (2)
6-6: LGTM! The import change is correct.The
Replacetype has been correctly added to the imports and is used in the updated type definitions.
158-165: LGTM! The type definition change improves type safety.The updated
FieldValuetype definition using theReplaceutility type is well-structured and improves type safety by replacing theupdated_atandcreated_atproperties with new types.src/v1/list_entries.ts (5)
77-79: LGTM!The transformation of
created_atto aDateusing theReplacetype enhances type safety.
81-83: LGTM!The transformation of
list_entriesto an array ofListEntryResponseusing theReplacetype ensures consistency.
208-212: LGTM!The refactor to use
transformListEntryReferencesimplifies the transformation process and ensures consistency.
241-241: LGTM!The refactor to use
transformListEntryReferencesimplifies the transformation process and ensures consistency.
338-338: LGTM!The refactor to use
transformListEntryReferencesimplifies the transformation process and ensures consistency.src/v1/persons.ts (13)
42-67: LGTM!The introduction of
PersonResponseRawenhances the data model and provides clarity.
69-69: LGTM!The use of
Replacetype for incorporatingInteractionDateResponseensures a dynamic and context-aware representation of person data.
71-76: LGTM!The expansion of
WithCurrentOrganizatonParamsadds flexibility to the API interactions.
78-91: LGTM!The expansion of
SearchPersonsRequestenhances the functionality of the search API.
93-97: LGTM!The expansion of
PagedPersonResponseRawimproves the structure of paginated responses.
99-101: LGTM!The transformation of
personsto an array ofPersonResponseusing theReplacetype ensures consistency.
103-107: LGTM!The expansion of
SinglePersonResponseRawimproves the structure of single person responses.
109-116: LGTM!The expansion of
SinglePersonResponseimproves the structure of single person responses.
118-122: LGTM!The expansion of
GetPersonRequestenhances the flexibility of the get person API.
124-127: LGTM!The expansion of
PersonReferenceimproves the structure of person references.
168-194: LGTM!The update to handle
SinglePersonResponseRawand transform it toSinglePersonResponseensures that the response format aligns with the latest data models.
216-240: LGTM!The update to handle
PagedPersonResponseRawand transform it toPagedPersonResponseensures that the response format aligns with the latest data models.
261-261: LGTM!The refactor to use
createSearchIteratorFnsimplifies the logic for iterating over search results.src/v1/organizations.ts (10)
55-63: LGTM!The introduction of
OpportunityIdResponseRawenhances the data model and provides clarity.
74-94: LGTM!The update to include detailed interaction date information allows for more precise data modeling.
96-117: LGTM!The update to include detailed interaction date information allows for more precise data modeling.
135-136: LGTM!The expansion to include
OpportunityIdResponseRawandInteractionDateResponseRawenhances the data model and provides clarity.
159-161: LGTM!The transformation of
dateto aDateusing theReplacetype enhances type safety.
163-166: LGTM!The use of
Replacetype for incorporatingInteractionDateResponseensures a dynamic and context-aware representation of organization data.
172-176: LGTM!The expansion of
PagedOrganizationResponseRawimproves the structure of paginated responses.
178-180: LGTM!The transformation of
organizationsto an array ofOrganizationResponseusing theReplacetype ensures consistency.
188-189: LGTM!The expansion of
SingleOrganizationResponseRawimproves the structure of single organization responses.
191-195: LGTM!The expansion of
SingleOrganizationResponseimproves the
| @@ -0,0 +1,16 @@ | |||
| // TODO(@joscha): see if we need to unify some of this with the `PagingParameters`. | |||
There was a problem hiding this comment.
Reminder: Unify with PagingParameters.
The TODO comment indicates a potential future improvement to unify this with PagingParameters.
Do you want me to open a GitHub issue to track this task?
| export const createSearchIteratorFn = < | ||
| FN extends (r: PAGED_REQUEST) => Promise<PAGED_RESPONSE>, | ||
| PAGED_REQUEST extends PagedRequest, | ||
| PAGED_RESPONSE extends | ||
| & PagedResponse | ||
| & Record<PAYLOAD_KEY, SINGLE_RESPONSE[]>, | ||
| PAYLOAD_KEY extends string = | ||
| & keyof Omit<PAGED_RESPONSE, keyof PagedResponse> | ||
| & string, | ||
| SINGLE_RESPONSE = object, | ||
| >(searchFn: FN, key: PAYLOAD_KEY) => { | ||
| async function* searchIterator( | ||
| params: Omit<PAGED_REQUEST, 'page_token'>, | ||
| ): AsyncGenerator<PAGED_RESPONSE[]> { | ||
| let page_token: string | undefined = undefined | ||
| while (true) { | ||
| const response: PAGED_RESPONSE = await searchFn( | ||
| // TODO(@joscha): remove cast | ||
| (page_token | ||
| ? { ...params, page_token } | ||
| : params) as PAGED_REQUEST, | ||
| ) | ||
|
|
||
| // TODO(@joscha): remove cast | ||
| yield response[key] as unknown as PAGED_RESPONSE[] | ||
|
|
||
| if (response.next_page_token === null) { | ||
| // no more pages to fetch | ||
| return | ||
| } else { | ||
| page_token = response.next_page_token | ||
| } | ||
| } | ||
| } | ||
| return searchIterator | ||
| } |
There was a problem hiding this comment.
Consider removing TODO comments or addressing them.
There are TODO comments indicating casts that need to be removed. Ensure these are addressed before finalizing the code.
- // TODO(@joscha): remove cast
- (page_token
- ? { ...params, page_token }
- : params) as PAGED_REQUEST,
+ const requestParams: PAGED_REQUEST = page_token
+ ? { ...params, page_token }
+ : params;
+ const response: PAGED_RESPONSE = await searchFn(requestParams);- // TODO(@joscha): remove cast
- yield response[key] as unknown as PAGED_RESPONSE[]
+ const responsePayload: PAGED_RESPONSE[] = response[key];
+ yield responsePayload;
src/v1/tests/persons_test.ts
Outdated
| // it('can search for persons with the appropriate dates', async (t) => { | ||
| // const myDate = new Date(1717428411010) | ||
| // const request: SearchPersonsRequest = { | ||
| // min_first_email_date: myDate, | ||
| // term: 'joscha', | ||
| // } | ||
| // mock?.onGet(personsUrl(), { | ||
| // params: { | ||
| // term: request.term, | ||
| // min_first_email_date: myDate.toISOString(), | ||
| // }, | ||
| // }).reply( | ||
| // 200, | ||
| // await getRawFixture('persons/search.raw.response.json'), | ||
| // ) | ||
| // const res = await affinity.persons.search(request) | ||
| // await assertSnapshot(t, res) | ||
| // }) |
There was a problem hiding this comment.
Commented-out test case: Clarify the status.
The test case for searching persons with the appropriate dates is commented out. Is this test case incomplete or under development? Do you need assistance in completing it?
src/v1/tests/persons_test.ts
Outdated
| // it('can create a new person', async (t) => { | ||
| // const data = { | ||
| // name: 'Acme Corporation', | ||
| // domain: 'acme.co', | ||
| // person_ids: [38706], | ||
| // } | ||
| // mock?.onPost(personsUrl()).reply( | ||
| // 201, | ||
| // await getRawFixture('persons/create.raw.response.json'), | ||
| // ) | ||
| // const res = await affinity.persons.create(data) | ||
| // await assertSnapshot(t, res) | ||
| // }) |
There was a problem hiding this comment.
Commented-out test case: Clarify the status.
The test case for creating a new person is commented out. Is this test case incomplete or under development? Do you need assistance in completing it?
src/v1/tests/persons_test.ts
Outdated
| // it('can update a person', async (t) => { | ||
| // const data = { | ||
| // person_id: 120611418, | ||
| // name: 'Acme Corp.', | ||
| // person_ids: [38706, 89734], | ||
| // } | ||
| // mock?.onPut(personsUrl(data.person_id)).reply( | ||
| // 200, | ||
| // await getRawFixture('persons/update.raw.response.json'), | ||
| // ) | ||
| // const res = await affinity.persons.update(data) | ||
| // await assertSnapshot(t, res) | ||
| // }) |
There was a problem hiding this comment.
Commented-out test case: Clarify the status.
The test case for updating a person is commented out. Is this test case incomplete or under development? Do you need assistance in completing it?
src/v1/tests/persons_test.ts
Outdated
| // it('can delete a person', async (t) => { | ||
| // const person_id = 120611418 | ||
| // mock?.onDelete(personsUrl(person_id)).reply(200, { | ||
| // success: true, | ||
| // }) | ||
| // const res = await affinity.persons.delete({ person_id }) | ||
| // await assertSnapshot(t, res) | ||
| // }) |
There was a problem hiding this comment.
Commented-out test case: Clarify the status.
The test case for deleting a person is commented out. Is this test case incomplete or under development? Do you need assistance in completing it?
src/v1/tests/persons_test.ts
Outdated
| // it('can get global person fields', async (t) => { | ||
| // mock?.onGet(personFieldsUrl()).reply( | ||
| // 200, | ||
| // await getRawFixture('persons/get_fields.raw.response.json'), | ||
| // ) | ||
| // const res = await affinity.persons.getFields() | ||
| // await assertSnapshot(t, res) | ||
| // }) |
There was a problem hiding this comment.
Commented-out test case: Clarify the status.
The test case for getting global person fields is commented out. Is this test case incomplete or under development? Do you need assistance in completing it?
| async function* searchIterator( | ||
| params: Omit<PAGED_REQUEST, 'page_token'>, | ||
| ): AsyncGenerator<PAGED_RESPONSE[]> { | ||
| let page_token: string | undefined = undefined | ||
| while (true) { | ||
| const response: PAGED_RESPONSE = await searchFn( | ||
| // TODO(@joscha): remove cast | ||
| (page_token | ||
| ? { ...params, page_token } | ||
| : params) as PAGED_REQUEST, | ||
| ) | ||
|
|
||
| // TODO(@joscha): remove cast | ||
| yield response[key] as unknown as PAGED_RESPONSE[] | ||
|
|
||
| if (response.next_page_token === null) { | ||
| // no more pages to fetch | ||
| return | ||
| } else { | ||
| page_token = response.next_page_token | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Ensure proper error handling for the asynchronous generator.
The function does not handle potential errors from the searchFn call. Consider adding try-catch blocks to handle errors gracefully.
+ try {
const response: PAGED_RESPONSE = await searchFn(
// TODO(@joscha): remove cast
(page_token
? { ...params, page_token }
: params) as PAGED_REQUEST,
)
// TODO(@joscha): remove cast
yield response[key] as unknown as PAGED_RESPONSE[]
if (response.next_page_token === null) {
// no more pages to fetch
return
} else {
page_token = response.next_page_token
}
+ } catch (error) {
+ console.error("Error fetching page:", error);
+ throw error;
+ }Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| async function* searchIterator( | |
| params: Omit<PAGED_REQUEST, 'page_token'>, | |
| ): AsyncGenerator<PAGED_RESPONSE[]> { | |
| let page_token: string | undefined = undefined | |
| while (true) { | |
| const response: PAGED_RESPONSE = await searchFn( | |
| // TODO(@joscha): remove cast | |
| (page_token | |
| ? { ...params, page_token } | |
| : params) as PAGED_REQUEST, | |
| ) | |
| // TODO(@joscha): remove cast | |
| yield response[key] as unknown as PAGED_RESPONSE[] | |
| if (response.next_page_token === null) { | |
| // no more pages to fetch | |
| return | |
| } else { | |
| page_token = response.next_page_token | |
| } | |
| } | |
| } | |
| async function* searchIterator( | |
| params: Omit<PAGED_REQUEST, 'page_token'>, | |
| ): AsyncGenerator<PAGED_RESPONSE[]> { | |
| let page_token: string | undefined = undefined | |
| while (true) { | |
| + try { | |
| const response: PAGED_RESPONSE = await searchFn( | |
| // TODO(@joscha): remove cast | |
| (page_token | |
| ? { ...params, page_token } | |
| : params) as PAGED_REQUEST, | |
| ) | |
| // TODO(@joscha): remove cast | |
| yield response[key] as unknown as PAGED_RESPONSE[] | |
| if (response.next_page_token === null) { | |
| // no more pages to fetch | |
| return | |
| } else { | |
| page_token = response.next_page_token | |
| } | |
| + } catch (error) { | |
| + console.error("Error fetching page:", error); | |
| + throw error; | |
| + } | |
| } | |
| } |
There was a problem hiding this comment.
Actionable comments posted: 1
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files ignored due to path filters (1)
src/v1/tests/__snapshots__/persons_test.ts.snapis excluded by!**/*.snap
Files selected for processing (9)
- src/v1/organizations.ts (14 hunks)
- src/v1/persons.ts (2 hunks)
- src/v1/tests/fixtures/persons/create.raw.response.json (1 hunks)
- src/v1/tests/fixtures/persons/get_fields.raw.response.json (1 hunks)
- src/v1/tests/fixtures/persons/search_with_dates.raw.response.json (1 hunks)
- src/v1/tests/fixtures/persons/update.raw.response.json (1 hunks)
- src/v1/tests/organizations_test.ts (1 hunks)
- src/v1/tests/persons_test.ts (1 hunks)
- src/v1/transform_interaction_date_response_raw.ts (1 hunks)
Files skipped from review due to trivial changes (3)
- src/v1/tests/fixtures/persons/create.raw.response.json
- src/v1/tests/fixtures/persons/get_fields.raw.response.json
- src/v1/tests/fixtures/persons/update.raw.response.json
Files skipped from review as they are similar to previous changes (1)
- src/v1/tests/persons_test.ts
Additional comments not posted (39)
src/v1/tests/fixtures/persons/search_with_dates.raw.response.json (1)
1-30: JSON Structure ValidationThe JSON structure appears to be well-formed and includes detailed information about a person, including interaction dates and interactions. Ensure that the data accurately reflects the expected schema and use cases.
src/v1/transform_interaction_date_response_raw.ts (2)
48-48: Reminder: Fix types to avoid casting.The TODO comment indicates a potential improvement to fix the types and avoid casting.
14-49: Function Implementation ValidationThe function
transformInteractionDateResponseRawprocesses raw interaction date responses and converts date strings toDateobjects. The implementation appears to be correct.src/v1/tests/organizations_test.ts (2)
51-51: New Property Addition ValidationThe new property
with_interaction_datesis correctly added to theSearchOrganizationsRequestobject. Ensure that the test cases cover all necessary scenarios.
57-57: New Property Addition ValidationThe new property
with_interaction_datesis correctly added to the parameters of the mocked GET request. Ensure that the test cases cover all necessary scenarios.src/v1/persons.ts (19)
73-73: LGTM!The type
PersonResponsecorrectly uses theReplaceutility type.
75-80: LGTM!The type
WithCurrentOrganizatonParamsis well-defined.
82-95: LGTM!The type
SearchPersonsRequestis well-defined and includes various query parameters.
97-105: LGTM!The type
PagedPersonResponseRawis well-defined and includes an array ofPersonResponseRaw.
109-112: LGTM!The type
SinglePersonResponseRawis well-defined and includes list entries andPersonResponseRaw.
113-121: LGTM!The type
SinglePersonResponsecorrectly uses theReplaceutility type.
123-127: LGTM!The type
GetPersonRequestis well-defined and includes various query parameters.
129-131: LGTM!The type
PersonReferenceis well-defined and includes the unique ID of the person.
136-153: LGTM!The type
CreatePersonRequestis well-defined and includes various fields for creating a person.
158-183: LGTM!The type
UpdatePersonRequestis well-defined and includes various fields for updating a person.
186-191: LGTM!The type
SimplePersonResponseis well-defined and includes thePersontype and selected fields fromPersonResponse.
232-259: LGTM!The
getmethod is well-implemented and handles the transformation of the response correctly.
280-305: LGTM!The
searchmethod is well-implemented and handles the transformation of the response correctly.
325-328: LGTM!The
searchIteratormethod is well-implemented and uses thecreateSearchIteratorFnutility function correctly.
347-355: LGTM!The
createmethod is well-implemented and handles the creation of a person correctly.
373-382: LGTM!The
updatemethod is well-implemented and handles the update of a person correctly.
396-402: LGTM!The
deletemethod is well-implemented and handles the deletion of a person correctly.
415-422: LGTM!The
getFieldsmethod is well-implemented and handles the fetching of person fields correctly.
60-71: Ensure the type system models the return type based on the query parameter type.The TODO comment suggests modeling the return type based on the query parameter type.
- current_organization_ids?: number[] + current_organization_ids?: number[] // TODO: Model this in the type systemLikely invalid or redundant comment.
src/v1/organizations.ts (15)
131-136: LGTM!The type
OrganizationResponseRawis well-defined and includes various fields.
172-177: LGTM!The type
PagedOrganizationResponseRawis well-defined and includes an array ofOrganizationResponseRaw.
178-180: LGTM!The type
PagedOrganizationResponsecorrectly uses theReplaceutility type.
189-195: LGTM!The type
SingleOrganizationResponseRawis well-defined and includes list entries andOrganizationResponseRaw.
191-195: LGTM!The type
SingleOrganizationResponsecorrectly uses theReplaceutility type.
168-170: LGTM!The type
ListEntryReferencecorrectly uses theReplaceutility type.
Line range hint
324-352:
LGTM!The
getmethod is well-implemented and handles the transformation of the response correctly.
Line range hint
378-399:
LGTM!The
searchmethod is well-implemented and handles the transformation of the response correctly.
419-422: LGTM!The
searchIteratormethod is well-implemented and uses thecreateSearchIteratorFnutility function correctly.
Line range hint
460-471:
LGTM!The
createmethod is well-implemented and handles the creation of an organization correctly.
Line range hint
493-502:
LGTM!The
updatemethod is well-implemented and handles the update of an organization correctly.
Line range hint
524-530:
LGTM!The
deletemethod is well-implemented and handles the deletion of an organization correctly.
Line range hint
548-555:
LGTM!The
getFieldsmethod is well-implemented and handles the fetching of organization fields correctly.
74-94: Ensure the type system models the return type based on the query parameter type.The TODO comment suggests modeling the return type based on the query parameter type.
- interaction_dates?: { - [key in InteractionDateKey]: DateTime - } - interactions?: { - [key in InteractionType]: InteractionDateRaw - } + interaction_dates?: { + [key in InteractionDateKey]: DateTime // TODO: Model this in the type system + } + interactions?: { + [key in InteractionType]: InteractionDateRaw // TODO: Model this in the type system + }Likely invalid or redundant comment.
96-117: Ensure the type system models the return type based on the query parameter type.The TODO comment suggests modeling the return type based on the query parameter type.
- interaction_dates?: { - [key in InteractionDateKey]: Date - } - interactions?: { - [key in InteractionType]: InteractionDate - } + interaction_dates?: { + [key in InteractionDateKey]: Date // TODO: Model this in the type system + } + interactions?: { + [key in InteractionType]: InteractionDate // TODO: Model this in the type system + }Likely invalid or redundant comment.
| export type OpportunityIdResponseRaw = { | ||
| /** | ||
| * An array of unique identifiers of opportunities ({@link Opportunity.id}) that are associated with the entity. | ||
| * Only returned when passing `{@link OpportunitiesQueryParams.with_opportunities}=true`. | ||
| * | ||
| * TODO(@joscha): model this in the type system, so the return type is based on the query parameter type. | ||
| */ | ||
| opportunity_ids?: number[] | ||
| } |
There was a problem hiding this comment.
Ensure the type system models the return type based on the query parameter type.
The TODO comment suggests modeling the return type based on the query parameter type.
- opportunity_ids?: number[]
+ opportunity_ids?: number[] // TODO: Model this in the type systemCommittable suggestion was skipped due to low confidence.
Summary by CodeRabbit
New Features
.coderabbit.yamlconfiguration.eof_newlineoption for YAML formatting to control end-of-file newlines.Bug Fixes
README.mdfor better clarity on availability of "Persons" and "Opportunities."Documentation
Refactor