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

check multiple address types #122

Merged
merged 2 commits into from
Jul 14, 2023
Merged

check multiple address types #122

merged 2 commits into from
Jul 14, 2023

Conversation

adamshire123
Copy link
Contributor

@adamshire123 adamshire123 commented Jul 13, 2023

Why these changes are being introduced:

We want to use the address specified as the payment address in alma. An address can have multiple types in alma. We were only checking the first type of each address to see if it was a payment address, so in these cases we were not correctly identifying the payment address

How this addresses that need:

Checks all address types for each address to determine if an address is the payment address

What does this PR do?

fixes bug where the payment address was not recognized if the address had multiple types
updates unit test

Helpful background context

an address can have multiple types in Alma. We want to use the address with the type "payment".

Includes new or updated dependencies?

YES

What are the relevant tickets?

Developer

  • All new ENV is documented in README (or there is none)
  • Stakeholder approval has been confirmed (or is not needed)

Code Reviewer

  • The commit message is clear and follows our guidelines
    (not just this pull request message)
  • There are appropriate tests covering any new functionality
  • The documentation has been updated or is unnecessary
  • The changes have been verified
  • New dependencies are appropriate or there were no changes

Why these changes are being introduced:

We want to use the address specified as the payment address in alma.
An address can have multiple types in alma. We were only checking the first type of each
address to see if it was a payment address, so in these cases we were not correctly
identifying the payment address

How this addresses that need:

Checks all address types for each address to determine if an address is the payment
address
pinning click to version 8.1.3 because click 8.1.4 and 8.1.5 have a
conflict with mypy 1.4.1

see:
pallets/click#255
Copy link

@ehanson8 ehanson8 left a comment

Choose a reason for hiding this comment

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

👍 Looks good. Also, we'll be tagging this repo in an issue so that when it's resolved, we can update all the affected repos

@adamshire123 adamshire123 merged commit 66dc782 into main Jul 14, 2023
@adamshire123 adamshire123 deleted the multiple-vendor-types branch July 14, 2023 19:27
@ehanson8 ehanson8 restored the multiple-vendor-types branch July 17, 2023 20:10
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.

2 participants