Skip to content
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

Missing fields TaxRatePercent and CountryOfPublication #11

Closed
dgil-unedbarbastro opened this issue Jun 16, 2021 · 7 comments
Closed

Missing fields TaxRatePercent and CountryOfPublication #11

dgil-unedbarbastro opened this issue Jun 16, 2021 · 7 comments

Comments

@dgil-unedbarbastro
Copy link
Contributor

Hi,
I need to receive those fields which currently are not being parsed:

  • OnixPriceTax.TaxRatePercent
  • OnixPublishingDetail.CountryOfPublication

I've already added to my local cloned ONIX-Data repo, if you want, I can create a pull-request so we can contribute to this repo.

Regards,
Daniel

@jaerith
Copy link
Owner

jaerith commented Jun 17, 2021

Hi Daniel,

Yes, I would definitely welcome the pull request!

Thanks for any contribution,

Aaron

@dgil-unedbarbastro
Copy link
Contributor Author

Great, probably you will have to give me contributor access, because right now, I'm getting a forbidden 403 error:

Pushing Added_TaxRatePercent_CountryOfPublication
Error encountered while pushing branch to the remote repository: Git failed with a fatal error.
Git failed with a fatal error.
unable to access 'https://github.com/jaerith/ONIX-Data.git/': The requested URL returned error: 403
Pushing to https://github.com/jaerith/ONIX-Data.git

Failed to push the branch to the remote repository. See the Output window for more details.

Btw, I've treated the TaxRatePercent field as a string although in the documentaion it's defined as a Real. I'm doing the same as you do for the field PriceAmount.

image

Regards,
Daniel

@szolkowski
Copy link
Contributor

@dgil-unedbarbastro Maybe you could fork this repository and make pull request from your fork? It is usually better way than giving access directly to "source" repo and GitHub have really nice support for such use case.

@jaerith
Copy link
Owner

jaerith commented Jun 18, 2021

Yes, @Stanislaw000 is correct. Perhaps one day I will add other contributors as authors to the project, but for now, I prefer to go with the route of pull requests.

@jaerith
Copy link
Owner

jaerith commented Jun 18, 2021

Thanks for the enhancements, @dgil-unedbarbastro! Yes, I tend to treat numeric fields as strings, mainly since the XML/JSON libraries will fail deserialization in the face of a bad number, and then the entire record becomes unavailable. But that's where the helper methods come into play. For example, on Price, you'll find PriceAmountNum, which does the work of providing a numeric version for you.

However, if something bad does actually happen upon deserialization, you can detect and observe the error by calling IsValid() and GetParsingError() on the Product class, as well as GetInputXml() to identify the problematic record.

@dgil-unedbarbastro
Copy link
Contributor Author

@dgil-unedbarbastro Maybe you could fork this repository and make pull request from your fork? It is usually better way than giving access directly to "source" repo and GitHub have really nice support for such use case.

Done! Forked and created a new Pull Request

@jaerith
Copy link
Owner

jaerith commented Jun 22, 2021

Cool! I merged the pull, and I add some properties for the short tags, along with a helper method for that numeric field.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants