-
Notifications
You must be signed in to change notification settings - Fork 0
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
Dev/gga/shopify v2 #4
base: main
Are you sure you want to change the base?
Conversation
WalkthroughThe recent updates enhance the integration between the application and Shopify by introducing new fields and actions for invoice synchronization and payment terms mapping. Codeunits for draft orders, fulfillment, posted invoice export, and payment terms API have been added, improving the handling of Shopify-related operations via GraphQL. A new report aids in syncing invoices to Shopify while a dedicated table manages Shopify payment terms, optimizing e-commerce workflow and data consistency. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant App as Application
participant ShpfyAPI as Shopify API
User->>App: Initiate Sync for Posted Sales Invoices
App->>ShpfyAPI: Authenticate & Send Export Request
ShpfyAPI-->>App: Acknowledge & Process Request
App-->>User: Sync Status and Feedback
User->>App: Manage Shopify Payment Terms
App->>ShpfyAPI: Request Payment Terms Data
ShpfyAPI-->>App: Return Payment Terms Data
App-->>User: Display/Update Payment Terms
Poem
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 Configration File (
|
Apps/W1/Shopify/app/src/GraphQL/Codeunits/ShpfyGQLDraftOrderComplete.Codeunit.al
Outdated
Show resolved
Hide resolved
Apps/W1/Shopify/app/src/Integration/Codeunits/ShpfyAuthenticationMgt.Codeunit.al
Outdated
Show resolved
Hide resolved
Apps/W1/Shopify/app/src/Invoicing/Codeunits/ShpfyDraftOrdersAPI.Codeunit.al
Outdated
Show resolved
Hide resolved
Apps/W1/Shopify/app/src/Invoicing/Codeunits/ShpfyDraftOrdersAPI.Codeunit.al
Show resolved
Hide resolved
Apps/W1/Shopify/app/src/Invoicing/Codeunits/ShpfyDraftOrdersAPI.Codeunit.al
Outdated
Show resolved
Hide resolved
Apps/W1/Shopify/app/src/Invoicing/Codeunits/ShpfyDraftOrdersAPI.Codeunit.al
Outdated
Show resolved
Hide resolved
Apps/W1/Shopify/app/src/Invoicing/Codeunits/ShpfyDraftOrdersAPI.Codeunit.al
Outdated
Show resolved
Hide resolved
Apps/W1/Shopify/app/src/Invoicing/Codeunits/ShpfyDraftOrdersAPI.Codeunit.al
Outdated
Show resolved
Hide resolved
Apps/W1/Shopify/app/src/Invoicing/Codeunits/ShpfyDraftOrdersAPI.Codeunit.al
Outdated
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: 14
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (22)
- Apps/W1/Shopify/app/src/Base/Pages/ShpfyShopCard.Page.al (4 hunks)
- Apps/W1/Shopify/app/src/Base/Tables/ShpfyShop.Table.al (2 hunks)
- Apps/W1/Shopify/app/src/GraphQL/Codeunits/ShpfyGQLDraftOrderComplete.Codeunit.al (1 hunks)
- Apps/W1/Shopify/app/src/GraphQL/Codeunits/ShpfyGQLFulfillOrder.Codeunit.al (1 hunks)
- Apps/W1/Shopify/app/src/GraphQL/Codeunits/ShpfyGQLPaymentTerms.Codeunit.al (1 hunks)
- Apps/W1/Shopify/app/src/GraphQL/Enums/ShpfyGraphQLType.Enum.al (1 hunks)
- Apps/W1/Shopify/app/src/Integration/Codeunits/ShpfyAuthenticationMgt.Codeunit.al (1 hunks)
- Apps/W1/Shopify/app/src/Invoicing/Codeunits/ShpfyDraftOrdersAPI.Codeunit.al (1 hunks)
- Apps/W1/Shopify/app/src/Invoicing/Codeunits/ShpfyFulfillmentAPI.Codeunit.al (1 hunks)
- Apps/W1/Shopify/app/src/Invoicing/Codeunits/ShpfyPostedInvoiceExport.Codeunit.al (1 hunks)
- Apps/W1/Shopify/app/src/Invoicing/Codeunits/ShpfyUpdateSalesInvoice.Codeunit.al (1 hunks)
- Apps/W1/Shopify/app/src/Invoicing/PageExt/ShpfySalesInvoiceUpdate.PageExt.al (1 hunks)
- Apps/W1/Shopify/app/src/Invoicing/Reports/ShpfyExportInvoiceToShpfy.Report.al (1 hunks)
- Apps/W1/Shopify/app/src/Invoicing/Tables/ShpfyInvoiceHeader.Table.al (1 hunks)
- Apps/W1/Shopify/app/src/Order handling/Codeunits/ShpfyImportOrder.Codeunit.al (3 hunks)
- Apps/W1/Shopify/app/src/Order handling/Codeunits/ShpfyProcessOrder.Codeunit.al (2 hunks)
- Apps/W1/Shopify/app/src/Order handling/Tables/ShpfyOrderHeader.Table.al (1 hunks)
- Apps/W1/Shopify/app/src/Payments/Codeunits/ShpfyPaymentTermAPI.Codeunit.al (1 hunks)
- Apps/W1/Shopify/app/src/Payments/Pages/ShpfyPaymentTermsMapping.Page.al (1 hunks)
- Apps/W1/Shopify/app/src/Payments/Tables/ShpfyPaymentTerm.Table.al (1 hunks)
- Apps/W1/Shopify/app/src/PermissionSets/ShpfyEdit.PermissionSet.al (2 hunks)
- Apps/W1/Shopify/app/src/PermissionSets/ShpfyRead.PermissionSet.al (2 hunks)
Additional comments not posted (24)
Apps/W1/Shopify/app/src/Invoicing/Tables/ShpfyInvoiceHeader.Table.al (1)
1-26
: Review of ShpfyInvoiceHeader.Table.alThe table definition for "Shpfy Invoice Header" appears well-defined with proper data classification, a primary key, and a single field for the Shopify Order ID. The use of
BigInteger
for the "Shopify Order Id" is appropriate given the potentially large values. The table is marked with aClustered
key, which is suitable for performance on the primary identifier.
- Data Classification: The use of
CustomerContent
for data classification is appropriate as it involves customer-specific data.- Primary Key: The choice of "Shopify Order Id" as a primary key is suitable as it should be unique per invoice.
- Documentation: The summary documentation is clear and concise, providing a good overview of the table's purpose.
Overall, the table setup seems to align well with the needs of handling Shopify invoice data.
Apps/W1/Shopify/app/src/GraphQL/Codeunits/ShpfyGQLPaymentTerms.Codeunit.al (1)
1-28
: Review of ShpfyGQLPaymentTerms.Codeunit.alThis codeunit is well-structured with clear and concise procedures for fetching GraphQL queries and expected costs for payment terms. The GraphQL query is correctly formatted to fetch necessary fields such as
id
,name
,paymentTermsType
,dueInDays
, anddescription
.
- Access Modifier: The use of
Internal
access is appropriate, ensuring that these procedures are only callable within the application.- Return Types: The procedures correctly return their expected data types,
Text
for the GraphQL query andInteger
for the expected cost.- GraphQL Query: The query is well-formed and targets specific, relevant fields, which should aid in efficient data retrieval.
The implementation is straightforward and aligns with the intended functionality.
Apps/W1/Shopify/app/src/GraphQL/Codeunits/ShpfyGQLFulfillOrder.Codeunit.al (1)
1-27
: Review of ShpfyGQLFulfillOrder.Codeunit.alThis codeunit effectively handles the GraphQL operations for fulfilling Shopify orders. The mutation is structured to create fulfillment orders and handle potential user errors.
- GraphQL Mutation: The mutation is well-constructed, using placeholders for dynamic data insertion, which is a good practice for reusability and flexibility.
- Expected Cost: The specified cost of
10
is noted, and it would be beneficial to ensure this aligns with actual query costs observed in practice, as discussed in previous developer comments.The structure and implementation are clear, and the internal access modifier is correctly used.
Apps/W1/Shopify/app/src/GraphQL/Codeunits/ShpfyGQLDraftOrderComplete.Codeunit.al (1)
1-27
: Review of ShpfyGQLDraftOrderComplete.Codeunit.alThe codeunit is designed to handle the GraphQL operations for completing draft orders in Shopify. The mutation includes comprehensive handling of order details and user errors.
- GraphQL Mutation: The mutation is appropriately detailed, including necessary fields and handling multiple orders. The use of placeholders (
{{DraftOrderId}}
,{{NumberOfOrders}}
) enhances the flexibility of the query.- Expected Cost: The cost is set to
15
, which should be verified against actual costs to ensure accuracy, as previously discussed by developers.The codeunit is well-implemented with clear documentation and appropriate internal access control.
Apps/W1/Shopify/app/src/Invoicing/Codeunits/ShpfyUpdateSalesInvoice.Codeunit.al (1)
1-20
: Review of ShpfyUpdateSalesInvoice.Codeunit.alThis codeunit provides event handling for updates to sales invoices, specifically focusing on the "Shpfy Order Id". The event subscribers are well-defined, ensuring that changes in the Shopify Order ID are handled correctly.
- Event Handling: The use of event subscribers for detecting changes and updating records is a robust method to ensure data consistency.
- Logic: The logic to exit early if there is no change (
IsChanged
) and to update the Shopify Order ID if it differs is straightforward and effective.The implementation is clear, and the use of local procedures ensures encapsulation of the business logic.
Apps/W1/Shopify/app/src/Payments/Tables/ShpfyPaymentTerm.Table.al (1)
8-64
: Table Definition Review for "Shpfy Payment Terms"The table is well-defined with appropriate data classifications and field attributes. The use of
BigInteger
for "Id" is suitable for systems expecting a large range of identifiers, which is common in distributed environments like Shopify. Setting fields as non-editable ensures data integrity from the application side, which is a good practice in scenarios involving synchronization with external systems.However, ensure that the "Shop Code" and "Id" fields are indeed intended to be non-editable as this could impact administrative tasks or data corrections from the UI if needed.
Apps/W1/Shopify/app/src/Payments/Pages/ShpfyPaymentTermsMapping.Page.al (1)
6-70
: Review of Page "Shpfy Payment Terms Mapping"The layout and field setup are appropriate for a mapping page, providing clear information on each payment term. Tooltips are well-used to enhance user understanding.
The "Refresh" action (lines 51-67) is well-implemented, using the
ShpfyPaymentTermAPI
to pull payment terms, which should help keep the data up-to-date. However, ensure that error handling is robust in thePullPaymentTermsCodes
method to handle any issues that might arise during the synchronization process.Apps/W1/Shopify/app/src/PermissionSets/ShpfyRead.PermissionSet.al (2)
35-35
: Addition of Read Permission for "Shpfy Invoice Header"The addition of the read permission for the "Shpfy Invoice Header" table is aligned with the PR's objective to handle invoicing enhancements. This change allows restricted read access to the invoice header data, which is essential for the new invoicing functionalities introduced.
51-51
: Addition of Read Permission for "Shpfy Payment Terms"The addition of the read permission for the "Shpfy Payment Terms" table supports the new functionality for managing payment terms synchronization with Shopify. This ensures that the necessary data can be accessed in a read-only format where needed.
Apps/W1/Shopify/app/src/PermissionSets/ShpfyEdit.PermissionSet.al (2)
35-35
: Addition of Edit Permission for "Shpfy Invoice Header"The addition of the edit permission for the "Shpfy Invoice Header" table is necessary for the newly introduced invoicing functionalities, allowing for the modification of invoice data directly. This change supports the robust handling of Shopify invoice data within the system.
55-55
: Addition of Edit Permission for "Shpfy Payment Terms"The addition of the edit permission for the "Shpfy Payment Terms" table is crucial for the new functionality allowing the synchronization and management of payment terms with Shopify. This permission enables the necessary modifications to the payment terms data as required by the synchronization process.
Apps/W1/Shopify/app/src/Invoicing/Reports/ShpfyExportInvoiceToShpfy.Report.al (1)
1-98
: Comprehensive Review of 'Shpfy Export Invoice to Shpfy' ReportThe new report for exporting invoices to Shopify is well-structured and includes necessary error handling, filters, and UI components to facilitate user interaction. The use of dialogues for process updates enhances user experience by providing real-time feedback during the export process. The integration with
ShpfyPostedInvoiceExport
for actual data handling is correctly implemented. This addition aligns with the invoicing enhancements described in the PR.Apps/W1/Shopify/app/src/Integration/Codeunits/ShpfyAuthenticationMgt.Codeunit.al (1)
18-18
: Updated Scopes in 'Shpfy Authentication Mgt.'The addition of new scopes such as
write_payment_terms
andwrite_draft_orders
to theScopeTxt
label is critical for enabling new functionalities that interact with Shopify's API for payment terms and draft orders. This update is necessary for the expanded capabilities described in the PR and ensures that the application has the required permissions to perform these operations.Apps/W1/Shopify/app/src/GraphQL/Enums/ShpfyGraphQLType.Enum.al (1)
403-417
: Addition of New Enum Values for GraphQL OperationsThe addition of new enum values for
DraftOrderComplete
,FulfillOrder
, andGetPaymentTerms
with their respective implementations links them correctly to the new GraphQL operations. These additions are crucial for supporting the expanded functionalities of the Shopify integration, allowing for more complex operations such as handling draft orders and payment terms synchronization.Apps/W1/Shopify/app/src/Order handling/Codeunits/ShpfyProcessOrder.Codeunit.al (2)
126-127
: Addition of Payment Terms Update LogicThe code for updating payment terms based on
Payment Terms Type
andPayment Terms Name
is a good addition, aligning with the new fields in theShpfy Order Header
table. It's essential to ensure that this new logic integrates correctly with the existing order processing flow.
155-163
: Implementation of UpdatePaymentTerms ProcedureThe implementation of the
UpdatePaymentTerms
procedure is concise and correctly uses theSetRange
method to filter theShpfy Payment Terms
records. TheFindFirst
method is used effectively to apply the payment terms code to the sales header. This is a straightforward and efficient way to handle payment terms updates.Apps/W1/Shopify/app/src/Order handling/Tables/ShpfyOrderHeader.Table.al (1)
730-739
: Addition of Payment Terms FieldsThe addition of
Payment Terms Type
andPayment Terms Name
fields is a significant enhancement. These fields are essential for better handling and synchronization of payment terms with Shopify, which can be crucial for financial reporting and compliance. Ensure that these new fields are properly integrated and tested in all relevant processes and interfaces where theShpfy Order Header
table is used.Apps/W1/Shopify/app/src/Base/Tables/ShpfyShop.Table.al (1)
Line range hint
210-210
: New fields for product mapping and invoice synchronization addedThe addition of the fields "Items Mapped to Products" and "Posted Invoice Sync" aligns with the PR's objective to enhance synchronization capabilities and data handling between the app and Shopify. This is crucial for ensuring data integrity and smooth operation between systems.
Also applies to: 792-798
Apps/W1/Shopify/app/src/Order handling/Codeunits/ShpfyImportOrder.Codeunit.al (2)
125-130
: Review the implementation ofShopifyInvoiceExists
.This function checks for the existence of an invoice based on the Shopify order ID. Given its critical role in conditional logic for order processing, it's important to ensure:
- Correctness: The function uses the
Get
method, which is appropriate for checking existence. Ensure that the field"Shopify Order Id"
is indexed for performance.- Maintainability: Consider adding a comment explaining what constitutes an invoice's existence and any potential edge cases.
521-522
: New fields for payment terms added to the JSON mapping.The addition of
"Payment Terms Type"
and"Payment Terms Name"
fields to the JSON mapping is crucial for handling payment terms more effectively in imported orders. Ensure the following:
- Data Mapping: Verify that these fields are correctly mapped from the JSON object to the
OrderHeader
record.- Data Validation: Consider validating the data to ensure it meets any expected formats or constraints before insertion.
Apps/W1/Shopify/app/src/Base/Pages/ShpfyShopCard.Page.al (4)
142-147
: Added field for Posted Invoice SyncThe addition of the "Posted Invoice Sync" field allows users to specify whether the posted sales invoices can be synchronized to Shopify. This is a beneficial feature for ensuring that financial records between Business Central and Shopify remain consistent. The implementation seems correct and adheres to the application's standards.
254-257
: Added field for Items Mapped to ProductsThe new field "Items Mapped to Products" is designed to control whether only items that are mapped to Shopify products or variants are synchronized from posted sales invoices to Shopify. This field enhances the granularity of sync control, which could be crucial for businesses with complex inventory management needs. The tooltip is clear and the field properties are appropriately set.
664-676
: Added action for Payment Terms MappingThe new action "Payment Terms Mapping" provides a direct way to access and configure the mapping of payment terms between Shopify and Business Central. This is crucial for ensuring that payment terms are correctly synchronized and managed across platforms, which can affect payment schedules and financial reporting. The action is well-placed in the navigation area, making it easily accessible.
1000-1019
: Added action for Syncing Posted Sales InvoicesThis action enables the synchronization of posted sales invoices to Shopify, which is a significant feature for businesses that need to keep their financial data in sync across both platforms. The conditional enabling of this action based on the "Posted Invoice Sync" field is a good practice as it prevents unintended operations. The implementation of the action including the trigger that runs the "Shpfy Export Invoice to Shpfy" report is correctly done.
Apps/W1/Shopify/app/src/Order handling/Codeunits/ShpfyImportOrder.Codeunit.al
Show resolved
Hide resolved
Apps/W1/Shopify/app/src/Order handling/Codeunits/ShpfyImportOrder.Codeunit.al
Show resolved
Hide resolved
Apps/W1/Shopify/app/src/Invoicing/Codeunits/ShpfyDraftOrdersAPI.Codeunit.al
Outdated
Show resolved
Hide resolved
Apps/W1/Shopify/app/src/Invoicing/Codeunits/ShpfyDraftOrdersAPI.Codeunit.al
Outdated
Show resolved
Hide resolved
Apps/W1/Shopify/app/src/Invoicing/Codeunits/ShpfyDraftOrdersAPI.Codeunit.al
Outdated
Show resolved
Hide resolved
Apps/W1/Shopify/app/src/Invoicing/Codeunits/ShpfyPostedInvoiceExport.Codeunit.al
Outdated
Show resolved
Hide resolved
Apps/W1/Shopify/app/src/Invoicing/Codeunits/ShpfyPostedInvoiceExport.Codeunit.al
Outdated
Show resolved
Hide resolved
Apps/W1/Shopify/app/src/Invoicing/PageExt/ShpfySalesInvoiceUpdate.PageExt.al
Outdated
Show resolved
Hide resolved
Apps/W1/Shopify/app/src/Invoicing/Codeunits/ShpfyFulfillmentAPI.Codeunit.al
Outdated
Show resolved
Hide resolved
Apps/W1/Shopify/app/src/Payments/Codeunits/ShpfyPaymentTermAPI.Codeunit.al
Outdated
Show resolved
Hide resolved
Apps/W1/Shopify/app/src/Invoicing/Codeunits/ShpfyDraftOrdersAPI.Codeunit.al
Outdated
Show resolved
Hide resolved
Apps/W1/Shopify/app/src/Invoicing/Codeunits/ShpfyDraftOrdersAPI.Codeunit.al
Outdated
Show resolved
Hide resolved
Apps/W1/Shopify/app/src/Invoicing/Codeunits/ShpfyPostedInvoiceExport.Codeunit.al
Outdated
Show resolved
Hide resolved
Apps/W1/Shopify/app/src/Invoicing/Codeunits/ShpfyPostedInvoiceExport.Codeunit.al
Outdated
Show resolved
Hide resolved
Apps/W1/Shopify/app/src/Invoicing/Codeunits/ShpfyPostedInvoiceExport.Codeunit.al
Outdated
Show resolved
Hide resolved
Apps/W1/Shopify/app/src/Invoicing/Reports/ShpfyExportInvoiceToShpfy.Report.al
Outdated
Show resolved
Hide resolved
Apps/W1/Shopify/app/src/Invoicing/Reports/ShpfyExportInvoiceToShpfy.Report.al
Outdated
Show resolved
Hide resolved
Apps/W1/Shopify/app/src/Payments/Codeunits/ShpfyPaymentTermAPI.Codeunit.al
Outdated
Show resolved
Hide resolved
Apps/W1/Shopify/app/src/Payments/Codeunits/ShpfyPaymentTermAPI.Codeunit.al
Outdated
Show resolved
Hide resolved
Apps/W1/Shopify/app/src/PermissionSets/ShpfyEdit.PermissionSet.al
Outdated
Show resolved
Hide resolved
Apps/W1/Shopify/app/src/Invoicing/Codeunits/ShpfyDraftOrdersAPI.Codeunit.al
Outdated
Show resolved
Hide resolved
Apps/W1/Shopify/app/src/Invoicing/Codeunits/ShpfyDraftOrdersAPI.Codeunit.al
Outdated
Show resolved
Hide resolved
Apps/W1/Shopify/app/src/Invoicing/Codeunits/ShpfyDraftOrdersAPI.Codeunit.al
Outdated
Show resolved
Hide resolved
Apps/W1/Shopify/app/src/Invoicing/Codeunits/ShpfyFulfillmentAPI.Codeunit.al
Outdated
Show resolved
Hide resolved
Apps/W1/Shopify/app/src/Invoicing/Codeunits/ShpfyFulfillmentAPI.Codeunit.al
Outdated
Show resolved
Hide resolved
Apps/W1/Shopify/app/src/Invoicing/Codeunits/ShpfyPostedInvoiceExport.Codeunit.al
Outdated
Show resolved
Hide resolved
Apps/W1/Shopify/app/src/Invoicing/Codeunits/ShpfyFulfillmentAPI.Codeunit.al
Outdated
Show resolved
Hide resolved
Apps/W1/Shopify/app/src/GraphQL/Codeunits/ShpfyGQLDraftOrderComplete.Codeunit.al
Outdated
Show resolved
Hide resolved
Apps/W1/Shopify/app/src/Invoicing/Codeunits/ShpfyPostedInvoiceExport.Codeunit.al
Outdated
Show resolved
Hide resolved
Apps/W1/Shopify/app/src/Payments/Codeunits/ShpfyPaymentTermAPI.Codeunit.al
Outdated
Show resolved
Hide resolved
Apps/W1/Shopify/app/src/Invoicing/Codeunits/ShpfyFulfillmentAPI.Codeunit.al
Outdated
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: 18
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (10)
- Apps/W1/Shopify/app/src/GraphQL/Codeunits/ShpfyGQLDraftOrderComplete.Codeunit.al (1 hunks)
- Apps/W1/Shopify/app/src/GraphQL/Codeunits/ShpfyGQLGetFulfillments.Codeunit.al (1 hunks)
- Apps/W1/Shopify/app/src/GraphQL/Enums/ShpfyGraphQLType.Enum.al (1 hunks)
- Apps/W1/Shopify/app/src/Integration/Codeunits/ShpfyAuthenticationMgt.Codeunit.al (1 hunks)
- Apps/W1/Shopify/app/src/Invoicing/Codeunits/ShpfyDraftOrdersAPI.Codeunit.al (1 hunks)
- Apps/W1/Shopify/app/src/Invoicing/Codeunits/ShpfyFulfillmentAPI.Codeunit.al (1 hunks)
- Apps/W1/Shopify/app/src/Invoicing/Codeunits/ShpfyPostedInvoiceExport.Codeunit.al (1 hunks)
- Apps/W1/Shopify/app/src/Invoicing/Reports/ShpfyExportInvoiceToShpfy.Report.al (1 hunks)
- Apps/W1/Shopify/app/src/Payments/Codeunits/ShpfyPaymentTermAPI.Codeunit.al (1 hunks)
- Apps/W1/Shopify/app/src/Payments/Pages/ShpfyPaymentTermsMapping.Page.al (1 hunks)
Additional comments not posted (8)
Apps/W1/Shopify/app/src/GraphQL/Codeunits/ShpfyGQLGetFulfillments.Codeunit.al (1)
12-15
: GraphQL Query Construction: Ensure Security and CorrectnessThe hardcoded GraphQL query string in
GetGraphQL
could potentially expose the system to injection attacks if the substitution tokens (e.g.,{{OrderId}}
) are not properly sanitized elsewhere. It's also important to ensure that the query string is constructed correctly for the intended operations.Apps/W1/Shopify/app/src/GraphQL/Codeunits/ShpfyGQLDraftOrderComplete.Codeunit.al (1)
14-17
: GraphQL Mutation String: Verify Security and CorrectnessLike the previous file, the hardcoded GraphQL mutation string in
GetGraphQL
is potentially vulnerable to injection attacks if{{DraftOrderId}}
is not sanitized. Ensure that inputs are sanitized before their insertion into the query string.Apps/W1/Shopify/app/src/Invoicing/Reports/ShpfyExportInvoiceToShpfy.Report.al (2)
17-56
: Review of DataItem and Triggers in SalesInvoiceHeader
- Error Handling: Proper error handling in
OnPreDataItem
trigger with clear messaging ifShopCode
is empty.- Filter Setup: Setting range on
"Shpfy Order Id"
to zero inOnPreDataItem
seems intentional, likely to filter out invoices already processed. Confirmation on this logic would be prudent.- Dialog Management: Good use of dialog for user interaction in
OnPreDataItem
andOnAfterGetRecord
. However, ensure thatProcessDialog.Close()
inOnPostDataItem
is adequately tested to handle scenarios where it might be called unexpectedly.- Background Processing: Utilization of
ShpfyBackgroundSyncs.InventorySync
inOnPostDataItem
to sync inventory after processing is a good integration point, but ensure this does not lead to performance issues if the inventory size is large.Overall, the triggers are well-structured with clear separation of concerns, but adding more comments explaining the logic, especially around dialog handling and background processes, would improve maintainability.
58-81
: Review of Request Page LayoutThe request page layout is straightforward and user-friendly:
- Mandatory Field: The
ShopCode
field is marked as mandatory, which is essential for processing the report.- Lookup Functionality: Proper lookup setup for
ShopCode
to ensure users can easily select the correct shop.This setup aligns well with user experience best practices by providing necessary information and controls to the user effectively.
Apps/W1/Shopify/app/src/Integration/Codeunits/ShpfyAuthenticationMgt.Codeunit.al (1)
18-18
: Extended Scope ManagementThe
ScopeTxt
label has been updated to includewrite_payment_terms
, which is critical for managing payment terms in Shopify. This change is essential for the new functionalities around payment terms synchronization.Ensure that all dependent functionalities that utilize this scope are tested to handle the new scope, especially in OAuth flows and permission checks.
Apps/W1/Shopify/app/src/GraphQL/Enums/ShpfyGraphQLType.Enum.al (1)
403-422
: Review of New Enum Values for GraphQL TypesThe addition of new enum values (
DraftOrderComplete
,FulfillOrder
,GetPaymentTerms
,GetFulfillments
) is aligned with the new functionalities described in the PR. Each value is well-defined with appropriate captions and linked to the correct implementation.Ensure that these new GraphQL types are integrated into the system and tested thoroughly, particularly in scenarios where they are expected to be triggered.
Apps/W1/Shopify/app/src/Invoicing/Codeunits/ShpfyDraftOrdersAPI.Codeunit.al (1)
161-199
: Optimize dictionary usage in GraphQL parameters.In the
CompleteDraftOrder
function, consider using a more efficient way to handle the dictionary of parameters. Currently, the dictionary is used to add parameters one by one, which can be optimized.- Parameters.Add('DraftOrderId', Format(DraftOrderId)); - Parameters.Add('NumberOfOrders', Format(NumberOfLines)); + Parameters := Dictionary of [Text, Text]{'DraftOrderId': Format(DraftOrderId), 'NumberOfOrders': Format(NumberOfLines)};Likely invalid or redundant comment.
Apps/W1/Shopify/app/src/Invoicing/Codeunits/ShpfyPostedInvoiceExport.Codeunit.al (1)
414-425
: Clarify document linking logic.The function
AddDocumentLinkToBCDocument
should have more detailed comments explaining the logic, especially how it interacts with Shopify and Business Central documents. The existing comment "nit: AddDocumentLinkToPostedInvoice" suggests enhancing the naming for clarity.- DocLinkToBCDoc."Shopify Document Type" := "Shpfy Shop Document Type"::"Shopify Shop Order"; + // Improved naming for clarity + DocLinkToBCDoc."Shopify Document Type" := "Shpfy Shop Document Type"::"Shopify Sales Order";
Apps/W1/Shopify/app/src/Invoicing/Codeunits/ShpfyDraftOrdersAPI.Codeunit.al
Show resolved
Hide resolved
Apps/W1/Shopify/app/src/Invoicing/Codeunits/ShpfyDraftOrdersAPI.Codeunit.al
Outdated
Show resolved
Hide resolved
Apps/W1/Shopify/app/src/Invoicing/Codeunits/ShpfyDraftOrdersAPI.Codeunit.al
Show resolved
Hide resolved
Apps/W1/Shopify/app/src/GraphQL/Codeunits/ShpfyGQLGetFulfillments.Codeunit.al
Show resolved
Hide resolved
Apps/W1/Shopify/app/src/Invoicing/Codeunits/ShpfyPostedInvoiceExport.Codeunit.al
Outdated
Show resolved
Hide resolved
Apps/W1/Shopify/app/src/Invoicing/Codeunits/ShpfyFulfillmentAPI.Codeunit.al
Outdated
Show resolved
Hide resolved
Apps/W1/Shopify/app/src/Invoicing/Codeunits/ShpfyFulfillmentAPI.Codeunit.al
Outdated
Show resolved
Hide resolved
Apps/W1/Shopify/app/src/Payments/Codeunits/ShpfyPaymentTermAPI.Codeunit.al
Outdated
Show resolved
Hide resolved
Apps/W1/Shopify/app/src/Payments/Codeunits/ShpfyPaymentTermAPI.Codeunit.al
Outdated
Show resolved
Hide resolved
Apps/W1/Shopify/app/src/Invoicing/Codeunits/ShpfyDraftOrdersAPI.Codeunit.al
Show resolved
Hide resolved
Apps/W1/Shopify/app/src/Invoicing/Codeunits/ShpfyDraftOrdersAPI.Codeunit.al
Outdated
Show resolved
Hide resolved
Apps/W1/Shopify/app/src/Invoicing/Codeunits/ShpfyFulfillmentAPI.Codeunit.al
Outdated
Show resolved
Hide resolved
Apps/W1/Shopify/app/src/Invoicing/Codeunits/ShpfyFulfillmentAPI.Codeunit.al
Outdated
Show resolved
Hide resolved
Apps/W1/Shopify/app/src/Invoicing/Codeunits/ShpfyDraftOrdersAPI.Codeunit.al
Show resolved
Hide resolved
Apps/W1/Shopify/app/src/Payments/Codeunits/ShpfyPaymentTermAPI.Codeunit.al
Outdated
Show resolved
Hide resolved
Apps/W1/Shopify/app/src/Payments/Tables/ShpfyPaymentTerm.Table.al
Outdated
Show resolved
Hide resolved
Apps/W1/Shopify/app/src/Invoicing/Codeunits/ShpfyUpdateSalesInvoice.Codeunit.al
Show resolved
Hide resolved
Apps/W1/Shopify/app/src/Invoicing/Codeunits/ShpfyPostedInvoiceExport.Codeunit.al
Outdated
Show resolved
Hide resolved
Apps/W1/Shopify/app/src/Invoicing/Codeunits/ShpfyFulfillmentAPI.Codeunit.al
Outdated
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: 7
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (11)
- Apps/W1/Shopify/app/src/Base/Pages/ShpfyShopCard.Page.al (4 hunks)
- Apps/W1/Shopify/app/src/GraphQL/Enums/ShpfyGraphQLType.Enum.al (1 hunks)
- Apps/W1/Shopify/app/src/Invoicing/Codeunits/ShpfyDraftOrdersAPI.Codeunit.al (1 hunks)
- Apps/W1/Shopify/app/src/Invoicing/Codeunits/ShpfyFulfillmentAPI.Codeunit.al (1 hunks)
- Apps/W1/Shopify/app/src/Invoicing/Codeunits/ShpfyPostedInvoiceExport.Codeunit.al (1 hunks)
- Apps/W1/Shopify/app/src/Invoicing/Codeunits/ShpfyUpdateSalesInvoice.Codeunit.al (1 hunks)
- Apps/W1/Shopify/app/src/Invoicing/PageExt/ShpfySalesInvoiceUpdate.PageExt.al (1 hunks)
- Apps/W1/Shopify/app/src/Invoicing/Reports/ShpfySyncInvoicesToShpfy.Report.al (1 hunks)
- Apps/W1/Shopify/app/src/Invoicing/Tables/ShpfyInvoiceHeader.Table.al (1 hunks)
- Apps/W1/Shopify/app/src/Payments/Codeunits/ShpfyPaymentTermsAPI.Codeunit.al (1 hunks)
- Apps/W1/Shopify/app/src/Payments/Tables/ShpfyPaymentTerms.Table.al (1 hunks)
Files skipped from review as they are similar to previous changes (6)
- Apps/W1/Shopify/app/src/Base/Pages/ShpfyShopCard.Page.al
- Apps/W1/Shopify/app/src/GraphQL/Enums/ShpfyGraphQLType.Enum.al
- Apps/W1/Shopify/app/src/Invoicing/Codeunits/ShpfyFulfillmentAPI.Codeunit.al
- Apps/W1/Shopify/app/src/Invoicing/Codeunits/ShpfyUpdateSalesInvoice.Codeunit.al
- Apps/W1/Shopify/app/src/Invoicing/PageExt/ShpfySalesInvoiceUpdate.PageExt.al
- Apps/W1/Shopify/app/src/Invoicing/Tables/ShpfyInvoiceHeader.Table.al
Additional comments not posted (4)
Apps/W1/Shopify/app/src/Payments/Tables/ShpfyPaymentTerms.Table.al (1)
47-50
: Confirm intention behind editable "Is Primary" field.The field "Is Primary" is set as editable. Please confirm if this is intended to allow users to set primary payment terms directly.
Apps/W1/Shopify/app/src/Payments/Codeunits/ShpfyPaymentTermsAPI.Codeunit.al (1)
38-42
: Confirm the usage of ShopCode in SetShop.The procedure
SetShop
sets the shop code for the payment terms API. Verify that thisShopCode
is being utilized correctly across all relevant procedures in this codeunit.Apps/W1/Shopify/app/src/Invoicing/Reports/ShpfySyncInvoicesToShpfy.Report.al (2)
24-35
: Ensure handling of non-GUI scenarios.The trigger
OnPreDataItem
uses a GUI dialog. Ensure that there are appropriate fallbacks or notifications when the GUI is not allowed.
54-54
: Review the necessity of inventory sync in this context.The procedure
InventorySync
is called after syncing invoices. Confirm whether syncing inventory is necessary in this context or if it could be handled separately to improve performance.
Apps/W1/Shopify/app/src/Payments/Codeunits/ShpfyPaymentTermsAPI.Codeunit.al
Show resolved
Hide resolved
Apps/W1/Shopify/app/src/Invoicing/Codeunits/ShpfyDraftOrdersAPI.Codeunit.al
Show resolved
Hide resolved
Apps/W1/Shopify/app/src/Invoicing/Codeunits/ShpfyPostedInvoiceExport.Codeunit.al
Show resolved
Hide resolved
Apps/W1/Shopify/app/src/Invoicing/Codeunits/ShpfyPostedInvoiceExport.Codeunit.al
Show resolved
Hide resolved
Apps/W1/Shopify/app/src/Invoicing/Codeunits/ShpfyPostedInvoiceExport.Codeunit.al
Outdated
Show resolved
Hide resolved
Apps/W1/Shopify/app/src/Invoicing/Codeunits/ShpfyDraftOrdersAPI.Codeunit.al
Show resolved
Hide resolved
Apps/W1/Shopify/app/src/Invoicing/Codeunits/ShpfyFulfillmentAPI.Codeunit.al
Outdated
Show resolved
Hide resolved
Apps/W1/Shopify/app/src/Invoicing/Codeunits/ShpfyPostedInvoiceExport.Codeunit.al
Outdated
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: 4
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (3)
- Apps/W1/Shopify/app/src/Base/Pages/ShpfyShopCard.Page.al (4 hunks)
- Apps/W1/Shopify/app/src/Invoicing/Codeunits/ShpfyPostedInvoiceExport.Codeunit.al (1 hunks)
- Apps/W1/Shopify/app/src/Invoicing/Reports/ShpfySyncInvoicesToShpfy.Report.al (1 hunks)
Files not reviewed due to errors (1)
- Apps/W1/Shopify/app/src/Invoicing/Codeunits/ShpfyPostedInvoiceExport.Codeunit.al (no review received)
Additional comments not posted (3)
Apps/W1/Shopify/app/src/Base/Pages/ShpfyShopCard.Page.al (3)
248-252
: Field addition approved: "Items Mapped to Products".The added field enhances control over which items are synchronized from Posted Sales Invoices to Shopify, ensuring only mapped items are considered.
532-537
: Field addition approved: "Posted Invoice Sync".This field allows users to enable or disable the synchronization of posted sales invoices to Shopify, providing flexibility in managing data flow.
665-677
: Action addition approved: "Payment Terms Mapping".This action facilitates the mapping of Shopify payment terms to related payment terms, enhancing the configurability and accuracy of payment data synchronization.
Apps/W1/Shopify/app/src/Invoicing/Reports/ShpfySyncInvoicesToShpfy.Report.al
Show resolved
Hide resolved
Apps/W1/Shopify/app/src/Invoicing/Reports/ShpfySyncInvoicesToShpfy.Report.al
Outdated
Show resolved
Hide resolved
Apps/W1/Shopify/app/src/Invoicing/Reports/ShpfySyncInvoicesToShpfy.Report.al
Outdated
Show resolved
Hide resolved
Apps/W1/Shopify/app/src/Invoicing/Reports/ShpfySyncInvoicesToShpfy.Report.al
Show resolved
Hide resolved
Apps/W1/Shopify/app/src/Invoicing/Reports/ShpfySyncInvoicesToShpfy.Report.al
Outdated
Show resolved
Hide resolved
Apps/W1/Shopify/app/src/Invoicing/Codeunits/ShpfyDraftOrdersAPI.Codeunit.al
Show resolved
Hide resolved
Apps/W1/Shopify/app/src/Invoicing/Codeunits/ShpfyPostedInvoiceExport.Codeunit.al
Outdated
Show resolved
Hide resolved
Apps/W1/Shopify/app/src/Invoicing/Codeunits/ShpfyPostedInvoiceExport.Codeunit.al
Show resolved
Hide resolved
Apps/W1/Shopify/app/src/Invoicing/Codeunits/ShpfyPostedInvoiceExport.Codeunit.al
Outdated
Show resolved
Hide resolved
Apps/W1/Shopify/app/src/Invoicing/Reports/ShpfySyncInvoicesToShpfy.Report.al
Outdated
Show resolved
Hide resolved
Apps/W1/Shopify/app/src/Invoicing/Codeunits/ShpfyDraftOrdersAPI.Codeunit.al
Outdated
Show resolved
Hide resolved
Apps/W1/Shopify/app/src/Invoicing/Codeunits/ShpfyDraftOrdersAPI.Codeunit.al
Outdated
Show resolved
Hide resolved
Apps/W1/Shopify/app/src/Invoicing/Codeunits/ShpfyDraftOrdersAPI.Codeunit.al
Outdated
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: 6
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (5)
- Apps/W1/Shopify/app/src/Base/Pages/ShpfyShopCard.Page.al (4 hunks)
- Apps/W1/Shopify/app/src/Invoicing/Codeunits/ShpfyDraftOrdersAPI.Codeunit.al (1 hunks)
- Apps/W1/Shopify/app/src/Invoicing/Codeunits/ShpfyPostedInvoiceExport.Codeunit.al (1 hunks)
- Apps/W1/Shopify/app/src/Invoicing/Reports/ShpfySyncInvoicesToShpfy.Report.al (1 hunks)
- Apps/W1/Shopify/app/src/Payments/Tables/ShpfyPaymentTerms.Table.al (1 hunks)
Files skipped from review as they are similar to previous changes (3)
- Apps/W1/Shopify/app/src/Base/Pages/ShpfyShopCard.Page.al
- Apps/W1/Shopify/app/src/Invoicing/Reports/ShpfySyncInvoicesToShpfy.Report.al
- Apps/W1/Shopify/app/src/Payments/Tables/ShpfyPaymentTerms.Table.al
Additional comments not posted (2)
Apps/W1/Shopify/app/src/Invoicing/Codeunits/ShpfyDraftOrdersAPI.Codeunit.al (2)
1-8
: Namespace and Using Directives are Correctly DefinedThese declarations are standard and correctly placed, facilitating proper organization and accessibility of external functionalities.
9-11
: Clear and Informative Class SummaryThe class summary provides a clear description of the purpose and functionality of the codeunit, which is beneficial for maintenance and understanding.
Apps/W1/Shopify/app/src/Invoicing/Codeunits/ShpfyDraftOrdersAPI.Codeunit.al
Show resolved
Hide resolved
Apps/W1/Shopify/app/src/Invoicing/Codeunits/ShpfyDraftOrdersAPI.Codeunit.al
Show resolved
Hide resolved
Apps/W1/Shopify/app/src/Invoicing/Codeunits/ShpfyDraftOrdersAPI.Codeunit.al
Show resolved
Hide resolved
Apps/W1/Shopify/app/src/Invoicing/Codeunits/ShpfyPostedInvoiceExport.Codeunit.al
Outdated
Show resolved
Hide resolved
Apps/W1/Shopify/app/src/Invoicing/Codeunits/ShpfyPostedInvoiceExport.Codeunit.al
Show resolved
Hide resolved
Apps/W1/Shopify/app/src/Invoicing/Codeunits/ShpfyPostedInvoiceExport.Codeunit.al
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
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- Apps/W1/Shopify/app/src/Invoicing/Reports/ShpfySyncInvoicesToShpfy.Report.al (1 hunks)
Files skipped from review as they are similar to previous changes (1)
- Apps/W1/Shopify/app/src/Invoicing/Reports/ShpfySyncInvoicesToShpfy.Report.al
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: CHILL
Files selected for processing (2)
- Apps/W1/Shopify/app/src/Invoicing/Codeunits/ShpfyPostedInvoiceExport.Codeunit.al (1 hunks)
- Apps/W1/Shopify/app/src/Payments/Codeunits/ShpfyPaymentTermsAPI.Codeunit.al (1 hunks)
Files skipped from review as they are similar to previous changes (1)
- Apps/W1/Shopify/app/src/Payments/Codeunits/ShpfyPaymentTermsAPI.Codeunit.al
Additional comments not posted (7)
Apps/W1/Shopify/app/src/Invoicing/Codeunits/ShpfyPostedInvoiceExport.Codeunit.al (7)
15-19
: Variable declarations are well-defined.The variables are appropriately scoped and relevant to the functionalities of the codeunit, such as handling Shopify drafts, fulfillment, and JSON operations.
79-85
: Well-implemented fulfillment creation logic.The
CreateFulfillmentsForShopifyOrder
procedure is succinct and effectively handles the creation of Shopify fulfillments.
341-376
: Comprehensive and detailed line mapping logic.The
MapSalesInvoiceLine
function comprehensively handles the mapping of sales invoice lines to Shopify order lines, including handling product variants and tax calculations.
438-449
: Effective linking of Shopify to Business Central documents.The
AddDocumentLinkToBCDocument
function effectively creates links between Shopify and Business Central documents, ensuring data coherence and traceability.
451-454
: Correct calculation of total order lines.The
GetNumberOfLines
function correctly calculates the total number of lines in an order by combining counts from tax lines and order lines.
424-427
: Proper handling of API response errors.The
IsSuccess
function effectively checks for errors in the JSON response from Shopify API calls, ensuring that only successful responses proceed further in the logic.
429-436
: Well-implemented invoice header creation.The
CreateShpfyInvoiceHeader
function is well-implemented, effectively creating Shopify invoice headers with appropriate validations.
This pull request does not have a related issue as it's part of the delivery for development agreed directly with @AndreiPanko
Here's a quick summary:
Created a report to export Posted Sales Invoices to Shopify as Orders.
Shopify Shop has a new action "Payment Terms Mapping" that takes the user to the new page "Shopify Payment Terms Mapping". Shopify Payment Terms cannot be manually inserted or deleted, they can only be refreshed with an action that pulls them from Shopify. By default FIXED Payment Term is selected as primary during the synchronization of the Payment Terms.
Before Posted Sales Invoices can be synced, a payment term code from Shopify needs to be mapped to the Business Central one or a primary Shopify Payment Term needs to be selected with a Boolean "Is Primary" - this payment term will be used if the Posted Sales Invoice payment terms code doesn't have a mapping to Shopify. This payment term code will also be filled during the sync of Shopify orders.
A new Scope is required. To update payment terms on a draft order – write_payment_terms (read_payment_terms scope was removed because it causes strange behavior - every time a Shopify shop card is opened it states that the scope has changed and requires authorization. write_payment_terms overwrites read_payment_terms). To create a draft order - write_draft_orders.
To export Posted Sales Invoices, here are some conditions that they need to meet:
If one of the previous conditions is not met, the Posted Sales Invoice "Shopify Order Id" field will be marked with -2. If the GraphQL request fails, then the field will be marked with -1. If everything goes correctly, the field will be filled with the created Shopify Order ID. Users can modify the "Shopify Order Id" field in the posted document (allowed changes from -1 or -2 to 0. The functionality works the same as in the Update Posted Shipment document.)
This export can be executed with a new action that is added to Shopify Shop - "Sync Posted Sales Invoices" or just by running the report "Sync Invoices to Shopify". The draft order in Shopify is created with the values from the Posted Sales Invoice order and lines. If the Remaining Amount of the Posted Sales Invoice is 0, then the order is marked as paid, otherwise, the status will be "Payment pending" and a mapped Payment Terms code will be sent out. After that, the created draft order will be completed and the Shopify order created. All orders are automatically fulfilled. To achieve “paid” status on the Shopify order – use the Manual payment function in the Shopify Order card. Once the report is executed, the Inventory will be synced.
Order sync will import created orders, then check if the order is present in the new Shopify Invoice Header table (this table is filled with the Shopify order ID of the exported Posted Sales Invoice) and if so:
A few details about the mapping of the values and some additional functionality:
Out of scope functionalities:
Known issues with different currencies (that's why Posted Sales Invoices with different currencies than Shopify Shop cannot be exported at the moment):
Summary by CodeRabbit