-
Notifications
You must be signed in to change notification settings - Fork 5.5k
[ACTION] Gorgias — Create a ticket message #15486
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
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 3 Skipped Deployments
|
WalkthroughThis pull request performs several minor version increments across multiple modules in the Gorgias OAuth component. Notably, the create-ticket-message action is updated: properties are renamed and added, and the original Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant App as gorgias_oauth.app
participant API as _makeRequest
Client->>App: retrieveUser({ id })
App->>API: GET /users/{id}
API-->>App: User data
App-->>Client: Return user data
Client->>App: createMessage({ ticketId, ...opts })
App->>API: POST /tickets/{ticketId}/messages with payload
API-->>App: Message confirmation
App-->>Client: Return confirmation
sequenceDiagram
participant User
participant Action as create-ticket-message.run
participant Util as Additional Logic
User->>Action: Invoke run with message details
Action->>Util: Evaluate `fromAgent` for sender/receiver
Util-->>Action: Provide sender & receiver IDs
Action-->>User: Return constructed message data
Assessment against linked issues
Suggested labels
Suggested reviewers
Poem
Tip 🌐 Web search-backed reviews and chat
✨ Finishing Touches
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 (
|
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
🧹 Nitpick comments (2)
components/gorgias_oauth/actions/create-ticket-message/create-ticket-message.mjs (2)
27-36: Optional suggestion for validation.Currently,
fromUseris required (optional: false) whenfromAgent = true, but there is no explicit validation thatfromUserhas a value. Consider adding a runtime check to ensurefromUseris indeed provided whenfromAgentistrue.
169-169: Hardcoding "email" might limit other channels.If the channel is intended to be flexible in the future, consider using a prop or referencing a definition. Otherwise, this is acceptable if "email" is the only supported channel.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (12)
components/gorgias_oauth/actions/create-customer/create-customer.mjs(1 hunks)components/gorgias_oauth/actions/create-ticket-message/create-ticket-message.mjs(6 hunks)components/gorgias_oauth/actions/create-ticket/create-ticket.mjs(1 hunks)components/gorgias_oauth/actions/list-tickets/list-tickets.mjs(1 hunks)components/gorgias_oauth/actions/retrieve-customer/retrieve-customer.mjs(1 hunks)components/gorgias_oauth/actions/update-customer/update-customer.mjs(1 hunks)components/gorgias_oauth/actions/update-ticket/update-ticket.mjs(1 hunks)components/gorgias_oauth/gorgias_oauth.app.mjs(2 hunks)components/gorgias_oauth/package.json(1 hunks)components/gorgias_oauth/sources/ticket-created/ticket-created.mjs(1 hunks)components/gorgias_oauth/sources/ticket-message-created/ticket-message-created.mjs(1 hunks)components/gorgias_oauth/sources/ticket-updated/ticket-updated.mjs(1 hunks)
✅ Files skipped from review due to trivial changes (10)
- components/gorgias_oauth/actions/create-ticket/create-ticket.mjs
- components/gorgias_oauth/sources/ticket-message-created/ticket-message-created.mjs
- components/gorgias_oauth/sources/ticket-created/ticket-created.mjs
- components/gorgias_oauth/actions/list-tickets/list-tickets.mjs
- components/gorgias_oauth/package.json
- components/gorgias_oauth/actions/retrieve-customer/retrieve-customer.mjs
- components/gorgias_oauth/sources/ticket-updated/ticket-updated.mjs
- components/gorgias_oauth/actions/update-ticket/update-ticket.mjs
- components/gorgias_oauth/actions/create-customer/create-customer.mjs
- components/gorgias_oauth/actions/update-customer/update-customer.mjs
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: pnpm publish
- GitHub Check: Verify TypeScript components
- GitHub Check: Publish TypeScript components
🔇 Additional comments (13)
components/gorgias_oauth/actions/create-ticket-message/create-ticket-message.mjs (11)
10-10: Version bump looks correct.The version increment from
0.0.1to0.0.2is appropriate, reflecting the new enhancements.
21-26: Property name clarity is good.Defining
fromAgentas a boolean to differentiate agent/customer-sent messages is clear and straightforward.
37-46: Align property naming with code usage.
toUseris clearly named to indicate the receiving user. No issues noted here.
47-55: Good transition from addresses to customer references.Renaming to
fromCustomeris more transparent and consistent with Gorgias domain terminology.
56-64: Maintain consistency withfromCustomer.Similar to
fromCustomer,toCustomereffectively describes the receiving customer. The hidden toggling logic for these fields is appropriate.
102-102: No action needed.This line is only closing a code block, no issues here.
103-109: Property visibility toggling is well-organized.Using
additionalPropsto dynamically hide or show fields based onfromAgentis a clean approach.
122-141: Method logic is concise and flexible.Splitting logic in
getEmailbyfromAgentandtypeis a straightforward way to determine which retrieval function to call. Looks good overall.
157-164: Clear assignment of sender and receiver IDs.The logic for selecting
fromIdandtoIdconditionally onfromAgentis consistent, making it easy to see who is sending and who is receiving.
165-165: Creation call is well-structured.Using
this.gorgiasOauth.createMessage()keeps the action’s code simpler and delegates complexity to the app file.
172-177: Asynchronously fetching email addresses.Using
await this.getEmailfor bothsource.from.addressandsource.to[0].addressis consistent. No issues noted.components/gorgias_oauth/gorgias_oauth.app.mjs (2)
332-339: User retrieval method matches existing conventions.Introducing
retrieveUseraligns with the_makeRequestpattern used throughout the file. This allows consistent error handling and request formation.
404-412: Good addition of a createMessage API helper.Centralizing the message creation in
createMessagemakes the action’s code more concise. If there's a need for extra validation or logging, consider adding it here to keep it consistent across all usage.
components/gorgias_oauth/actions/create-ticket-message/create-ticket-message.mjs
Show resolved
Hide resolved
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
🧹 Nitpick comments (3)
components/gorgias_oauth/actions/create-ticket-message/create-ticket-message.mjs (3)
21-64: Consider adding validation for mutually exclusive properties.The dynamic visibility of properties based on
fromAgentis good, but consider adding validation to ensure that:
- When
fromAgentis true, eitherfromUserandtoCustomerare set (not both empty or both filled)- When
fromAgentis false, eitherfromCustomerandtoUserare set (not both empty or both filled)fromAgent: { type: "boolean", label: "From Agent", description: "Whether the message was sent by your company to a customer, or the opposite", reloadProps: true, + validate: (value, { fromUser, toCustomer, fromCustomer, toUser }) => { + if (value && (!fromUser || !toCustomer) && (fromUser && toCustomer)) { + throw new Error("When sending from agent, exactly one of fromUser and toCustomer must be set"); + } + if (!value && (!fromCustomer || !toUser) && (fromCustomer && toUser)) { + throw new Error("When sending from customer, exactly one of fromCustomer and toUser must be set"); + } + return true; + }, },
103-109: Simplify additionalProps method.The method can be simplified by:
- Removing the unnecessary empty object return
- Using object destructuring for cleaner property access
additionalProps(props) { + const { toUser, fromCustomer, toCustomer, fromUser } = props; + const isFromAgent = this.fromAgent; + - props.toUser.hidden = this.fromAgent; - props.fromCustomer.hidden = this.fromAgent; - props.toCustomer.hidden = !this.fromAgent; - props.fromUser.hidden = !this.fromAgent; - return {}; + Object.assign(props, { + toUser: { ...toUser, hidden: isFromAgent }, + fromCustomer: { ...fromCustomer, hidden: isFromAgent }, + toCustomer: { ...toCustomer, hidden: !isFromAgent }, + fromUser: { ...fromUser, hidden: !isFromAgent }, + }); }
157-200: LGTM! Consider adding input validation.The implementation correctly addresses the previous review comment about variable references. The logic for determining sender and receiver IDs is sound.
Consider adding validation for required inputs before making the API call:
async run({ $ }) { + // Validate required inputs + if (!this.ticketId) { + throw new ConfigurationError("Ticket ID is required"); + } + if (!this.message) { + throw new ConfigurationError("Message content is required"); + } + if ((this.attachmentUrl && !this.attachmentName) || (!this.attachmentUrl && this.attachmentName) ) {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
components/gorgias_oauth/actions/create-ticket-message/create-ticket-message.mjs(6 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: Publish TypeScript components
- GitHub Check: pnpm publish
- GitHub Check: Verify TypeScript components
components/gorgias_oauth/actions/create-ticket-message/create-ticket-message.mjs
Show resolved
Hide resolved
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: 0
🧹 Nitpick comments (1)
components/gorgias_oauth/actions/create-ticket-message/create-ticket-message.mjs (1)
103-109: Consider refactoring property visibility logic for better maintainability.While the logic is correct, consider grouping related properties and their visibility rules for better maintainability.
- additionalProps(props) { - props.toUser.hidden = this.fromAgent; - props.fromCustomer.hidden = this.fromAgent; - props.toCustomer.hidden = !this.fromAgent; - props.fromUser.hidden = !this.fromAgent; - return {}; - }, + additionalProps(props) { + const agentProps = ['toUser', 'fromCustomer']; + const customerProps = ['toCustomer', 'fromUser']; + + agentProps.forEach(prop => props[prop].hidden = this.fromAgent); + customerProps.forEach(prop => props[prop].hidden = !this.fromAgent); + + return {}; + },
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
components/gorgias_oauth/actions/create-ticket-message/create-ticket-message.mjs(6 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: pnpm publish
- GitHub Check: Verify TypeScript components
- GitHub Check: Publish TypeScript components
- GitHub Check: Lint Code Base
🔇 Additional comments (3)
components/gorgias_oauth/actions/create-ticket-message/create-ticket-message.mjs (3)
10-64: LGTM! Well-structured property definitions with clear documentation.The new properties are well-organized with appropriate labels, descriptions, and visibility controls. The
fromAgentboolean effectively toggles between company and customer message flows.
122-141: Improve readability and error handling in getEmail method.The nested ternary operators make the code hard to read and the method lacks proper error handling.
Please refer to the previous review comment for the suggested improvements, which includes:
- Using early returns for better readability
- Adding proper error handling for invalid IDs
- Implementing try-catch blocks
157-209: LGTM! Robust validation and clear error messages.The implementation includes:
- Clear logic for determining sender and receiver IDs
- Thorough validation with contextual error messages
- Proper message structure with sender/receiver objects
|
/approve |
Resolves #15406
Summary by CodeRabbit