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

Added a primary address to Zizi Retail #155

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

danieltaylor-nz
Copy link
Member

Added a primary address, because without it, the service ContactServices.get#PartyContactInfo returns nothing on demo data.

…ces.get#PartyContactInfo returns nothing on demo data
@jonesde
Copy link
Member

jonesde commented May 9, 2020

This could be there just to have a Primary address for ZIRET, but what is the business context where it is needed? There are lots of services, not all demo data is meant to be used for all services, doing so wouldn't even make sense from business requirement or rational design perspectives.

@danieltaylor-nz
Copy link
Member Author

danieltaylor-nz commented May 10, 2020

I think you already saw this PR linked in #151 where I was trying to run the code and ContactServices.get#PartyContactInfo was returning nothing despite defaultToPrimaryPurpose being passed. While I expected that 'PostalOrder' would return nothing for the demo org, I expected to see values because of defaultToPrimaryPurpose.

I lost a bit of time discovering that the demo data had no 'primary address', as I had assumed that a primary address would exist given that it's treated as a default value.

I thought that one of the purposes of demo data was for developers to have a quick start for running/testing code. I also assume that the demo data should be the ideal data-set, i.e, default values are present. While not all services have to have relevant data, ContactServices.get#PartyContactInfo gets used a lot, so having working data could save other developers some time. There is no business case.

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

Successfully merging this pull request may close these issues.

2 participants