-
Notifications
You must be signed in to change notification settings - Fork 622
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
Shopify Connector Tax ID, Multiple Company Locations, Tax ID export, Company Mapping by Tax ID, Payment Terms Export/Import, Company External ID #27577
base: main
Are you sure you want to change the base?
Conversation
…as Company export
…onal action to open Company Locations
… for Company Locations
Could not find linked issues in the pull request description. Please make sure the pull request description contains a line that contains 'Fixes #' followed by the issue number being fixed. Use that pattern for every issue you want to link. |
@@ -28,7 +28,7 @@ page 30159 "Shpfy Catalogs" | |||
field("Customer No."; Rec."Customer No.") | |||
{ | |||
ApplicationArea = All; | |||
Visible = false; | |||
Editable = false; |
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.
@AndreiPanko this means each catalog will be assigned a customer and only that customer will be used to calculate the price. Not the settings in the catalogs page. ("Customer Price Group", "Customer Disc. Group", "Allow Line Disc.")
Is that the intention? Then shouldn't we remove those settings from the page?
} | ||
field(14; "Shpfy Payment Terms Id"; BigInteger) | ||
{ | ||
Caption = 'Shpfy Payment Terms Id'; |
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.
Caption = 'Shpfy Payment Terms Id'; | |
Caption = 'Shopify Payment Terms Id'; |
ShopifyCustomer: Record "Shpfy Customer"; | ||
CustomerMapping: Codeunit "Shpfy Customer Mapping"; | ||
PhoneFilter: Text; | ||
ShpfyCompByEmailPhone: Codeunit "Shpfy Comp. By Email/Phone"; |
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.
Please fix the variable naming.
If same name exists in Base Application then Shopify prefix, otherwise no prefix.
Example:
ShopifyCustomer: Record "Shpfy Customer" -> customer exists in Base Application
Catalog: Record "Shpfy Catalog" -> catalog does not exist in Base Application
@@ -199,4 +208,16 @@ codeunit 30284 "Shpfy Company Export" | |||
begin | |||
CreateCustomers := NewCustomers; | |||
end; | |||
|
|||
local procedure GetShpfyPaymentTermsIdFromCustomer(Customer: Record Customer; var ShpfyPaymentTermsId: BigInteger): Boolean |
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.
There's no need to use Shpfy in the procedure names. Just use Shopify
Have you considered upgrade scenarios? What will happen for already imported companies? How will locations sync in that case? |
@onbuyuka resolved your comments about the naming issues Regarding upgrade - it updates the locations well when you Sync Companies, but the problem is that update happens only if Company was updated on the date later than Last Synchronization for Shopify Companies. So, if somebody has old version and multiple location existing and had not updated company in Shopify in meantime won't get these updated locations in BC. Since this is how the Company Sync. worked before is this correct behavior? |
This pull request does not have a related issue as it's part of the delivery for development agreed directly with @AndreiPanko
Implementation 1 (Customer No. in Shopify Catalogs)
Implementation 2 (Import of Multiple Company Locations)
Implementation 3 (Tax ID export and Company/Customer mapping by Tax Id)
Implementation 4 (Company Location Payment Terms Export/Import)
Implementation 5 (Populate External ID during the export customer as a company)