GetKlaviyoReviews query #517
Conversation
WalkthroughThe changes in this pull request include the addition of a new GraphQL query, Changes
Assessment against linked issues
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 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: 3
🧹 Outside diff range and nitpick comments (1)
packages/vendure-plugin-klaviyo/src/api/klaviyo-shop.resolver.ts (1)
7-7: Rename injected service.Change 'productService' to 'klaviyoService' for clarity.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (2)
packages/vendure-plugin-klaviyo/codegen.ymlis excluded by!**/*.ymlpackages/vendure-plugin-klaviyo/package.jsonis excluded by!**/*.json
📒 Files selected for processing (9)
- packages/vendure-plugin-klaviyo/CHANGELOG.md (1 hunks)
- packages/vendure-plugin-klaviyo/README.md (1 hunks)
- packages/vendure-plugin-klaviyo/src/api/api-extensions.ts (1 hunks)
- packages/vendure-plugin-klaviyo/src/api/klaviyo-shop.resolver.ts (1 hunks)
- packages/vendure-plugin-klaviyo/src/klaviyo.plugin.ts (2 hunks)
- packages/vendure-plugin-klaviyo/src/klaviyo.service.ts (3 hunks)
- packages/vendure-plugin-klaviyo/test/e2e.spec.ts (2 hunks)
- packages/vendure-plugin-klaviyo/test/mock-data.ts (1 hunks)
- packages/vendure-plugin-klaviyo/test/queries.ts (1 hunks)
🧰 Additional context used
🔇 Additional comments (10)
packages/vendure-plugin-klaviyo/test/queries.ts (1)
1-11: Looks good.The query is correctly implemented.
packages/vendure-plugin-klaviyo/src/api/klaviyo-shop.resolver.ts (3)
1-3: Imports look good.
5-6: Class declaration is correct.
9-12: Query method looks good.packages/vendure-plugin-klaviyo/CHANGELOG.md (1)
1-3: Looks good.The changelog entry is correct and concise.
packages/vendure-plugin-klaviyo/src/klaviyo.plugin.ts (2)
9-10: New imports added.The new imports look good and are related to the new functionality.
37-40: shopApiExtensions added to @VendurePlugin decorator.The addition of shopApiExtensions with KlaviyoShopResolver and shopApiExtension schema looks correct.
packages/vendure-plugin-klaviyo/README.md (1)
95-115: Looks good.The new section about fetching reviews is clear and useful.
packages/vendure-plugin-klaviyo/test/e2e.spec.ts (2)
19-21: New imports added.New imports for Klaviyo reviews functionality.
191-206: New test case for fetching Klaviyo reviews.Test case added to fetch all reviews from Klaviyo. It sets up a mock HTTP request and checks the response.
| author: 'Jane D', | ||
| content: 'Nice product, but the delivery was late.', | ||
| title: 'Good, but delivery issues', | ||
| smart_quote: 'Delivery was late', |
There was a problem hiding this comment.
Fix smart_quote content.
The smart_quote doesn't match the content.
| const scalars = gql` | ||
| scalar JSON | ||
| `; |
There was a problem hiding this comment.
'JSON' scalar not included in the schema
The scalar JSON is defined but not included in the exported GraphQL schema. This can lead to errors when using the JSON type in your schema.
Apply this diff to include the JSON scalar in the schema:
-const scalars = gql`
- scalar JSON
-`;
export const shopApiExtension = gql`
+ scalar JSON
enum KlaviyoReviewStatus {
featured
pending
published📝 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.
| const scalars = gql` | |
| scalar JSON | |
| `; | |
| export const shopApiExtension = gql` | |
| scalar JSON | |
| enum KlaviyoReviewStatus { | |
| featured | |
| pending | |
| published |
| const response = await axios.get<KlaviyoResponse>(url, { headers }); | ||
| return response.data; |
There was a problem hiding this comment.
Handle errors from axios GET request
The axios GET request lacks error handling. If the request fails, it could cause unhandled exceptions.
Apply this diff to add error handling:
- const response = await axios.get<KlaviyoResponse>(url, { headers });
- return response.data;
+ try {
+ const response = await axios.get<KlaviyoResponse>(url, { headers });
+ return response.data;
+ } catch (error) {
+ // Handle error appropriately
+ 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.
| const response = await axios.get<KlaviyoResponse>(url, { headers }); | |
| return response.data; | |
| try { | |
| const response = await axios.get<KlaviyoResponse>(url, { headers }); | |
| return response.data; | |
| } catch (error) { | |
| // Handle error appropriately | |
| throw error; | |
| } |
|
Placed on hold for now, because of a change in priority in the client project |
Description
The changes in this PR implement and close #516
Deploy
This needs to happen to deploy this feature, for example:
Breaking changes
Does this PR include any breaking changes we should be aware of? NO
Screenshots
Checklist
📌 Always:
👍 Most of the time:
📦 For publishable packages:
package.jsonCHANGELOG.mdSee this example