Add Vertex, TaxJar, Avalara #59

oliverheywood451 · 2022-02-01T19:46:27Z

VertexOCIntegrationCommand.cs with two methods for calculating tax. The functionality of the actual integration. Also lots of models and mappers.
4 general exceptions to handle various types of errors inside an integration
A guide for adding integrations, a table with a list of integrations, and guide specifically for the vertex integration
Unit tests that mock vertex api responses using flurl

Typos on Integrations README

…1/ordercloud-dotnet-catalyst into tax-integrations

…tnet-catalyst into tax-integrations

library/OrderCloud.Catalyst/Integrations/Exceptions/IntegrationAuthFailedException.cs

smitbmx

Hi @oliverheywood451, please check the comments and let me know if we need a separate call, thank you in advance!

library/OrderCloud.Catalyst/Integrations/Exceptions/IntegrationErrorResponseException.cs.cs

library/OrderCloud.Catalyst/Integrations/Exceptions/IntegrationNoResponseException.cs.cs

smitbmx · 2022-02-02T11:28:00Z

library/OrderCloud.Catalyst/Integrations/Implementations/Vertex/Mappers/VertexRequestMapper.cs

+		public static VertexCalculateTaxRequest ToVertexCalculateTaxRequest(this OrderWorksheet order, List<OrderPromotion> promosOnOrder, string companyCode, VertexSaleMessageType type)
+		{
+			var itemLines = order.LineItems.Select(li => ToVertexLineItem(li));
+			var shippingLines = order.ShipEstimateResponse.ShipEstimates.Select(se =>


What do you think of avoiding using the var? Because it's not clear for reviewers which type is it and decreases the speed of reading and understanding the code. The IEnumerable gives more information in this case :)

Using var is a pattern that's pretty engrained in me from all the C# projects at four51. I do see how it makes reviewing code hard if you're not viewing in the VS editor (where hovering shows you the type).

I'm willing to consider this. It would be a rather large code change throughout the project. Do you know what the OC platform code does? That's often what we model these other tools on.

My personal opinion is to use it only if the type can't be easily inferred by its context/usage. Adds a lot of clutter which in my opinion actually makes it harder to read (more mental load) and is unnecessarily verbose in many situations

ie:

Car car = new Car();

library/OrderCloud.Catalyst/Integrations/Implementations/Vertex/Mappers/VertexRequestMapper.cs

smitbmx · 2022-02-02T11:57:30Z

.../OrderCloud.Catalyst/Integrations/Implementations/Vertex/Models/VertexCalculateTaxRequest.cs

+{
+	public class VertexCalculateTaxRequest
+	{
+		public string saleMessageType { get; set; }


Just curious, why do we have the public properties which start with a lower case? Do we have a code notation standard for the OC Catalyst?

They are lowercase because when they are serialized to JSON they need to be lower case to match the property names the Vertex API is expecting.

We don't have a code notation standard for the OC Catalyst - I can create an issue for that. If you want to work on it that would be awesome.

If we really cared, we could use JsonProperty to determine how something is serialized. For example

[JsonProperty("saleMessageType")] public string SaleMessageType {get; set;}

But it seems like an abstraction just for the sake of abstraction. I personally like that the property names match exactly what the vertex properties are.

library/OrderCloud.Catalyst/Integrations/Implementations/Vertex/Models/VertexException.cs

library/OrderCloud.Catalyst/Integrations/OCIntegrationConfig.cs

…an intermediate model. unit tests are broken now.

library/OrderCloud.Catalyst/Errors/CatalystBaseException.cs

…iuations call for different account credentials.

oliverheywood451 and others added 28 commits January 26, 2022 10:59

adding vertex and writing a contributing guide

f9d0f92

update readme and folder structure

e0a6b18

wording tweak

bfd45b6

wording

281ce1f

more tweaks

591f0f6

last tweaks

f987d94

more detail about tests

2eddce5

wording

6fa6de2

Update README.md

cc92cb6

Update README.md

3ea532d

Update README.md

405fb34

Update README.md

40d4e49

Update README.md

551254a

Update README.md

0ad08c7

Update README.md

ae2d96b

Update README.md

25a89df

Typos on Integrations README

0f14441

Merge pull request #1 from crhistianramirez/patch-2

0bf6eec

Typos on Integrations README

Update README.md

81db3b9

remove duplicates

432abdf

Merge branch 'tax-integrations' of https://github.com/oliverheywood45…

4e4e713

…1/ordercloud-dotnet-catalyst into tax-integrations

error handling is solid. now need to do tests

266bde9

couple readme updates

6263c51

solid progress on the testing framework

553d5a1

more tests

03c56a6

readme updates

ae11694

dont actually want this

df73caa

Merge branch 'dev' of https://github.com/ordercloud-api/ordercloud-do…

bbd8aa4

…tnet-catalyst into tax-integrations

smitbmx reviewed Feb 2, 2022

View reviewed changes

library/OrderCloud.Catalyst/Integrations/Exceptions/IntegrationAuthFailedException.cs Outdated Show resolved Hide resolved

smitbmx reviewed Feb 2, 2022

View reviewed changes

oliverheywood451 added 8 commits February 7, 2022 09:23

switch exception type from int to HttpStatusCode

995b14c

change double to decimal

1848632

putting shipping code into a const

d214766

update readme

15e7509

taxjar is working. still needs tests and error handling.

56023ea

vertex looking pretty good

0567b83

refactored Interface to not rely on OrderWorksheet. Instead, there's …

080dd6d

…an intermediate model. unit tests are broken now.

units tests are back. nice!

eaae6f5

crhistianramirez approved these changes Feb 9, 2022

View reviewed changes

library/OrderCloud.Catalyst/Errors/CatalystBaseException.cs Outdated Show resolved Hide resolved

Avalara looks good! still needs tests and readme

b07c270

oliverheywood451 changed the title ~~Add Vertex Integration as a first integration to serve as a template~~ Add Vertex, TaxJar, Avalara Feb 9, 2022

oliverheywood451 added 6 commits February 11, 2022 11:55

updated to support config overrides. This is important if different s…

7f53c36

…iuations call for different account credentials.

move to separate projects

4b1ada0

updating readmes for project structure

59bf246

remove this as were not ready to be that public

aa73883

readme updates

094d5cf

readme and package description updates

4146a6e

oliverheywood451 merged commit 0e366de into ordercloud-api:dev Feb 15, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add Vertex, TaxJar, Avalara #59

Add Vertex, TaxJar, Avalara #59

oliverheywood451 commented Feb 1, 2022

smitbmx left a comment

smitbmx Feb 2, 2022

oliverheywood451 Feb 7, 2022 •

edited

Loading

crhistianramirez Feb 9, 2022 •

edited

Loading

smitbmx Feb 2, 2022

oliverheywood451 Feb 7, 2022

oliverheywood451 Feb 7, 2022

crhistianramirez Feb 9, 2022

Add Vertex, TaxJar, Avalara #59

Add Vertex, TaxJar, Avalara #59

Conversation

oliverheywood451 commented Feb 1, 2022

smitbmx left a comment

Choose a reason for hiding this comment

smitbmx Feb 2, 2022

Choose a reason for hiding this comment

oliverheywood451 Feb 7, 2022 • edited Loading

Choose a reason for hiding this comment

crhistianramirez Feb 9, 2022 • edited Loading

Choose a reason for hiding this comment

smitbmx Feb 2, 2022

Choose a reason for hiding this comment

oliverheywood451 Feb 7, 2022

Choose a reason for hiding this comment

oliverheywood451 Feb 7, 2022

Choose a reason for hiding this comment

crhistianramirez Feb 9, 2022

Choose a reason for hiding this comment

oliverheywood451 Feb 7, 2022 •

edited

Loading

crhistianramirez Feb 9, 2022 •

edited

Loading