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

[spaceship] Adding support for DEP_ID #14852

Merged
merged 1 commit into from Aug 30, 2019

Conversation

heartofagoof
Copy link
Contributor

Checklist

  • I've run bundle exec rspec from the root directory to see all new and existing tests pass
  • I've followed the fastlane code style and run bundle exec rubocop -a to ensure the code style is valid
  • I've read the Contribution Guidelines
  • I've updated the documentation if necessary.

Motivation and Context

This change allows the DEP id to be updated in avaliability - this will be default in coming times instead of VPP id.

Description

  • Added the class B2bOrganization and added related parsing for it in Availability and TunesClient
  • Added tests
  • Updated test fixtures to work with the tests

@janpio janpio changed the title Adding support for DEP_ID [spaceship] Adding support for DEP_ID Jun 19, 2019
@max-ott
Copy link
Contributor

max-ott commented Jul 5, 2019

Hi @heartofagoof thanks for your PR! What's the easiest way to test this via spaceship / irb so we can get this approved?

Copy link

@leifktaylor leifktaylor left a comment

Choose a reason for hiding this comment

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

I've tested this in IRB and Spaceship Sandbox.
It works like a charm. I have REDACTED the values of the company names etc in the output to protect my the privacy of my firm.

Below is the output of the terminal:

