Skip to content

Conversation

@chrisrollins65
Copy link
Contributor

No description provided.


// Validate checks if the given VAT number exists and is active. If no error is returned, then it is.
func (s *ukVATService) Validate(vatNumber string) error {
func (s *ukVATService) Validate(vatNumber string, opts ValidatorOpts) error {
Copy link
Contributor

@bakaat bakaat Feb 25, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd need to try this to be honest, but I'd suggest handling lookup service configuration, authentication and auth caching within the package. Sure it might take a bit longer but we're talking about major package version change anyway as we're either changing the Validate interface or the flow.

Personally I'd suggest to try providing additional "Configure" function if we want to keep the current structure. Flow would change to something like

vat.Configure(&ValidatorOptions{...}) 

or 

vat.Configure([]LookupServiceInterface{NewUKLookupService{username, pass, etc.}, NewVEISLookupService})

// after that calling Validate would mean it goes through configured services
Validate(vatNumber) {
  if vatNumber.HasPrefix(GB) {
    if lookupService := vat.LookupServiceManager.get("GB") {
      return lookupService.Validate(vatNumber)
    } else { 
      return LookupServiceNotConfigured{} 
    }
  }
}

alternatively ditch the single Validate function being exposed and provide a NewVat() with ability to configure validators, stop using the package directly. Initialise and pass where need instead.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Discussed directly. We can look at the whole package design later on. For now we'll just keep it simple and get rid of the errors we're getting atm.

Copy link
Contributor Author

@chrisrollins65 chrisrollins65 Feb 26, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This all sounds good, would probably need some sort of cache interface setup and what not. As @bakaat mentioned, we can use the changes in this PR as a "fix" for the UK issue in this version and we can implement those other types of changes in the next version (v4) of this package.

Copy link
Contributor

@bakaat bakaat left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm


// Validate checks if the given VAT number exists and is active. If no error is returned, then it is.
func (s *ukVATService) Validate(vatNumber string) error {
func (s *ukVATService) Validate(vatNumber string, opts ValidatorOpts) error {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Discussed directly. We can look at the whole package design later on. For now we'll just keep it simple and get rid of the errors we're getting atm.

@chrisrollins65 chrisrollins65 merged commit cfd1afb into master Feb 26, 2025
2 checks passed
@chrisrollins65 chrisrollins65 deleted the feat-use-newest-uk-api branch February 26, 2025 10:49
@chrisrollins65 chrisrollins65 linked an issue Feb 26, 2025 that may be closed by this pull request
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Version 1 of the UK Validation service has been deprecated

3 participants