# Summary of first few lines without output
[20] pry(#<Spaceship::Playground>)> app = Spaceship::Application.find('REDACTED')
[22] pry(#<Spaceship::Playground>)> org_map = {'type' => 'ADD', 'depCustomerId' => 'REDACTED', 'organizationId' => 'REDACTED', 'name' => 'REDACTED'}

[26] pry(#<Spaceship::Playground>)> new_org = Spaceship::Tunes::B2bOrganization.new('value' =>  org_map)
=> <Spaceship::Tunes::B2bOrganization 
	type="ADD", 
	dep_customer_id="REDACTED", 
	dep_organization_id="REDACTED", 
	name="REDACTED">

[28] pry(#<Spaceship::Playground>)> availability = app.availability
=> <Spaceship::Tunes::Availability 
	include_future_territories=true, 
	territories=[<Spaceship::Tunes::Territory 
		code="US", 
		currency_code="USD", 
		name="United States", 
		region="The United States and Canada", 
		region_locale_key="ITC.region.NAM">], 
	cleared_for_preorder=false, 
	app_available_date=nil, 
	b2b_app_enabled=false, 
	educational_discount=true, 
	b2b_unavailable=false, 
	b2b_users=[], 
	b2b_organizations=[]>
[29] pry(#<Spaceship::Playground>)> availability.enable_b2b_app!
=> <Spaceship::Tunes::Availability 
	include_future_territories=true, 
	territories=[<Spaceship::Tunes::Territory 
		code="US", 
		currency_code="USD", 
		name="United States", 
		region="The United States and Canada", 
		region_locale_key="ITC.region.NAM">], 
	cleared_for_preorder=false, 
	app_available_date=nil, 
	b2b_app_enabled=true, 
	educational_discount=false, 
	b2b_unavailable=false, 
	b2b_users=[], 
	b2b_organizations=[]>
[30] pry(#<Spaceship::Playground>)> availability.update_dep_organizations([new_org])
=> <Spaceship::Tunes::Availability 
	include_future_territories=true, 
	territories=[<Spaceship::Tunes::Territory 
		code="US", 
		currency_code="USD", 
		name="United States", 
		region="The United States and Canada", 
		region_locale_key="ITC.region.NAM">], 
	cleared_for_preorder=false, 
	app_available_date=nil, 
	b2b_app_enabled=true, 
	educational_discount=false, 
	b2b_unavailable=false, 
	b2b_users=[], 
	b2b_organizations=[<Spaceship::Tunes::B2bOrganization 
		type="ADD", 
		dep_customer_id="REDACTED", 
		dep_organization_id="REDACTED", 
		name="REDACTED">]>
[31] pry(#<Spaceship::Playground>)> app.update_availability!(availability)
=> <Spaceship::Tunes::Availability 
	include_future_territories=true, 
	territories=[<Spaceship::Tunes::Territory 
		code="US", 
		currency_code="USD", 
		name="United States", 
		region="The United States and Canada", 
		region_locale_key="ITC.region.NAM">], 
	cleared_for_preorder=false, 
	app_available_date=nil, 
	b2b_app_enabled=true, 
	educational_discount=false, 
	b2b_unavailable=false, 
	b2b_users=[], 
	b2b_organizations=[<Spaceship::Tunes::B2bOrganization 
		type="NO_CHANGE", 
		dep_customer_id="REDACTED", 
		dep_organization_id=REDACTED, 
		name="REDACTED">]>
[32] pry(#<Spaceship::Playground>)> 

Below is the POST and response (redacted) in the spaceship logs.

INFO  [16:12:13]: >> POST ra/apps/REDACTED/pricing/intervals: {"sectionErrorKeys":[],"sectionInfoKeys":[],"sectionWarningKeys":[],"value":null,"countriesChanged":true,"countries":[{"code":"US"}],"b2BAppFlagDisabled":false,"educationDiscountDisabledFlag":false,"hotFudgeFeatureEnabled":true,"coldFudgeFeatureEnabled":true,"bitcodeAutoRecompileDisAllowed":false,"b2bAppEnabled":true,"educationalDiscount":false,"theWorld":true,"pricingIntervalsFieldTO":{"value":[{"priceTierEffectiveDate":null,"priceTierEndDate":null,"priceTierEffectiveISODate":null,"priceTierEndISODate":null,"tierStem":"0"}],"isEditable":true,"isRequired":false,"errorKeys":null},"availableDate": REDACTED,"unavailableDate":-2,"creatingApp":false,"hasApprovedVersion":false,"preOrder":{"clearedForPreOrder":{"value":false,"isEditable":true,"isRequired":true,"errorKeys":null},"appAvailableDate":{"value":null,"isEditable":true,"isRequired":true,"errorKeys":null},"preOrderAvailableDate":null},"appVersionsForOTAByPlatforms":{"iOS":[]},"b2bUsers":[],"b2bOrganizations":[{"value":{"type":"ADD","depCustomerId":"REDACTED","organizationId":"REDACTED","name":"REDACTED"}}]} 
DEBUG [16:12:26]: << POST ra/apps/REDACTED/pricing/intervals: 200 {"data"=>{"sectionErrorKeys"=>[], "sectionInfoKeys"=>[], "sectionWarningKeys"=>[], "value"=>nil, "countriesChanged"=>false, "countries"=>[{"code"=>"US", "name"=>"United States", "region"=>"The United States and Canada", "regionLocaleKey"=>"ITC.region.NAM", "currencyCodeISO3A"=>"USD"}], "b2BAppFlagDisabled"=>false, "educationDiscountDisabledFlag"=>false, "hotFudgeFeatureEnabled"=>true, "coldFudgeFeatureEnabled"=>true, "bitcodeAutoRecompileDisAllowed"=>false, "b2bAppEnabled"=>true, "educationalDiscount"=>false, "theWorld"=>true, "pricingIntervalsFieldTO"=>{"value"=>[{"priceTierEffectiveDate"=>nil, "priceTierEndDate"=>nil, "priceTierEffectiveISODate"=>nil, "priceTierEndISODate"=>nil, "tierStem"=>"0"}], "isEditable"=>true, "isRequired"=>false, "errorKeys"=>nil}, "availableDate"=>1562914800000, "unavailableDate"=>-2, "creatingApp"=>false, "hasApprovedVersion"=>false, "preOrder"=>{"clearedForPreOrder"=>{"value"=>false, "isEditable"=>true, "isRequired"=>true, "errorKeys"=>nil}, "appAvailableDate"=>{"value"=>nil, "isEditable"=>true, "isRequired"=>true, "errorKeys"=>nil}, "preOrderAvailableDate"=>nil}, "appVersionsForOTAByPlatforms"=>{"iOS"=>[]}, "b2bUsers"=>[], "b2bOrganizations"=>[{"value"=>{"type"=>"NO_CHANGE", "depCustomerId"=>"REDACTED", "organizationId"=> REDACTED, "name"=>"REDACTED"}, "isEditable"=>true, "isRequired"=>false, "errorKeys"=>nil}]}, "messages"=>{"warn"=>nil, "error"=>nil, "info"=>nil}, "statusCode"=>"SUCCESS"}

I've attached a screenshot of the GUI in the appstore showing the added B2B customer.
Screen Shot 2019-07-12 at 4 13 23 PM

@leifktaylor
Copy link

@max-ott I hope this review is sufficient.
I hope this gets merged in soon.
This fix unblocks my team.

@heartofagoof
Copy link
Contributor Author

@leifktaylor Thank you for your interest in the PR and thank you for taking time to test the change. : )

@max-ott I think @leifktaylor s steps are all you need to test. One thing that is left untested in his review is whether the change can successfully add a new DEP id and remove an existing one in one go.

You will need to get https://support.apple.com/en-us/HT204142 - apple business maanger ( or school manager ) on your account and then you can get a DEP id.

@leifktaylor
Copy link

leifktaylor commented Jul 17, 2019

@max-ott @janpio @heartofagoof
Tested ADD and DELETE in the same request. Works great.
I think this is ready for merge.

There is a quality of life issue that you may want to look into, see the comments at the bottom of the post.


# Create app object
[1] pry(#<Spaceship::Playground>)> app = Spaceship::Application.find('REDACTED BUNDLEID')
=> <Spaceship::Tunes::Application 
	apple_id="REDACTED_APPLE_ID", 
	name="REDACTED APPNAME", 
	vendor_id="REDACTED VENDORID", 
	bundle_id="REDACTED BUNDLEID", 
	last_modified=REDACTED,
	issues_count=0, 
	app_icon_preview_url="REDACTED URL", 
	version_sets=[<Spaceship::Tunes::VersionSet 
		type="APP", 
		application=<Spaceship::Tunes::Application 
		#<Object ...>>, 
		platform="ios">]>
		
# Instantiate two b2b organization objects (One to remove, one to add)
[2] pry(#<Spaceship::Playground>)> add_map = {'type' => 'ADD', 'depCustomerId' => 'REDACTED_DEP_ID', 'organizationId' => 'REDACTED_DEP_ID', 'name' => 'Fake Company A'}
=> {"type"=>"ADD", "depCustomerId"=>"REDACTED_DEP_ID", "organizationId"=>"REDACTED_DEP_ID", "name"=>"Fake Company A"}
[3] pry(#<Spaceship::Playground>)> remove_map = {'type' => 'DELETE', 'depCustomerId' => 'REDACTED_DEP_ID', 'organizationId' => 'REDACTED_DEP_ID', 'name' => 'Fake Company B'}
=> {"type"=>"DELETE", "depCustomerId"=>"REDACTED_DEP_ID", "organizationId"=>"REDACTED_DEP_ID", "name"=>"Fake Company B"}
[4] pry(#<Spaceship::Playground>)> add_org = Spaceship::Tunes::B2bOrganization.new('value' => add_map)
=> <Spaceship::Tunes::B2bOrganization 
	type="ADD", 
	dep_customer_id="REDACTED_DEP_ID A", 
	dep_organization_id="REDACTED_DEP_ID A", 
	name="Fake Company A">
[5] pry(#<Spaceship::Playground>)> remove_org = Spaceship::Tunes::B2bOrganization.new('value' =>  remove_map)
=> <Spaceship::Tunes::B2bOrganization 
	type="DELETE", 
	dep_customer_id="REDACTED_DEP_ID B", 
	dep_organization_id="REDACTED_DEP_ID B", 
	name="Fake Company B">

# Get Pricing and Availability object for the app
[6] pry(#<Spaceship::Playground>)> availability = app.availability
=> <Spaceship::Tunes::Availability 
	include_future_territories=true, 
	territories=[<Spaceship::Tunes::Territory 
		code="US", 
		currency_code="USD", 
		name="United States", 
		region="The United States and Canada", 
		region_locale_key="ITC.region.NAM">], 
	cleared_for_preorder=false, 
	app_available_date=nil, 
	b2b_app_enabled=true, 
	educational_discount=false, 
	b2b_unavailable=false, 
	b2b_users=[], 
	b2b_organizations=[<Spaceship::Tunes::B2bOrganization 
		type="NO_CHANGE", 
		dep_customer_id="REDACTED_DEP_ID B", 
		dep_organization_id=REDACTED_DEP_ID, 
		name="Fake Company B">]>

# Clear the current b2b organizations before adding our ADD and DELETE b2b objects to the b2b_organizations list.
# This is to avoid duplicate entries (e.g. an ADD b2b obj, and a NO_CHANGE b2b obj of the same Organization)
# duplicate entries will cause API error, see below.
[7] pry(#<Spaceship::Playground>)> availability.b2b_organizations = []
=> []

# Add the 2 b2b organization objects to the b2b_organizations array
[8] pry(#<Spaceship::Playground>)> availability.update_dep_organizations([add_org, remove_org])
=> <Spaceship::Tunes::Availability 
	include_future_territories=true, 
	territories=[<Spaceship::Tunes::Territory 
		code="US", 
		currency_code="USD", 
		name="United States", 
		region="The United States and Canada", 
		region_locale_key="ITC.region.NAM">], 
	cleared_for_preorder=false, 
	app_available_date=nil, 
	b2b_app_enabled=true, 
	educational_discount=false, 
	b2b_unavailable=false, 
	b2b_users=[], 
	b2b_organizations=[<Spaceship::Tunes::B2bOrganization 
		type="ADD", 
		dep_customer_id="REDACTED_DEP_ID A", 
		dep_organization_id="REDACTED_DEP_ID A", 
		name="Fake Company A">, <Spaceship::Tunes::B2bOrganization 
		type="DELETE", 
		dep_customer_id="REDACTED_DEP_ID B", 
		dep_organization_id="REDACTED_DEP_ID B", 
		name="Fake Company B">]>

# Update availability command goes through and works
[9] pry(#<Spaceship::Playground>)> app.update_availability!(availability)
=> <Spaceship::Tunes::Availability 
	include_future_territories=true, 
	territories=[<Spaceship::Tunes::Territory 
		code="US", 
		currency_code="USD", 
		name="United States", 
		region="The United States and Canada", 
		region_locale_key="ITC.region.NAM">], 
	cleared_for_preorder=false, 
	app_available_date=nil, 
	b2b_app_enabled=true, 
	educational_discount=false, 
	b2b_unavailable=false, 
	b2b_users=[], 
	b2b_organizations=[<Spaceship::Tunes::B2bOrganization 
		type="NO_CHANGE", 
		dep_customer_id="REDACTED_DEP_ID A", 
		dep_organization_id=REDACTED_DEP_ID A, 
		name="Fake Company A">]>
[10] pry(#<Spaceship::Playground>)> 

Possible Quality of life improvement for next PR

Note: Regarding above comment about duplicate entries (e.g. an ADD b2b obj and a NO_CHANGE b2b obj of the same organization):

The new availability.update_dep_organizations method will not automatically remove NO_CHANGE entry of ORGANIZATION_A if you are adding a DELETE entry for ORGANIZATION_A.

E.g. , if we have an Availability.b2b_organizations array like this:

	b2b_organizations=[<Spaceship::Tunes::B2bOrganization 
		type="NO_CHANGE", 
		dep_customer_id="REDACTED", 
		dep_organization_id=REDACTED, 
		name="COMPANY A">]>

And then we pass in this object with availability.update_dep_organizations:

<Spaceship::Tunes::B2bOrganization 
		type="DELETE", 
		dep_customer_id="REDACTED", 
		dep_organization_id="REDACTED", 
		name="COMPANY A">]

Our availability.b2b_organizations object will now look like this, and will be rejected by the Appstore API if we attempt to call app.update_availability!

	b2b_organizations=[<Spaceship::Tunes::B2bOrganization 
		type="DELETE", 
		dep_customer_id="REDACTED", 
		dep_organization_id="REDACTED", 
		name="COMPANY A">, <Spaceship::Tunes::B2bOrganization 
		type=false, 
		dep_customer_id="REDACTED", 
		dep_organization_id=REDACTED, 
		name="COMPANY A">]>

Calling app.update_availability with this b2b_organizations array will result in an API Error from Apple.

[31] pry(#<Spaceship::Playground>)> app.update_availability!(availability)
Spaceship::Tunes::Error: invalid_request

@EvilTony
Copy link

Is there any chance we can get this reviewed and merged soon @max-ott ?

It is clean enough to work with and I also need this to help speed up my CI/CD pipeline and remove manual actions.

@max-ott
Copy link
Contributor

max-ott commented Jul 23, 2019

I don't have write access, so my review does not really count here. With all the changes going on with regards to TestFlight etc. more niche things might take more time. Sorry about that 🙏

@leifktaylor
Copy link

@joshdholtz
Are there any further actions I can take to expedite this PR?

Copy link
Member

@joshdholtz joshdholtz left a comment

Choose a reason for hiding this comment

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

Thanks again for adding all of the b2b logic! ❤️ This is 🔥

@joshdholtz joshdholtz merged commit 62fb4c5 into fastlane:master Aug 30, 2019
@fastlane-bot
Copy link

Congratulations! 🎉 This was released as part of fastlane 2.130.0 🚀

@fastlane fastlane locked and limited conversation to collaborators Oct 29, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

8 participants